imshow zorder (patch for review)

Andrew_Straw5 · November 9, 2009, 4:21pm

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly ignored for imshow(). (See e.g. http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314 ) The question is whether I should apply the attached patch.

The worry is that someone is relying on the old behavior of images being underneath everything else. To a degree, this could be achieved by having the default image zorder be lower than everything else. Currently, imshow() creates images with zorder 1, which is pretty low on the hierarchy, but there may be something lower. (The artist base class default is 0, but the Line2D class bumps it to 2.) All tests pass with the change, and manual inspection of several of the pylab_examples don't show any difference before and after the patch.

-Andrew

image_zorder.patch (565 Bytes)

Gael_Varoquaux1 · November 9, 2009, 4:28pm

IMHO: go for it!

Gaël

···

On Mon, Nov 09, 2009 at 08:21:30AM -0800, Andrew Straw wrote:

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly
ignored for imshow(). (See e.g.
http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314
) The question is whether I should apply the attached patch.

Eric_Firing2 · November 9, 2009, 5:23pm

Andrew Straw wrote:

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly ignored for imshow(). (See e.g. http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314 ) The question is whether I should apply the attached patch.

Andrew,

Looks good to me. I think you should go ahead.

Eric

···

The worry is that someone is relying on the old behavior of images being underneath everything else. To a degree, this could be achieved by having the default image zorder be lower than everything else. Currently, imshow() creates images with zorder 1, which is pretty low on the hierarchy, but there may be something lower. (The artist base class default is 0, but the Line2D class bumps it to 2.) All tests pass with the change, and manual inspection of several of the pylab_examples don't show any difference before and after the patch.

-Andrew

------------------------------------------------------------------------

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july

------------------------------------------------------------------------

_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options

_Jae-Joon_Lee · November 9, 2009, 5:29pm

Andrew,

One of my worry is that this can results in inconsistent ouputs
between backends. Your patch only affects backends with compositing
capabilities. And backends such as ps backend will still render images
at the bottom of all other artists.

I think it is often sufficient if we draw images at the bottom but
respect zorders among images, and I guess this actually solves the
issue of the original post.

So, my inclination is to respect zorders between images by default
(images still drawn at the bottom), and optionally with all other
artists.

Anyhow, I guess the cases affected by this issue is quite rare, and
I'm actually fine if you submit the patch as is.

Regards,

-JJ

···

On Mon, Nov 9, 2009 at 11:21 AM, Andrew Straw <strawman@...36...> wrote:

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly
ignored for imshow(). (See e.g.
http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314
) The question is whether I should apply the attached patch.

The worry is that someone is relying on the old behavior of images being
underneath everything else. To a degree, this could be achieved by having
the default image zorder be lower than everything else. Currently, imshow()
creates images with zorder 1, which is pretty low on the hierarchy, but
there may be something lower. (The artist base class default is 0, but the
Line2D class bumps it to 2.) All tests pass with the change, and manual
inspection of several of the pylab_examples don't show any difference before
and after the patch.

-Andrew

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus
on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options

Eric_Firing2 · November 9, 2009, 5:44pm

Jae-Joon Lee wrote:

Andrew,

One of my worry is that this can results in inconsistent ouputs
between backends. Your patch only affects backends with compositing
capabilities. And backends such as ps backend will still render images
at the bottom of all other artists.

PS backend already does things differently from others because it doesn't handle alpha, correct? Does the patch make this situation any worse?

Eric

···

I think it is often sufficient if we draw images at the bottom but
respect zorders among images, and I guess this actually solves the
issue of the original post.

So, my inclination is to respect zorders between images by default
(images still drawn at the bottom), and optionally with all other
artists.

Anyhow, I guess the cases affected by this issue is quite rare, and
I'm actually fine if you submit the patch as is.

Regards,

-JJ

On Mon, Nov 9, 2009 at 11:21 AM, Andrew Straw <strawman@...36...> wrote:

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly
ignored for imshow(). (See e.g.
http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314
) The question is whether I should apply the attached patch.

The worry is that someone is relying on the old behavior of images being
underneath everything else. To a degree, this could be achieved by having
the default image zorder be lower than everything else. Currently, imshow()
creates images with zorder 1, which is pretty low on the hierarchy, but
there may be something lower. (The artist base class default is 0, but the
Line2D class bumps it to 2.) All tests pass with the change, and manual
inspection of several of the pylab_examples don't show any difference before
and after the patch.

-Andrew

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus
on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options

_John_Hunter · November 9, 2009, 6:01pm

Will this help with the OP's original "wart", which was the inability
to add multiple images and move one to the top with zorder. Your
patch is only applied when len(images)<=1 or
renderer.option_image_nocomposite(), both of which will be False when
using Agg with multiple images, no?

