iframe-proxy

QuLogic · 2026-07-23T19:03:31Z

PR summary

We cache the marker for performance, but this causes it to look very pixel-grid aligned, especially if there are multiple in a regular pattern, or they are animated.

Apply the same algorithm as mplcairo noted by @anntzer in #7233, specifically, render and cache the marker in n_subdiv levels in each direction, and pick the closest one for each marker.

Fixes #7233

AI Disclosure

None

PR checklist

"closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
Plotting related features are demonstrated in an example
New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

QuLogic · 2026-07-23T19:04:15Z

+    // TODO: Pixel markers *must* be drawn snapped.
+    auto const& is_pixel_marker = false;
+        // py_eq(marker_path, PIXEL_MARKER.attr("get_path")())
+        // && marker_trans == PIXEL_MARKER.attr("get_transform")().cast<agg::trans_affine>();


mplcairo special-cases the pixel marker; I have not checked how that turns out or whether that is necessary.

IIRC the idea is that not snapping the pixel marker results in it being smeared across two pixels, whereas it would otherwise look sharp (exactly one pixel filled) on a raster output.

Yes, okay, this does look pretty bad right now:

QuLogic · 2026-07-23T19:04:53Z

+                auto marker_path_snapped = PathSnapper{
+                    marker_path_nan_removed,
+                    gc.snap_mode, marker_path.total_vertices(),
+                    points_to_pixels(gc.linewidth)};


I'm not 100% certain we should be snapping here for each of the cached subpixel markers. It affects a lot more test images if we drop this though.

From a quick look mplcairo disables snapping when generating subpixel markers, which still makes sense to me? (Actually, if you snap, I don't see how you can benefit from subpixel positioning.)

Maybe the issue with test images is with ticks, which are effectively markers and should indeed be drawn snapped? Can that be fixed by explicitly forcing snapping on them? (I didn't re-check snap control semantics, but maybe(?) it makes sense that if snapping is requested on the edges of a Line2D, then it is also enabled on the markers at its vertices?)

QuLogic · 2026-07-23T19:23:06Z

The original issue with current style looks like this:

and now looks smoother:

and the animation is smoother:

tacaswell · 2026-08-21T02:48:48Z

What is between us and marking this ready for review?

timhoffm · 2026-08-21T04:09:32Z

This certainly works well for circles, which do not have special directional edges. We should check how this looks for squares: Do subpixel interpolated edges give the impression of “in between position” or just “blurry”?
Same question for line-like markers like “x” and “+”.
Maybe whether to interpolate and the number of interpolation steps needs to be chosen depending on the marker type.

QuLogic · 2026-08-21T18:13:10Z

What is between us and marking this ready for review?

The main issue is this one. It looks like if we disable snapping on the cached markers, we end up with ~450 test failures, though they are mostly due to ticks being 1 pixel shorter.

timhoffm · 2026-08-21T20:30:52Z

Can we special-case TICKDOWN / TICKUP markers so that they keep their old behavior?

IMHO mid-term, the implementation of ticks via Line2D with specific markers should become an implementation detail. But tick1line and similar are currently exposed, so that we don't want to wait for the necessary changes there to add subpixel snapping.

anntzer · 2026-08-22T10:36:18Z

In mplcairo ticks are not drawn with stamping at all but by drawing them one at a time, which IIRC is indeed needed to make them sharp and not blurry. Actually this is not an explicit switch but just the consequence of other choices: the size of the pattern cache (which is pre-populated prior to sampling) is normally between 9**2 (where 9 is the inverse of the default path.simplify_threshold) and an arbitrary upper bound of of 16**2 (i.e. between 9 and 16 subpixels in both x and y), and if the total number of markers to be drawn is less than the size of the cache then I don't use the cache at all (as we don't expect it to help).
So for a single-marker draw (e.g. a tick) we just bypass the cache.
(Maybe if the design was to lazily populate the cache as entries are used then I would have added an explicit check instead to bypass the cache for single-marker draws.)

@anntzer

We cache the marker for performance, but this causes it to look very pixel-grid aligned, especially if there are multiple in a regular pattern, or they are animated. Apply the same algorithm as mplcairo noted by @anntzer in matplotlib#7233, specifically, render and cache the marker in *n_subdiv* levels in each direction, and pick the closest one for each marker. Fixes matplotlib#7233

QuLogic · 2026-08-25T06:07:04Z

I've rebased and added the image changes for people to look at. I don't quite think we should keep them all, but I haven't looked into any additional special-casing besides the pixel marker (which needs to be optimized, probably.)

There also seems to be some check_figures_equal tests that fail, which I'll need to investigate further.

and if the total number of markers to be drawn is less than the size of the cache then I don't use the cache at all (as we don't expect it to help).
So for a single-marker draw (e.g. a tick) we just bypass the cache.

Ah, I don't think I noticed that and didn't implement it. The downside of doing that is it then depends on how we draw ticks, and there is of course some hope that we can refactor enough to draw them as a collection in the future.

(Maybe if the design was to lazily populate the cache as entries are used then I would have added an explicit check instead to bypass the cache for single-marker draws.)

I thought of this one originally, but I'm not sure how easy it is, since the Agg rasterizer is re-used for both marker cache and the actual drawing, so we'd have to do some careful tracking of state if we switch between them.

QuLogic · 2026-08-25T08:10:24Z

We should check how this looks for squares: Do subpixel interpolated edges give the impression of “in between position” or just “blurry”?
Same question for line-like markers like “x” and “+”.

Here is how it looks for those before:

and with this PR:

skewedgridqtagg.mp4

You may have to click through to view full size.

QuLogic · 2026-08-25T17:35:03Z

Looking at the test failure in lib/matplotlib/tests/test_axes.py::test_stairs_fill, it appears that this PR negatively affects the single path-as-draw_marker optimization, since it's no longer snapped. I wonder why this doesn't cause the same problem on mplcairo.

QuLogic · 2026-08-25T19:28:12Z

Comparing to mplcairo, both test/ref images that it creates match the new result from this PR (minus the text):

anntzer · 2026-08-26T20:18:37Z

The antialiasing looks really bad with squares in the animation, how does mplcairo do with it?

QuLogic · 2026-08-27T05:54:03Z

I see some moire for the squares, but I think that's from scaling down to fit the GitHub comment width. I'm not sure it's so bad at full size, but here is what mplcairo looks like:

anntzer · 2026-08-27T09:35:00Z

Ah, indeed the moire goes away when the video is blown up. But it's still a bit weird, the moire doesn't show up even downscaled with mplcairo?

timhoffm · 2026-08-27T09:57:34Z

github-actions Bot added the backend: agg label Jul 23, 2026

QuLogic commented Jul 23, 2026

View reviewed changes

QuLogic mentioned this pull request Aug 20, 2026

Fix Collection.draw() single-path optimization when edgecolor/facecolor is 'none' #32232

Draft

2 tasks

tacaswell added this to the v3.12.0 milestone Aug 21, 2026

QuLogic added 3 commits August 24, 2026 21:09

Agg: De-duplicate loop for marker rendering

05dd034

Don't snap internal cached markers

1599575

QuLogic force-pushed the marker-snap branch from f98d6c2 to 1599575 Compare August 25, 2026 05:39

github-actions Bot added the topic: mplot3d label Aug 25, 2026

github-actions Bot added the status: needs rebase label Aug 27, 2026

anntzer mentioned this pull request Sep 3, 2026

[MNT]: Deprecate internal Cairo backends #32276

Open

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Uh oh!

Conversation

QuLogic commented Jul 23, 2026

PR summary

AI Disclosure

PR checklist

Uh oh!

QuLogic Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

anntzer Jul 26, 2026

Choose a reason for hiding this comment

Uh oh!

QuLogic Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

QuLogic Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

anntzer Jul 26, 2026

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Jul 23, 2026

Uh oh!

tacaswell commented Aug 21, 2026

Uh oh!

timhoffm commented Aug 21, 2026

Uh oh!

QuLogic commented Aug 21, 2026

Uh oh!

timhoffm commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anntzer commented Aug 22, 2026

Uh oh!

QuLogic commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QuLogic commented Aug 25, 2026

Uh oh!

QuLogic commented Aug 25, 2026

Uh oh!

QuLogic commented Aug 25, 2026

Uh oh!

anntzer commented Aug 26, 2026

Uh oh!

QuLogic commented Aug 27, 2026

Uh oh!

anntzer commented Aug 27, 2026

Uh oh!

timhoffm commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

timhoffm commented Aug 21, 2026 •

edited

Loading

QuLogic commented Aug 25, 2026 •

edited

Loading