Fix half-filled markers protrusion #19354
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7fb8f86 to
91becf3
Compare
Don't let the miter joins protrude by manually cropping them. This is currently only implemented for diamond, but others should work "similarly". However, this reveals a bug(?) in Agg's handling of miter with close corners, which does not handle that case correctly. (pdf, ps, svg, and cairo all handle it fine). Also don't forget a closepoly on half-filled squares.
|
I think the following is perhaps a better take on this: we "just" separately define and draw the surrounding path, instead of trying to use the paths that define the two halves of the marker, i.e. diff --git i/lib/matplotlib/lines.py w/lib/matplotlib/lines.py
index f843ccd49f..68642cc100 100644
--- i/lib/matplotlib/lines.py
+++ w/lib/matplotlib/lines.py
@@ -817,7 +817,6 @@ class Line2D(Artist):
and not cbook._str_lower_equal(
self.get_markerfacecolor(), "none")):
ec_rgba = ec_rgba[:3] + (fc_rgba[3],)
- gc.set_foreground(ec_rgba, isRGBA=True)
if self.get_sketch_params() is not None:
scale, length, randomness = self.get_sketch_params()
gc.set_sketch_params(scale/2, length/2, 2*randomness)
@@ -854,24 +853,36 @@ class Line2D(Artist):
gc.set_capstyle(marker.get_capstyle())
marker_path = marker.get_path()
marker_trans = marker.get_transform()
+ alt_marker_path = marker.get_alt_path()
+ joint_surround_path = marker._get_joint_surround_path()
w = renderer.points_to_pixels(self._markersize)
- if cbook._str_equal(marker.get_marker(), ","):
- gc.set_linewidth(0)
+ if not alt_marker_path:
+ gc.set_foreground(ec_rgba, isRGBA=True)
+ if cbook._str_equal(marker.get_marker(), ","):
+ # Don't scale for pixels, and don't stroke them
+ gc.set_linewidth(0)
+ else:
+ marker_trans = marker_trans.scale(w)
+ renderer.draw_markers(
+ gc, marker_path, marker_trans, subsampled,
+ affine, fc_rgba)
+
else:
- # Don't scale for pixels, and don't stroke them
+ gc.set_foreground((0, 0, 0, 0), isRGBA=True)
marker_trans = marker_trans.scale(w)
- renderer.draw_markers(gc, marker_path, marker_trans,
- subsampled, affine.frozen(),
- fc_rgba)
-
- alt_marker_path = marker.get_alt_path()
- if alt_marker_path:
- alt_marker_trans = marker.get_alt_transform()
- alt_marker_trans = alt_marker_trans.scale(w)
renderer.draw_markers(
- gc, alt_marker_path, alt_marker_trans, subsampled,
- affine.frozen(), fcalt_rgba)
+ gc, marker_path, marker_trans, subsampled,
+ affine, fc_rgba)
+ alt_marker_trans = marker.get_alt_transform().scale(w)
+ renderer.draw_markers(
+ gc, alt_marker_path, alt_marker_trans, subsampled,
+ affine, fcalt_rgba)
+ if joint_surround_path is not None:
+ gc.set_foreground(ec_rgba, isRGBA=True)
+ renderer.draw_markers(
+ gc, joint_surround_path, marker_trans, subsampled,
+ affine, (0, 0, 0, 0))
gc.restore()
diff --git i/lib/matplotlib/markers.py w/lib/matplotlib/markers.py
index c9fc014193..0636a940a9 100644
--- i/lib/matplotlib/markers.py
+++ w/lib/matplotlib/markers.py
@@ -279,6 +279,7 @@ class MarkerStyle:
return
self._path = _empty_path
self._transform = IdentityTransform()
+ self._joint_surround_path = None
self._alt_path = None
self._alt_transform = None
self._snap_threshold = None
@@ -385,6 +386,9 @@ class MarkerStyle:
else:
return (self._transform + self._user_transform).frozen()
+ def _get_joint_surround_path(self):
+ return self._joint_surround_path
+
def get_alt_path(self):
"""
Return a `.Path` for the alternate part of the marker.
@@ -550,6 +554,12 @@ class MarkerStyle:
self._path = Path.unit_circle()
else:
self._path = self._alt_path = Path.unit_circle_righthalf()
+ p = Path.unit_circle()
+ v = p.vertices.copy()
+ c = p.codes.copy()
+ v[-1] = (0, 1)
+ c[-1] = Path.LINETO
+ self._joint_surround_path = Path(v, c)
fs = self.get_fillstyle()
self._transform.rotate_deg(
{'right': 0, 'top': 90, 'left': 180, 'bottom': 270}[fs])
@@ -574,11 +584,15 @@ class MarkerStyle:
_triangle_path = Path._create_closed([[0, 1], [-1, -1], [1, -1]])
# Going down halfway looks to small. Golden ratio is too far.
- _triangle_path_u = Path._create_closed([[0, 1], [-3/5, -1/5], [3/5, -1/5]])
- _triangle_path_d = Path._create_closed(
+ _triangle_path_t = Path._create_closed([[0, 1], [-3/5, -1/5], [3/5, -1/5]])
+ _triangle_path_b = Path._create_closed(
[[-3/5, -1/5], [3/5, -1/5], [1, -1], [-1, -1]])
+ _triangle_path_tb = Path(
+ [[3/5, -1/5], [0, 1], [-3/5, -1/5], [3/5, -1/5], [1, -1], [-1, -1], [-3/5, -1/5]])
_triangle_path_l = Path._create_closed([[0, 1], [0, -1], [-1, -1]])
_triangle_path_r = Path._create_closed([[0, 1], [0, -1], [1, -1]])
+ _triangle_path_lr = Path(
+ [[0, 1], [0, -1], [-1, -1], [0, 1], [1, -1], [0, -1]])
def _set_triangle(self, rot, skip):
self._transform = Affine2D().scale(0.5).rotate_deg(rot)
@@ -587,24 +601,30 @@ class MarkerStyle:
if not self._half_fill():
self._path = self._triangle_path
else:
- mpaths = [self._triangle_path_u,
+ mpaths = [self._triangle_path_t,
self._triangle_path_l,
- self._triangle_path_d,
+ self._triangle_path_b,
self._triangle_path_r]
+ jpaths = [self._triangle_path_tb,
+ self._triangle_path_lr]
fs = self.get_fillstyle()
if fs == 'top':
self._path = mpaths[(0 + skip) % 4]
self._alt_path = mpaths[(2 + skip) % 4]
+ self._joint_surround_path = jpaths[skip % 2]
elif fs == 'bottom':
self._path = mpaths[(2 + skip) % 4]
self._alt_path = mpaths[(0 + skip) % 4]
+ self._joint_surround_path = jpaths[skip % 2]
elif fs == 'left':
self._path = mpaths[(1 + skip) % 4]
self._alt_path = mpaths[(3 + skip) % 4]
+ self._joint_surround_path = jpaths[(skip + 1) % 2]
else:
self._path = mpaths[(3 + skip) % 4]
self._alt_path = mpaths[(1 + skip) % 4]
+ self._joint_surround_path = jpaths[(skip + 1) % 2]
self._alt_transform = self._transform
@@ -648,6 +668,9 @@ class MarkerStyle:
else:
self._path = Path([[0, 0], [1, 0], [1, 1], [0, 0]])
self._alt_path = Path([[0, 0], [0, 1], [1, 1], [0, 0]])
+ self._joint_surround_path = Path(
+ [[0, 0], [0, 1], [1, 1], [1, 0], [0, 0], [1, 1]],
+ [Path.MOVETO, *[Path.LINETO] * 3, Path.CLOSEPOLY, Path.LINETO])
fs = self.get_fillstyle()
rotate = {'right': 0, 'top': 90, 'left': 180, 'bottom': 270}[fs]
self._transform.rotate_deg(rotate)Here the surrounding path is called _joint_surround_path, probably a better name is possible (and it could possibly be made public). The patch above only defines the surround paths for triangle and diamond paths to show that this approach can indeed fix #14357, but a full patch would need to also define the surround paths for the remaining markers (and then joint_surround_path would be guaranteed to always be non-None whenever alt_path is defined, so we'd get rid of the none-check on joint_surround_path in Line2D.draw). |
|
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
|
Probably worth reviving in some form, but the basic issue is already tracked at #14357 so let's just close this for now, we can always reopen if anyone is interesting in picking up the patches. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Don't let the miter joins protrude by manually cropping them (#14357). This is
currently only implemented for diamond, but others should work
"similarly". However, this reveals a bug(?) in Agg's handling of
miter with close corners, which does not handle that case correctly.
(pdf, ps, svg, and cairo all handle it fine).
Also don't forget a closepoly on half-filled squares.
Goes on top of #19357.
PR Summary
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).