FIX: keep sub-pixel segments visible when snapping (#20243)#31981
FIX: keep sub-pixel segments visible when snapping (#20243)#31981Zish19 wants to merge 2 commits into
Conversation
|
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process. You can also join us on discourse chat for real-time discussion. For details on testing, writing docs, and our review process, please see the developer guide. We strive to be a welcoming and open project. Please follow our Code of Conduct. |
343a057 to
0911d67
Compare
PathSnapper rounds each vertex to the nearest pixel. A line mark shorter than one pixel then has both endpoints rounded onto the same pixel, so it collapses to zero length and is not drawn at all. eventplot builds every event as its own two-point segment, so plotting many closely spaced events silently dropped most of them. When the opening LINETO of a subpath snaps onto its MOVETO even though the raw segment was non-degenerate, extend the endpoint by one pixel along the segment's dominant axis so a single pixel is still drawn. Limiting this to the first segment of a subpath leaves polygon outlines (bars, markers, grids) snapping exactly as before.
0911d67 to
eec0fee
Compare
Update PathSnapper to look ahead one vertex. This ensures that only isolated zero-length segments (like those generated by eventplot) are extended to 1 pixel. This fixes unintended rendering distortion in polygons from streamplot, hist_step_filled, etc.
|
While investigating the regressions from this renderer-based approach, I instrumented One observation stood out: by the time geometry reaches I also experimented with an alternative implementation outside the renderer by reverting the Before I rewrite this PR around that approach, I'd appreciate some guidance on the preferred design direction. Would maintainers prefer continuing with a renderer-level solution in I'm happy to continue in whichever direction is preferred. |
PR summary
Closes #20243
PathSnapper rounds every vertex to the nearest pixel. When a line segment is shorter than a pixel, both endpoints
round to the same pixel, so the segment collapses to zero length and the Agg renderer draws nothing. With eventplots
of many events, this silently dropped most of the data, see #20243.
This change tracks the previous raw/snapped vertex in PathSnapper. When a LINETO snaps onto the previous point even
though the raw segment had non-zero length, the snapped endpoint is nudged by one pixel along the segment's dominant
direction. The result is at least one full-color pixel rather than blurring the mark by disabling snapping (per the
discussion in #20243, this matches the preference for a full-color pixel over anti-aliased fuzz). Segments that
already span a pixel are untouched, so normal rendering (lines, bars, grids) is byte-for-byte unchanged.
Minimum self-contained example
import matplotlib.pyplot as plt
import numpy as np
data = [np.random.random(np.random.randint(10)) for _ in range(500)]
fig, ax = plt.subplots()
ax.eventplot(data, linelength=1)
plt.show()
Before: most rows are missing.
After: every event renders as at least one full pixel.
Verification: A regression test (test_eventplot_sub_pixel_events_not_dropped) renders 200 isolated sub-pixel event
marks and asserts they are all drawn. On the unpatched renderer 0/200 marks appear; with this change all of them do.
A separate check confirms the pixel buffers of ordinary line plots, bar charts, grids, and normal-scale eventplots
are identical before and after the change, so there is no visual regression on content that already covers a pixel.
PR checklist
(https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
(https://matplotlib.org/devdocs/devel/document.html#write-examples-and-tutorials)
(https://matplotlib.org/devdocs/devel/api_changes.html#announce-changes-deprecations-and-new-features)
docstring (https://matplotlib.org/devdocs/devel/document.html#write-docstrings) guidelines