JDH

···

On Mon, Nov 9, 2009 at 10:21 AM, Andrew Straw <strawman@...36...> wrote:

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly
ignored for imshow(). (See e.g.
http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314
) The question is whether I should apply the attached patch.

_Jae-Joon_Lee · November 9, 2009, 6:12pm

I believe renderer.option_image_nocomposite() is True for the agg backend.
So, it will work with agg.

-JJ

···

On Mon, Nov 9, 2009 at 1:01 PM, John Hunter <jdh2358@...149...> wrote:

Your
patch is only applied when len(images)<=1 or
renderer.option_image_nocomposite(), both of which will be False when
using Agg with multiple images, no?

Andrew_Straw5 · November 9, 2009, 6:16pm

John Hunter wrote:

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly
ignored for imshow(). (See e.g.
http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314
) The question is whether I should apply the attached patch.

Will this help with the OP's original "wart", which was the inability
to add multiple images and move one to the top with zorder. Your
patch is only applied when len(images)<=1 or
renderer.option_image_nocomposite(), both of which will be False when
using Agg with multiple images, no?

OK, sorry I linked to a motivating example that actually wasn't exactly the same as the use case I have -- I want a single image to play well with other artists' zorder. This patch does handle that scenario. I simply was too quick choosing this example out of the email history to justify the patch.

So now the question for me is what is this option_image_nocomposite is so that I can generalize the patch to both when it's True and False. From the emails here, (my only knowledge on it so far) it seems it has something to do with the capabilities of the renderer rather than a run-time option selected by the user. I'll dig a little deeper into this option and see if my patch can deal with other cases, too. Stay tuned...

-Andrew

···

On Mon, Nov 9, 2009 at 10:21 AM, Andrew Straw <strawman@...36...> wrote:

_John_Hunter · November 9, 2009, 6:18pm

Ahh, you're right. Thanks.

JDH

···

On Mon, Nov 9, 2009 at 12:12 PM, Jae-Joon Lee <lee.j.joon@...149...> wrote:

On Mon, Nov 9, 2009 at 1:01 PM, John Hunter <jdh2358@...149...> wrote:

Your
patch is only applied when len(images)<=1 or
renderer.option_image_nocomposite(), both of which will be False when
using Agg with multiple images, no?

I believe renderer.option_image_nocomposite() is True for the agg backend.
So, it will work with agg.

_John_Hunter · November 9, 2009, 6:25pm

The compositing is in support of things like
pylab_examples/layer_images.py, where two images, possibly of
different sizes, are composited together

···

On Mon, Nov 9, 2009 at 12:16 PM, Andrew Straw <strawman@...36...> wrote:

So now the question for me is what is this option_image_nocomposite is so
that I can generalize the patch to both when it's True and False. From the

_Jae-Joon_Lee · November 9, 2009, 6:32pm

When there are multiple Images and render.option_image_nocomposite()
is false (as in the ps backend ), the images are composited
(rasterized) into a single image before it is passed to the backend.
So, AS FAR AS the images are concerned, I think the ps backend does
handle alpha correctly and the results are consistent between
backends.

And with the Andrew's patch, and if there are non-image artists
between two images, the results can be different. For example, those
non-image artists can be obscured by the foreground image in the agg
backend, but will always show up in the ps backend.
So, I think this is somewhat different issue than alpha being ignored
in the ps backend.

Regards,

-JJ

···

On Mon, Nov 9, 2009 at 12:44 PM, Eric Firing <efiring@...229...> wrote:

PS backend already does things differently from others because it doesn't
handle alpha, correct? Does the patch make this situation any worse?

Andrew_Straw5 · November 9, 2009, 8:55pm

Jae-Joon Lee wrote:

PS backend already does things differently from others because it doesn't
handle alpha, correct? Does the patch make this situation any worse?

When there are multiple Images and render.option_image_nocomposite()
is false (as in the ps backend ), the images are composited
(rasterized) into a single image before it is passed to the backend.
So, AS FAR AS the images are concerned, I think the ps backend does
handle alpha correctly and the results are consistent between
backends.

And with the Andrew's patch, and if there are non-image artists
between two images, the results can be different. For example, those
non-image artists can be obscured by the foreground image in the agg
backend, but will always show up in the ps backend.
So, I think this is somewhat different issue than alpha being ignored
in the ps backend.

What's the motivation of the ps backend "compositing" (rasterizing to a single bitmap) multiple images? It seems it will, by design, preclude the use of non-image artists between two images. I guess the motivation is to reduce output file size. Maybe Axes.draw() should disable compositing in the scenario where a non-image artist is sandwiched by image artists and suffer the increased file size.

-Andrew

···

On Mon, Nov 9, 2009 at 12:44 PM, Eric Firing <efiring@...229...> wrote:

