From 81aa2233d67539a8fdcba2b8dba96872864f431e Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Fri, 4 Jun 2021 00:37:16 -0400 Subject: [PATCH] Don't disable path clipping on paths with codes. When `PathClipper` is applied, it is disabled if `has_curves()` is true. However, that method simply checks if the Path has _codes_, not curves. This means Paths created with codes, but using only MOVETO/LINETO, are not clipped when they could be. Additionally, the description for `PathClipper` says that "Curve segments are not clipped, but are always included in their entirety.", meaning there is no need to turn it off when the Path contains curves, as they will at least be handled, if not fully clipped. --- lib/matplotlib/tests/test_patheffects.py | 3 ++- lib/matplotlib/tests/test_simplification.py | 23 +++++++++++++++++++++ src/_backend_agg.h | 4 ++-- src/_path.h | 6 +++--- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/matplotlib/tests/test_patheffects.py b/lib/matplotlib/tests/test_patheffects.py index 34462fa0d179..d0f4fabf4b5c 100644 --- a/lib/matplotlib/tests/test_patheffects.py +++ b/lib/matplotlib/tests/test_patheffects.py @@ -136,7 +136,8 @@ def test_collection(): 'edgecolor': 'blue'}) -@image_comparison(['tickedstroke'], remove_text=True, extensions=['png']) +@image_comparison(['tickedstroke'], remove_text=True, extensions=['png'], + tol=0.22) # Increased tolerance due to fixed clipping. def test_tickedstroke(): fig, (ax1, ax2, ax3) = plt.subplots(1, 3, figsize=(12, 4)) path = Path.unit_circle() diff --git a/lib/matplotlib/tests/test_simplification.py b/lib/matplotlib/tests/test_simplification.py index 07287618f26c..952e890ce660 100644 --- a/lib/matplotlib/tests/test_simplification.py +++ b/lib/matplotlib/tests/test_simplification.py @@ -47,6 +47,29 @@ def test_diamond(): ax.set_ylim(-0.6, 0.6) +def test_clipping_out_of_bounds(): + # Should work on a Path *without* codes. + path = Path([(0, 0), (1, 2), (2, 1)]) + simplified = path.cleaned(clip=(10, 10, 20, 20)) + assert_array_equal(simplified.vertices, [(0, 0)]) + assert simplified.codes == [Path.STOP] + + # Should work on a Path *with* codes, and no curves. + path = Path([(0, 0), (1, 2), (2, 1)], + [Path.MOVETO, Path.LINETO, Path.LINETO]) + simplified = path.cleaned(clip=(10, 10, 20, 20)) + assert_array_equal(simplified.vertices, [(0, 0)]) + assert simplified.codes == [Path.STOP] + + # A Path with curves does not do any clipping yet. + path = Path([(0, 0), (1, 2), (2, 3)], + [Path.MOVETO, Path.CURVE3, Path.CURVE3]) + simplified = path.cleaned() + simplified_clipped = path.cleaned(clip=(10, 10, 20, 20)) + assert_array_equal(simplified.vertices, simplified_clipped.vertices) + assert_array_equal(simplified.codes, simplified_clipped.codes) + + def test_noise(): np.random.seed(0) x = np.random.uniform(size=50000) * 50 diff --git a/src/_backend_agg.h b/src/_backend_agg.h index 55795a77894c..d61dd5baa966 100644 --- a/src/_backend_agg.h +++ b/src/_backend_agg.h @@ -470,7 +470,7 @@ RendererAgg::draw_path(GCAgg &gc, PathIterator &path, agg::trans_affine &trans, trans *= agg::trans_affine_scaling(1.0, -1.0); trans *= agg::trans_affine_translation(0.0, (double)height); - bool clip = !face.first && !gc.has_hatchpath() && !path.has_curves(); + bool clip = !face.first && !gc.has_hatchpath(); bool simplify = path.should_simplify() && clip; double snapping_linewidth = points_to_pixels(gc.linewidth); if (gc.color.a == 0.0) { @@ -992,7 +992,7 @@ inline void RendererAgg::_draw_path_collection_generic(GCAgg &gc, } } - bool do_clip = !face.first && !gc.has_hatchpath() && !has_curves; + bool do_clip = !face.first && !gc.has_hatchpath(); if (check_snap) { gc.isaa = antialiaseds(i % Naa); diff --git a/src/_path.h b/src/_path.h index 49d862eafcad..577b90032aab 100644 --- a/src/_path.h +++ b/src/_path.h @@ -985,7 +985,7 @@ void convert_path_to_polygons(PathIterator &path, transformed_path_t tpath(path, trans); nan_removal_t nan_removed(tpath, true, path.has_curves()); - clipped_t clipped(nan_removed, do_clip && !path.has_curves(), width, height); + clipped_t clipped(nan_removed, do_clip, width, height); simplify_t simplified(clipped, simplify, path.simplify_threshold()); curve_t curve(simplified); @@ -1050,7 +1050,7 @@ void cleanup_path(PathIterator &path, transformed_path_t tpath(path, trans); nan_removal_t nan_removed(tpath, remove_nans, path.has_curves()); - clipped_t clipped(nan_removed, do_clip && !path.has_curves(), rect); + clipped_t clipped(nan_removed, do_clip, rect); snapped_t snapped(clipped, snap_mode, path.total_vertices(), stroke_width); simplify_t simplified(snapped, do_simplify, path.simplify_threshold()); @@ -1209,7 +1209,7 @@ bool convert_to_string(PathIterator &path, transformed_path_t tpath(path, trans); nan_removal_t nan_removed(tpath, true, path.has_curves()); - clipped_t clipped(nan_removed, do_clip && !path.has_curves(), clip_rect); + clipped_t clipped(nan_removed, do_clip, clip_rect); simplify_t simplified(clipped, simplify, path.simplify_threshold()); buffersize = path.total_vertices() * (precision + 5) * 4;