Add blend modes and blend groups for compositing - #31162
Conversation
dee65fa to
37b0b9c
Compare
|
This looks interesting. Thanks for working on it. Since I’m not into the topic I can dare to ask the stupid questions:
|
Yup, that is correct: the history of how that "background" was constructed has no bearing on how the next Artist is blended in using its specific blend mode. It's also important to remember that that the "empty" background of an Axes is solid white, and thus not actually empty as far as these blend modes are concerned. For example, using "screen" to blend in an image on a truly empty background will just return the image, but on a white background will return solid white, which can make it look instead like the image call failed. That's why I turn off the face colors in my example above. What I still need to investigate is how my changes interact with collections of Artists. A user may want "over" blending (the default) within the collection before using a different blend mode for the collection as a whole.
Yes. In principle, the transformation functions for the hue/saturation/color/luminosity operators could have more than one possibility, but in practice I think everyone has simply used the same functions for decades (as defined in the PDF specification). |
37b0b9c to
bb554e7
Compare
2221d69 to
6d49bcf
Compare
|
This is pretty cool :-)
Actually I suspect that another possibility is to want some nonstandard blending between multiple artists, then "over" blending of the result over the background. In general I suspect this would be related to adding support for temporary, intermediate rendering buffers, which is also something that would be useful for other purposes e.g. contour label overplotting (#26971 (comment)). |
|
By the way, I decided to rename "over" to "normal". That mode of blending is referred to as "normal" often enough, and it makes it readily apparent to users that it is the standard choice (and the default). |
6d49bcf to
63f53c9
Compare
63f53c9 to
c9552d5
Compare
f49e8d0 to
f7af1e4
Compare
f7af1e4 to
d313054
Compare
d04f663 to
c287d35
Compare
|
Updates over the past two weeks:
|
story645
left a comment
There was a problem hiding this comment.
So big picture is this is fantastic and thank you for all the work and sticking with it.
While reviewing the C++, I was struggling a bit with which formulas each section is implementing. Can you add some comments in the C++ with short forms of the formulas you're implementing? I tried to use copilot and searching for some of them, but was a bit muddled, and I think adding the formulas will make it easier to make sure things stay in sync.
Not sure where to add it, but the note about how "not supported" means "no easy path to implementation. Which in hindsight, this PR would probably be easier to review if it was broken out by backend - feature comes in w/ agg/cairo, then pdf/pgf, then svg. And I mentioned this in the review comments too, but l think this pr is so large and indepth that the motivating examples of antialiasing and gourard(sp?) triangles should be pushed out into their own prs b/c they require additional context on top of this PR.
Also I think all of this should get a giant provisional flag for now? attn @timhoffm
| There are now alternative options for blending and compositing artists on top | ||
| of previously drawn artists, instead of the normal alpha blending. The |
There was a problem hiding this comment.
| There are now alternative options for blending and compositing artists on top | |
| of previously drawn artists, instead of the normal alpha blending. The | |
| There are now alternative options, to normal alpha blending, for blending and compositing artists on top of previously drawn artists. The |
I dunno that I like my suggestion either, but this sentence feels a bit wonky
| for a in artists: | ||
| if (isinstance(a, _ImageBase) and a.can_composite() and | ||
| a.get_clip_on() and not a.get_clip_path()): | ||
| a.get_clip_on() and not a.get_clip_path() and |
There was a problem hiding this comment.
so any non normal blend mode should trigger a draw?
There was a problem hiding this comment.
Yup. This code path is about collapsing a sequence of images into a single image so that the renderer needs to be called for only one image, which is done with "normal" blending. Essentially it acts like an isolated blend group where the group blend mode is locked to "normal". That means that if any image has a blend mode other than "normal", it cannot be included in a group. So, in that case, we draw the group of the preceding images, and then render that next image separately.
| if(alpha == 0) return; | ||
|
|
||
| // The following code does not accurately transform to higher bit depth | ||
| // TODO: Improve the accuracy of this code when we are prepared to regenerate all baseline images |
There was a problem hiding this comment.
this should probably be converted to an issue when this pr gets merged?
There was a problem hiding this comment.
In principle, yes, but it's an incredibly subtle difference
| axs[i].set_title(blend_mode) | ||
|
|
||
|
|
||
| class ArtistGroup(Artist): |
There was a problem hiding this comment.
I know this seems silly, but can you add a test of your testing helper? that artist group properly groups?
There was a problem hiding this comment.
I've enhanced the blend-group image test so that it checks that the combined output of artists of multiple zorders is rendered at a single zorder
Thanks for reviewing it! =)
I will make this more clear in the comments, but the four non-separable blend modes I've added to Agg are a near-literal implementation of the pseudocode provided in the PDF specification (e.g., pages 326–328 of PDF 1.7). That includes the structure of the code and functions, the naming of variables, and the magic numbers. This exact pseudocode underpins not only the implementations of every PDF renderer, but also any renderer that plans to produce matching output (e.g., Cairo's page about their blending/compositing operators parrots the same pseudocode). As such, I think that it is important to mirror the pseudocode as closely as reasonably possible.
Once this PR is merged, I envision a follow-up post to #6210 that lists every "not supported" case and the reason why. Some enterprising individual in the future may come up with a solution given existing constraints, or perhaps newer versions of specifications (e.g., SVG or PDF) may enable implementation. I don't think any discussion of implementation difficulties should go into user-facing documentation.
I used to have separate commits for each backend, but I merged them into one commit because the changes to each backend were already completely decoupled at the file level.
I've intentionally kept those fixes as separate commits, so they could certainly be split off to subsequent PRs. That said, I am hesitant to do so because that would reduce this PR to be largely of "hypothetical" use, which I fear would mean that reviewers are even less likely to be motivated to look at it.
I don't know what the term "provisional" means for matplotlib, but I certainly would not consider this functionality to be "provisional". Given that the output is as desired in nearly all cases, is the concern that the API might change? |
Yes, provisional is just our flag for "this API might change" and I'll open an issue about how we should add info about it to the dev docs. There's an example in colorizer
I don't think so since you can point to the uses directly. I think right now the lack of motivation is solely b/c of the size and scope of this PR - it's a bit hard to find/keep bearings while doing the review if you don't already have it in your head.
Sounds good to me.
That's fine so long as the sourcing is more explicit in the comments. Copilot and google at no point pulled this up, |
I think it incredibly unlikely that the blend-mode API would change. There might be reason to change the blend-group API, but that also seems unlikely to me. If I were trying to add |
I'm not sure we've ever actually changed our provisional API (subplot_mosaic was provisional for years), it's just a kind of emergency escape/hedge on mostly big changes. |
|
I have moved the fixes for contourf and pcolor/pcolormesh out of this PR to make this PR slightly more wieldy to review. The bugs are less apparent to users because it requires antialiasing to be turned on, which is not the default. I'll PR those fixes after this PR is merged. I have retained the fix to Agg Gouraud shading because the bug is easier to be encountered by users and would be immediately seen in the blend-mode gallery. |
| if blend_mode not in _BLEND_MODES_PDFSPEC + _BLEND_MODES_PORTERDUFF: | ||
| raise ValueError(f"{blend_mode} is not a supported blend mode") |
There was a problem hiding this comment.
This should use _api.check_in_list.
There was a problem hiding this comment.
Since the blend modes are now a StrEnum, it felt cleaner to me to stick with using in rather than _api.check_in_list(), but let me know if you disagree
There was a problem hiding this comment.
The upside of check_in_list is that we get a nice error message that suggests near miss-spelling which makes the error message way nice for the user if they happen to be passing strings rather than the Enum objects.
We have been trying to upgrade every error message we have to do this consistently.
There was a problem hiding this comment.
Ah, indeed, and it works nicely with StrEnum too. I've updated to use check_in_list(), and I realized I hadn't even been checking the input for open_blend_group(), so that's now protected too.
| Start filtering. It simply creates a new canvas (the old one is saved). | ||
| """ | ||
| self._filter_renderers.append(self._renderer) | ||
| self._group_states.append(("filter", self._renderer, None, None)) |
There was a problem hiding this comment.
Maybe it would be useful to make it a namedtuple?
PR summary
This PR adds support for blend modes beyond alpha blending (e.g., "screen" or "hard light"), so closes #6210. With this PR, all artists can specify
blend_mode, and they are supported by Agg-based and Cairo-based backends, and mostly supported by SVG/PDF/PGF backends.Of course,
mplcairoprovides access to these blend modes, but this PR provides blend-mode support without needingcairo.Update: This PR uses this functionality to fix a long-standing bug (e.g., fixes #27016) with Agg rendering of Gouraud shading, where the edges of triangles would become visible when transparency is involved.
Update: This PR adds support for blend groups, which can be isolated, knockout, or both.
✅ = supported, 🟡 = supported through rasterization, ❌ = not supported
color dodge, color burn, hard light, soft light,
difference, exclusion
Agg showcase
Gouraud shading can look wrong with some blend modes, for the same reason it can look wrong under normal blend mode when alpha < 1, due to overlapping trianglesNow fixedCairo showcase
Gouraud shading is apparently not supported by the Cairo backend, so I commented out the pcolormesh callI added support for Gouraud shadingWindows
macOS
SVG showcase
PDF showcase
Figure_1.pdf
PGF showcase
Figure_1.pgf.pdf
Generating code
Put off to future work:
pcolormesh.snapbehaves when the mesh edges are not horizontal/verticalcontourf()/pcolor()/pcolormesh()to beTruePR checklist