_Jae-Joon_Lee · November 9, 2009, 10:05pm

I can't say about what the original intention was, but I guess one of
the benefit is that you can have alpha compositing of multiple images.
I personally think that no alpha support for images is a more serious
issue than no alpha support for other artists like lines and patches.
So, I'm +1 for doing the alpha compositing of images before passing it
to the backends (that do not support the alpha compositing).

However, as I said in my first email, I don't think there are much
cases that this actually matters,
while I'm also not sure about the benefit of having images above other
artists like lines.

Anyhow, as far as zorders between images are respected (in all
backends!), it should be good.

Regards,

-JJ

···

On Mon, Nov 9, 2009 at 3:55 PM, Andrew Straw <strawman@...36...> wrote:

What's the motivation of the ps backend "compositing" (rasterizing to a
single bitmap) multiple images? It seems it will, by design, preclude the
use of non-image artists between two images. I guess the motivation is to
reduce output file size. Maybe Axes.draw() should disable compositing in the
scenario where a non-image artist is sandwiched by image artists and suffer
the increased file size.

Andrew_Straw5 · November 10, 2009, 9:06pm

Andrew Straw wrote:

John Hunter wrote:

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly
ignored for imshow(). (See e.g.
http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314
) The question is whether I should apply the attached patch.

I went ahead and committed this patch to the trunk in r7950.

Here's the CHANGELOG entry:

2009-11-10 Single images, and all images in renderers with
           option_image_nocomposite (i.e. agg, macosx and the svg
           backend when rcParams['svg.image_noscale'] is True), are
           now drawn respecting the zorder relative to other
           artists. (Note that there may now be inconsistencies across
           backends when more than one image is drawn at varying
           zorders, but this change introduces correct behavior for
           the backends in which it's easy to do so.)

Jae Joon raised a couple of concerns:

1) the patch would introduces inconsistent behavior between backends -- namely that PS and other backends, when given more than one image, would "composite" (or rasterize) all images and stick them underneath everything else. Agg would have proper zordering between images and other artists.

2) there is doubt about the utility of such functionality. Jae-Joon says "I think it is often sufficient if we draw images at the bottom but respect zorders among images".

As for #1, it seems to me this is simply a bug/limitation with the compositing functionality (e.g. the PS backend). The SVG backend has the possibility to turn compositing on or off based on the svg.image_noscale rcParam, and perhaps other backends could grow something similar. I can't see why this patch -- that specifically solves the bug/limitation in the backends where its easily possible -- should be held back due to this.

As for #2, perhaps my use case will be convincing -- I'm trying to draw reconstructions of various experimental setups, and the easiest way to do this is to create texture-mapped rectangles in which I can control the zorder -- a single texture may need to be over some artists but under others. Perhaps there's an easier way to achieve this, but I'm not aware of it.

Jae Joon has also stated that he's OK with the patch, so I went ahead and committed it. In light of all this, please let me know if you still have concerns, and hopefully they can be addressed. In the worst case, we can back this patch out.

-Andrew

···

On Mon, Nov 9, 2009 at 10:21 AM, Andrew Straw <strawman@...36...> wrote:

_Jae-Joon_Lee · November 12, 2009, 12:07am

I also committed a small patch that makes zorders respected among
images (not with other artists) for noncomposit backends.

Anyhow, can we get rid of the second items in the "dsu" list? My guess
is that this is to make the sort stable, but python sort is already
stable, isn't it?

-JJ

···

On Tue, Nov 10, 2009 at 4:06 PM, Andrew Straw <strawman@...36...> wrote:

Andrew Straw wrote:

John Hunter wrote:

On Mon, Nov 9, 2009 at 10:21 AM, Andrew Straw <strawman@...36...> wrote:

Hi All,

I have addressed what I think is a long-standing wart: zorder is mostly
ignored for imshow(). (See e.g.
http://old.nabble.com/Re%3A--Matplotlib-users--imshow-zorder-tt19047314.html#a19047314
) The question is whether I should apply the attached patch.

I went ahead and committed this patch to the trunk in r7950.

Here's the CHANGELOG entry:

2009-11-10 Single images, and all images in renderers with
option_image_nocomposite (i.e. agg, macosx and the svg
backend when rcParams['svg.image_noscale'] is True), are
now drawn respecting the zorder relative to other
artists. (Note that there may now be inconsistencies across
backends when more than one image is drawn at varying
zorders, but this change introduces correct behavior for
the backends in which it's easy to do so.)

Jae Joon raised a couple of concerns:

1) the patch would introduces inconsistent behavior between backends --
namely that PS and other backends, when given more than one image, would
"composite" (or rasterize) all images and stick them underneath
everything else. Agg would have proper zordering between images and
other artists.

