Skip to content

Fix a bug of ignoring the closing path segment when automatically determining whether to snap#32018

Open
ayshih wants to merge 2 commits into
matplotlib:mainfrom
ayshih:snap_path_closure
Open

Fix a bug of ignoring the closing path segment when automatically determining whether to snap#32018
ayshih wants to merge 2 commits into
matplotlib:mainfrom
ayshih:snap_path_closure

Conversation

@ayshih

@ayshih ayshih commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR summary

The current code for automatic determination of whether a path should be snapped – by checking whether all line segments are either horizontal or vertical – neglects to check the closing line segment for a closed path. That is, a closed path will be automatically determined to be snappable even when the closing line segment is not horizontal or vertical as long as all other line segments are either horizontal or vertical. This PR fixes that oversight.

A not-uncommon place for this bug to have an effect in the wild is when plotting contours, and one contour region is just a triangular section at the corner of the plot. If the diagonal of that triangle happens to be the closing line segment, then that contour region will snap, while its adjacent contour region will typically not snap. A handful of figure tests show this effect.

Here is a minimal example illustrating the bug:

import matplotlib.pyplot as plt
from matplotlib.patches import Polygon

# diagonal is not the closing segment, so will not be snapped
triangle1 = Polygon([[0, 0], [1, 0], [0, 1]])

# diagonal is the closing segment, so will be snapped due to the bug
triangle2 = Polygon([[0, 1], [1, 1], [1, 0]])

fig, ax = plt.subplots()

ax.add_patch(triangle1)
ax.add_patch(triangle2)

plt.show()

Before this PR

The gap between the two triangles is not a consistent width, showing that the two triangles are not snapping the same. The gap will change/flicker if the plot window is resized due to this difference in snapping.
before

After this PR

The gap between the two triangles is a consistent width, showing that both triangles are snapping the same.
after

AI Disclosure

No AI was used

PR checklist

@ayshih ayshih force-pushed the snap_path_closure branch 3 times, most recently from 8e7924c to 5a00b40 Compare July 8, 2026 13:53
@ayshih ayshih changed the title Fix a bug that ignored the closing path segment when automatically determining whether to snap Fix a bug of ignoring the closing path segment when automatically determining whether to snap Jul 8, 2026
@ayshih ayshih marked this pull request as ready for review July 8, 2026 14:18
@ayshih ayshih force-pushed the snap_path_closure branch 2 times, most recently from 4848ceb to e5d637f Compare July 8, 2026 15:49

@QuLogic QuLogic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you run oxipng on the new images? If I've read GitHub's annotations correctly, some of them have grown quite a bit.

Comment thread src/path_converters.h
@ayshih ayshih force-pushed the snap_path_closure branch from e5d637f to 23c0be7 Compare July 8, 2026 19:27
@ayshih ayshih force-pushed the snap_path_closure branch from 23c0be7 to acfeb50 Compare July 8, 2026 19:28
@ayshih

ayshih commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Can you run oxipng on the new images? If I've read GitHub's annotations correctly, some of them have grown quite a bit.

Ah, good catch! Differences are now much more reasonable after using oxipng.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants