Different alphas for lines and fills. #1899
Conversation
|
@mdboom - the two polygons, from what I can tell, should be identical. Clearly they are nowhere near. Is this something you'd expect? If not, I've already got a test that I can work towards fixing 😄 |
|
Yes -- I agree they should look the same. I wonder if this is related to #1860 in any way... we should probably at least try this on either side of it to check. |
|
Well -- probably not, actually, since I don't think #1860 would affect the Agg backend. |
|
I spent some time looking into this. The differences between how a patch and a collection in handle alpha values runs quite deep. Basically, for the backends I've looked at (AGG, macosx, and the base classes), renderer.draw_path (which is what Patch classes call) is NOT capable of having a different alphas for the edge and the face. This seems to be by design. When calling draw_path, the alpha channel value in rgbFace (if present) is overridden by the alpha value of the GraphicsContext (which is in turn either set explicitly, or inferred from the alpha value of the edgecolor, if it's in RGBA format). renderer.draw_path_collection (used to render Collection classes), on the other hand, has no explicit parameter(s) for alpha value -- it only has edgecolors and facecolors -- and the backends generally implement draw_path_collection independently of draw_path, using code paths that handle RGBA values for edgecolors and facecolors correctly. To fix Patches to support distinct alpha values, you'd need to make some non-trivial changes to how colours are handled in multiple backends (both in Renderers and GraphicsContexts), the Patch class itself, and possibly also the backend base classes. The absence of the proper linestyle for the Patch, on the other hand, is an easy-to-fix bug; GraphicsContext.set_linestyle seems to be ignored by backends in favour of GraphicsContext.set_dashes. The Patch class should do the conversion from linestyle to dashes itself (as Collection.set_linestyle does). |
|
Thanks @Westacular - nice write-up of what has clearly been some thorough investigation. Did you manage to see why the two red facecolors had different alphas? Does setting an alpha on an edgecolor modify the alpha of the face? |
|
Yes, thanks for that, @Westacular: This is a long-standing shortcoming, and one I'd like to fix. The difficulty has always been how to do it in a backward-compatible way -- or at least a way that handles the majority of existing uses in the wild. |
|
@pelson Yes, the alpha component of edgecolor overrides any alpha component in facecolor, except in the special case when facecolor=(x,x,x,0), for which the face is invisible. I did some testing as to the output produced by different combinations of the alpha, facecolor, and edgecolor arguments. The results: Case 1:alpha unset face and edge both rendered using the alpha value from edgecolor. If edgecolor is set using something other than an RGBA value, alpha=1 for both. Case 2:alpha set face and edge both rendered using the explicitly set alpha value Case 3:alpha unset edge is invisible, face rendered using its own alpha value Case 4:alpha unset edge is default color (black for me), both edge and face rendered with alpha=1. Case 5:alpha set face and edge both rendered using the explicitly set alpha value. Case 6:alpha set face and edge both rendered using the explicitly set alpha value. edge is rendered using default color (black). This seems like it should be considered a bug. Case 7:alpha unset face is invisible, edge rendered using its own alpha value Case 8:alpha unset face is default color (blue for me), both face and edge both rendered using alpha value of edgecolor Case 9:alpha set face is invisible, edge rendered using the explicitly set alpha value Case 10:alpha set face and edge rendered using the explicitly set alpha value Case 11:alpha unset nothing is rendered (face and edge both invisible) Case 12:alpha unset nothing is rendered (face and edge both invisible) Case 13:alpha set face is invisible, edge is rendered using default color with the explicitly set alpha value. Seems like the same bug as case 6. Case 14:alpha set face and edge both rendered using their respective colors, with the explicitly set alpha value In general, I think the most backward-compatible way to fix this would be to maintain the logic that the alpha argument overrides the alpha components of both facecolor and edgecolor, but change it so that if alpha=None, the facecolor and edgecolor alpha components are independently preserved. This would mean altering the output for cases 1, 4, and 8 in the above list. However, implementing this would require making potentially delicate changes to how the backends handle the combination of gc.set_alpha, gc.set_foreground (=edgecolor), and the rgbFace argument to the renderer (=facecolor); currently, at least for the draw_path methods, alpha components for both foreground and rgbFace are always ignored, regardless of whether the gc's alpha attribute has been set. (Also, the bug in cases 6 and 13 should be fixed, so that edgecolor='none' is not overridden by setting alpha. That part should be a relatively straightforward fix to just the Patch class). I'll go ahead and fix the little bugs affecting only the Patch class, and if you think it's worth a shot, I can take a crack at making the additional changes to the backends. |
@Westacular - you have my support to change this for the better. Given your detailed review, it sounds like your changes will be well considered, minimise the disruption and result in more sensible behaviour. In short, go for it! |
|
Closed based on having merged #1954. |
I saw an interesting question on Stackoverflow the other day for which I found it was possible to supply an RGBA for both face and edge colors separately. I'm guessing this isn't tested, so thought I would add a comprehensive test, unfortunately the test results are wrong (so don't merge this).
Am I doing something silly in the test?