2) there is doubt about the utility of such functionality. Jae-Joon says
"I think it is often sufficient if we draw images at the bottom but
respect zorders among images".

As for #1, it seems to me this is simply a bug/limitation with the
compositing functionality (e.g. the PS backend). The SVG backend has the
possibility to turn compositing on or off based on the svg.image_noscale
rcParam, and perhaps other backends could grow something similar. I
can't see why this patch -- that specifically solves the bug/limitation
in the backends where its easily possible -- should be held back due to
this.

As for #2, perhaps my use case will be convincing -- I'm trying to draw
reconstructions of various experimental setups, and the easiest way to
do this is to create texture-mapped rectangles in which I can control
the zorder -- a single texture may need to be over some artists but
under others. Perhaps there's an easier way to achieve this, but I'm not
aware of it.

Jae Joon has also stated that he's OK with the patch, so I went ahead
and committed it. In light of all this, please let me know if you still
have concerns, and hopefully they can be addressed. In the worst case,
we can back this patch out.

-Andrew

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options

Andrew_Straw5 · November 12, 2009, 12:12am

Jae-Joon Lee wrote:

I also committed a small patch that makes zorders respected among
images (not with other artists) for noncomposit backends.

That looks fine to me. Thanks.

Anyhow, can we get rid of the second items in the "dsu" list? My guess
is that this is to make the sort stable, but python sort is already
stable, isn't it?

I had a patch waiting in the wings for that, but I wanted to see the dust settle before committing it. I think the dust is officially settled, so please commit yours or else I'll commit mine.

-Andrew

_Jae-Joon_Lee · November 12, 2009, 8:45pm

Please go ahead.

By the way, I just encountered some zorder issue with the new patch.
The thing is, zorder=1 for Images seems to high.
For example, patches also have zorders=1. So, if I draw an image, and
add some patches (which I often do to indicate regions of interests),
the patches are drawn before images and become invisible. This happens
because images are appended to dsu list after all other artists.

Reducing the zorder of images would be a one option. Or we may add
images to the dsu list before all other artists, which seems sensible
to me.

Regards,

-JJ

···

On Wed, Nov 11, 2009 at 7:12 PM, Andrew Straw <strawman@...36...> wrote:

I had a patch waiting in the wings for that, but I wanted to see the dust
settle before committing it. I think the dust is officially settled, so
please commit yours or else I'll commit mine.

_Jae-Joon_Lee · November 12, 2009, 8:54pm

The other thing I noticed by quickly going through axes.py is that
when _axisbelow==True, the zorders of xaxis and yaxis is set to 0.5,
which is lower than images. While the doc say

*axisbelow* draw the grids and ticks below the other
artists

I don't think one wants to draw axis (grid+ticks) even below images.

Well, there should be a few ways to fix this, but how others think
about reducing the Image's zorder to 0?

Regards,

-JJ

···

On Thu, Nov 12, 2009 at 3:45 PM, Jae-Joon Lee <lee.j.joon@...149...> wrote:

By the way, I just encountered some zorder issue with the new patch.
The thing is, zorder=1 for Images seems to high.
For example, patches also have zorders=1. So, if I draw an image, and
add some patches (which I often do to indicate regions of interests),
the patches are drawn before images and become invisible. This happens
because images are appended to dsu list after all other artists.

Andrew_Straw5 · November 12, 2009, 9:35pm

> I had a patch waiting in the wings for that, but I wanted to see the dust
> settle before committing it. I think the dust is officially settled, so
> please commit yours or else I'll commit mine.
>

Please go ahead.

Will do.

By the way, I just encountered some zorder issue with the new patch.
The thing is, zorder=1 for Images seems to high.
For example, patches also have zorders=1. So, if I draw an image, and
add some patches (which I often do to indicate regions of interests),
the patches are drawn before images and become invisible. This happens
because images are appended to dsu list after all other artists.

The other thing I noticed by quickly going through axes.py is that
when _axisbelow==True, the zorders of xaxis and yaxis is set to 0.5,
which is lower than images. While the doc say

*axisbelow* draw the grids and ticks below the other
artists

I don't think one wants to draw axis (grid+ticks) even below images.

Well, there should be a few ways to fix this, but how others think
about reducing the Image's zorder to 0?

I'm fine with the default Image's zorder at 0 -- I'm explicitly setting it for the cases I'm interested in anyway, and I think that would give better consistency with the old way. Also, I think this is better and more explicit than just inserting them at the beginning of the dsu list and relying on a stable sort to keep them drawn below various patches, which is how I interpreted your meaning for that approach.

I'll check in this change, too.

-Andrew

···

On Wed, Nov 11, 2009 at 7:12 PM, Andrew Straw <strawman@...36...> wrote: