SVG: simplify large filled paths e.g. fill_between (#22803)#31796
SVG: simplify large filled paths e.g. fill_between (#22803)#31796evanscastonguay wants to merge 1 commit into
Conversation
|
The PR text looks completely AI generated and does not use our PR template. This does not instill trust any human oversight was put into this. Please fill in all information from the PR template and make sure to comply with our AI policy. |
|
⏰ This pull request might be automatically closed in two weeks from now. Thank you for your contribution to Matplotlib and for the effort you have put into this PR. This pull request does not yet meet the quality and clarity standards needed for an effective review. Project maintainers have limited time for code reviews, and our goal is to prioritize well-prepared contributions to keep Matplotlib maintainable. Matplotlib maintainers cannot provide one-to-one guidance on this PR. However, if you ask focused, well-researched questions, a community member may be willing to help. 💬 To increase the chance of a productive review:
As the author, you are responsible for driving this PR, which entails doing necessary background research as well as presenting its context and your thought process. If you are a new contributor, or do not know how to fulfill these requirements, we recommend that you familiarize yourself with Matplotlib's development conventions or engage with the community via our Discourse or one of our meetings before submitting code. If you substantially improve this PR within two weeks, leave a comment and a team member may remove the |
|
Updated the description: I was able to run the SVG comparisons locally (Inkscape) — 107 pass, 4 baselines shift by small/benign amounts (RMS 0.01–1.6, edge drift). Details and the two resolution options are in the PR body. Happy to push whichever direction you prefer. |
|
You still fail to use the PR template. Contributions not following this will not be considered and closed. |
|
Closing this. I opened it without properly checking the existing work — #31361 already addresses this at the FillBetweenPolyCollection level, which is cleaner than my SVG-backend change, and the discussion there is rightly leaning toward making vector simplification opt-in rather than on-by-default (which mine isn't). I also should have used the PR template and the AI-disclosure section. Apologies for the duplicate and the noise — I'll follow #31361. |
Closes #22803.
The problem
fill_betweenwrites a very large SVG — ~1 MB for a plot whose equivalent line version is ~400 KB.Why (first principles)
A vector file should carry only the geometry needed to render it. matplotlib already strips sub-pixel-redundant points from line plots via path simplification — but the polygon produced by
fill_betweenis written verbatim, because the SVG backend hardcodessimplify=Falsewhere it emits filled / cached paths. So tens of thousands of invisible, redundant vertices end up in the file.The fix
Let the SVG backend simplify large paths — including filled ones — exactly as it already does for strokes: honor
path.should_simplify(plus a vertex-count threshold for closed fills, whichshould_simplifyexcludes), respectrcParams["path.simplify"], and leave hatched paths exact. SVG backend only — agg/pdf rendering is untouched.Proof it fixes the issue
On the issue's reproduction:
fill_betweenSVGTesting
imshow_clip,arc_ellipse,contour_colorbar.test_fill_between_svg_simplifiedasserts the fill path is simplified and stays vector.arc_ellipse(RMS 0.010),test_log_locator_levels(0.476),contour_colorbar(0.642),test_set_line_coll_dash_image(1.616).The one decision for maintainers
Because this is SVG-only, those 4 plots' SVG output now differs slightly from their (agg-based) baselines — the expected consequence of an intended rendering change. Two clean ways to resolve, your call:
I've proposed (a) as the lighter option and can push either. Marked draft pending that decision.