Skip to content

Commit 7eaf90b

Browse files
jklymakjklymak
authored
Merge pull request matplotlib#21387 from QuLogic/clipped-closed-path
Fix path simplification of closed loops
2 parents 8daad33 + 8adf493 commit 7eaf90b

File tree

7 files changed

+316
-108
lines changed

7 files changed

+316
-108
lines changed

‎lib/matplotlib/tests/test_simplification.py‎

Lines changed: 152 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66

77
import pytest
88

9-
from matplotlib.testing.decorators import image_comparison
9+
from matplotlib.testing.decorators import (
10+
check_figures_equal, image_comparison, remove_ticks_and_titles)
1011
import matplotlib.pyplot as plt
1112

1213
from matplotlib import patches, transforms
@@ -230,7 +231,7 @@ def test_sine_plus_noise():
230231
assert simplified.vertices.size == 25240
231232

232233

233-
@image_comparison(['simplify_curve'], remove_text=True)
234+
@image_comparison(['simplify_curve'], remove_text=True, tol=0.017)
234235
def test_simplify_curve():
235236
pp1 = patches.PathPatch(
236237
Path([(0, 0), (1, 0), (1, 1), (np.nan, 1), (0, 0), (2, 0), (2, 2),
@@ -245,6 +246,155 @@ def test_simplify_curve():
245246
ax.set_ylim((0, 2))
246247

247248

249+
@check_figures_equal()
250+
def test_closed_path_nan_removal(fig_test, fig_ref):
251+
ax_test = fig_test.subplots(2, 2).flatten()
252+
ax_ref = fig_ref.subplots(2, 2).flatten()
253+
254+
# NaN on the first point also removes the last point, because it's closed.
255+
path = Path(
256+
[[-3, np.nan], [3, -3], [3, 3], [-3, 3], [-3, -3]],
257+
[Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.CLOSEPOLY])
258+
ax_test[0].add_patch(patches.PathPatch(path, facecolor='none'))
259+
path = Path(
260+
[[-3, np.nan], [3, -3], [3, 3], [-3, 3], [-3, np.nan]],
261+
[Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.LINETO])
262+
ax_ref[0].add_patch(patches.PathPatch(path, facecolor='none'))
263+
264+
# NaN on second-last point should not re-close.
265+
path = Path(
266+
[[-2, -2], [2, -2], [2, 2], [-2, np.nan], [-2, -2]],
267+
[Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.CLOSEPOLY])
268+
ax_test[0].add_patch(patches.PathPatch(path, facecolor='none'))
269+
path = Path(
270+
[[-2, -2], [2, -2], [2, 2], [-2, np.nan], [-2, -2]],
271+
[Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.LINETO])
272+
ax_ref[0].add_patch(patches.PathPatch(path, facecolor='none'))
273+
274+
# Test multiple loops in a single path (with same paths as above).
275+
path = Path(
276+
[[-3, np.nan], [3, -3], [3, 3], [-3, 3], [-3, -3],
277+
[-2, -2], [2, -2], [2, 2], [-2, np.nan], [-2, -2]],
278+
[Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.CLOSEPOLY,
279+
Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.CLOSEPOLY])
280+
ax_test[1].add_patch(patches.PathPatch(path, facecolor='none'))
281+
path = Path(
282+
[[-3, np.nan], [3, -3], [3, 3], [-3, 3], [-3, np.nan],
283+
[-2, -2], [2, -2], [2, 2], [-2, np.nan], [-2, -2]],
284+
[Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.LINETO,
285+
Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.LINETO])
286+
ax_ref[1].add_patch(patches.PathPatch(path, facecolor='none'))
287+
288+
# NaN in first point of CURVE3 should not re-close, and hide entire curve.
289+
path = Path(
290+
[[-1, -1], [1, -1], [1, np.nan], [0, 1], [-1, 1], [-1, -1]],
291+
[Path.MOVETO, Path.LINETO, Path.CURVE3, Path.CURVE3, Path.LINETO,
292+
Path.CLOSEPOLY])
293+
ax_test[2].add_patch(patches.PathPatch(path, facecolor='none'))
294+
path = Path(
295+
[[-1, -1], [1, -1], [1, np.nan], [0, 1], [-1, 1], [-1, -1]],
296+
[Path.MOVETO, Path.LINETO, Path.CURVE3, Path.CURVE3, Path.LINETO,
297+
Path.CLOSEPOLY])
298+
ax_ref[2].add_patch(patches.PathPatch(path, facecolor='none'))
299+
300+
# NaN in second point of CURVE3 should not re-close, and hide entire curve
301+
# plus next line segment.
302+
path = Path(
303+
[[-3, -3], [3, -3], [3, 0], [0, np.nan], [-3, 3], [-3, -3]],
304+
[Path.MOVETO, Path.LINETO, Path.CURVE3, Path.CURVE3, Path.LINETO,
305+
Path.LINETO])
306+
ax_test[2].add_patch(patches.PathPatch(path, facecolor='none'))
307+
path = Path(
308+
[[-3, -3], [3, -3], [3, 0], [0, np.nan], [-3, 3], [-3, -3]],
309+
[Path.MOVETO, Path.LINETO, Path.CURVE3, Path.CURVE3, Path.LINETO,
310+
Path.LINETO])
311+
ax_ref[2].add_patch(patches.PathPatch(path, facecolor='none'))
312+
313+
# NaN in first point of CURVE4 should not re-close, and hide entire curve.
314+
path = Path(
315+
[[-1, -1], [1, -1], [1, np.nan], [0, 0], [0, 1], [-1, 1], [-1, -1]],
316+
[Path.MOVETO, Path.LINETO, Path.CURVE4, Path.CURVE4, Path.CURVE4,
317+
Path.LINETO, Path.CLOSEPOLY])
318+
ax_test[3].add_patch(patches.PathPatch(path, facecolor='none'))
319+
path = Path(
320+
[[-1, -1], [1, -1], [1, np.nan], [0, 0], [0, 1], [-1, 1], [-1, -1]],
321+
[Path.MOVETO, Path.LINETO, Path.CURVE4, Path.CURVE4, Path.CURVE4,
322+
Path.LINETO, Path.CLOSEPOLY])
323+
ax_ref[3].add_patch(patches.PathPatch(path, facecolor='none'))
324+
325+
# NaN in second point of CURVE4 should not re-close, and hide entire curve.
326+
path = Path(
327+
[[-2, -2], [2, -2], [2, 0], [0, np.nan], [0, 2], [-2, 2], [-2, -2]],
328+
[Path.MOVETO, Path.LINETO, Path.CURVE4, Path.CURVE4, Path.CURVE4,
329+
Path.LINETO, Path.LINETO])
330+
ax_test[3].add_patch(patches.PathPatch(path, facecolor='none'))
331+
path = Path(
332+
[[-2, -2], [2, -2], [2, 0], [0, np.nan], [0, 2], [-2, 2], [-2, -2]],
333+
[Path.MOVETO, Path.LINETO, Path.CURVE4, Path.CURVE4, Path.CURVE4,
334+
Path.LINETO, Path.LINETO])
335+
ax_ref[3].add_patch(patches.PathPatch(path, facecolor='none'))
336+
337+
# NaN in third point of CURVE4 should not re-close, and hide entire curve
338+
# plus next line segment.
339+
path = Path(
340+
[[-3, -3], [3, -3], [3, 0], [0, 0], [0, np.nan], [-3, 3], [-3, -3]],
341+
[Path.MOVETO, Path.LINETO, Path.CURVE4, Path.CURVE4, Path.CURVE4,
342+
Path.LINETO, Path.LINETO])
343+
ax_test[3].add_patch(patches.PathPatch(path, facecolor='none'))
344+
path = Path(
345+
[[-3, -3], [3, -3], [3, 0], [0, 0], [0, np.nan], [-3, 3], [-3, -3]],
346+
[Path.MOVETO, Path.LINETO, Path.CURVE4, Path.CURVE4, Path.CURVE4,
347+
Path.LINETO, Path.LINETO])
348+
ax_ref[3].add_patch(patches.PathPatch(path, facecolor='none'))
349+
350+
# Keep everything clean.
351+
for ax in [*ax_test.flat, *ax_ref.flat]:
352+
ax.set(xlim=(-3.5, 3.5), ylim=(-3.5, 3.5))
353+
remove_ticks_and_titles(fig_test)
354+
remove_ticks_and_titles(fig_ref)
355+
356+
357+
@check_figures_equal()
358+
def test_closed_path_clipping(fig_test, fig_ref):
359+
vertices = []
360+
for roll in range(8):
361+
offset = 0.1 * roll + 0.1
362+
363+
# A U-like pattern.
364+
pattern = [
365+
[-0.5, 1.5], [-0.5, -0.5], [1.5, -0.5], [1.5, 1.5], # Outer square
366+
# With a notch in the top.
367+
[1 - offset / 2, 1.5], [1 - offset / 2, offset],
368+
[offset / 2, offset], [offset / 2, 1.5],
369+
]
370+
371+
# Place the initial/final point anywhere in/out of the clipping area.
372+
pattern = np.roll(pattern, roll, axis=0)
373+
pattern = np.concatenate((pattern, pattern[:1, :]))
374+
375+
vertices.append(pattern)
376+
377+
# Multiple subpaths are used here to ensure they aren't broken by closed
378+
# loop clipping.
379+
codes = np.full(len(vertices[0]), Path.LINETO)
380+
codes[0] = Path.MOVETO
381+
codes[-1] = Path.CLOSEPOLY
382+
codes = np.tile(codes, len(vertices))
383+
vertices = np.concatenate(vertices)
384+
385+
fig_test.set_size_inches((5, 5))
386+
path = Path(vertices, codes)
387+
fig_test.add_artist(patches.PathPatch(path, facecolor='none'))
388+
389+
# For reference, we draw the same thing, but unclosed by using a line to
390+
# the last point only.
391+
fig_ref.set_size_inches((5, 5))
392+
codes = codes.copy()
393+
codes[codes == Path.CLOSEPOLY] = Path.LINETO
394+
path = Path(vertices, codes)
395+
fig_ref.add_artist(patches.PathPatch(path, facecolor='none'))
396+
397+
248398
@image_comparison(['hatch_simplify'], remove_text=True)
249399
def test_hatch():
250400
fig, ax = plt.subplots()

‎lib/matplotlib/tests/test_transforms.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ def test_clipping_of_log():
201201
clip=(0, 0, 100, 100),
202202
simplify=False)
203203
tpoints, tcodes = zip(*result)
204-
assert_allclose(tcodes, path.codes)
204+
assert_allclose(tcodes, path.codes[:-1]) # No longer closed.
205205

206206

207207
class NonAffineForTest(mtransforms.Transform):

‎src/_backend_agg.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,10 @@ bool RendererAgg::render_clippath(py::PathIterator &clippath,
152152

153153
rendererBaseAlphaMask.clear(agg::gray8(0, 0));
154154
transformed_path_t transformed_clippath(clippath, trans);
155-
nan_removed_t nan_removed_clippath(transformed_clippath, true, clippath.has_curves());
155+
nan_removed_t nan_removed_clippath(transformed_clippath, true, clippath.has_codes());
156156
snapped_t snapped_clippath(nan_removed_clippath, snap_mode, clippath.total_vertices(), 0.0);
157157
simplify_t simplified_clippath(snapped_clippath,
158-
clippath.should_simplify() && !clippath.has_curves(),
158+
clippath.should_simplify() && !clippath.has_codes(),
159159
clippath.simplify_threshold());
160160
curve_t curved_clippath(simplified_clippath);
161161
theRasterizer.add_path(curved_clippath);

‎src/_backend_agg.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ class RendererAgg
277277
DashesVector &linestyles,
278278
AntialiasedArray &antialiaseds,
279279
bool check_snap,
280-
bool has_curves);
280+
bool has_codes);
281281

282282
template <class PointArray, class ColorArray>
283283
void _draw_gouraud_triangle(PointArray &points,
@@ -478,7 +478,7 @@ RendererAgg::draw_path(GCAgg &gc, PathIterator &path, agg::trans_affine &trans,
478478
}
479479

480480
transformed_path_t tpath(path, trans);
481-
nan_removed_t nan_removed(tpath, true, path.has_curves());
481+
nan_removed_t nan_removed(tpath, true, path.has_codes());
482482
clipped_t clipped(nan_removed, clip, width, height);
483483
snapped_t snapped(clipped, gc.snap_mode, path.total_vertices(), snapping_linewidth);
484484
simplify_t simplified(snapped, simplify, path.simplify_threshold());
@@ -512,7 +512,7 @@ inline void RendererAgg::draw_markers(GCAgg &gc,
512512
trans *= agg::trans_affine_translation(0.5, (double)height + 0.5);
513513

514514
transformed_path_t marker_path_transformed(marker_path, marker_trans);
515-
nan_removed_t marker_path_nan_removed(marker_path_transformed, true, marker_path.has_curves());
515+
nan_removed_t marker_path_nan_removed(marker_path_transformed, true, marker_path.has_codes());
516516
snap_t marker_path_snapped(marker_path_nan_removed,
517517
gc.snap_mode,
518518
marker_path.total_vertices(),
@@ -910,7 +910,7 @@ inline void RendererAgg::_draw_path_collection_generic(GCAgg &gc,
910910
DashesVector &linestyles,
911911
AntialiasedArray &antialiaseds,
912912
bool check_snap,
913-
bool has_curves)
913+
bool has_codes)
914914
{
915915
typedef agg::conv_transform<typename PathGenerator::path_iterator> transformed_path_t;
916916
typedef PathNanRemover<transformed_path_t> nan_removed_t;
@@ -998,11 +998,11 @@ inline void RendererAgg::_draw_path_collection_generic(GCAgg &gc,
998998
gc.isaa = antialiaseds(i % Naa);
999999

10001000
transformed_path_t tpath(path, trans);
1001-
nan_removed_t nan_removed(tpath, true, has_curves);
1001+
nan_removed_t nan_removed(tpath, true, has_codes);
10021002
clipped_t clipped(nan_removed, do_clip, width, height);
10031003
snapped_t snapped(
10041004
clipped, gc.snap_mode, path.total_vertices(), points_to_pixels(gc.linewidth));
1005-
if (has_curves) {
1005+
if (has_codes) {
10061006
snapped_curve_t curve(snapped);
10071007
_draw_path(curve, has_clippath, face, gc);
10081008
} else {
@@ -1012,9 +1012,9 @@ inline void RendererAgg::_draw_path_collection_generic(GCAgg &gc,
10121012
gc.isaa = antialiaseds(i % Naa);
10131013

10141014
transformed_path_t tpath(path, trans);
1015-
nan_removed_t nan_removed(tpath, true, has_curves);
1015+
nan_removed_t nan_removed(tpath, true, has_codes);
10161016
clipped_t clipped(nan_removed, do_clip, width, height);
1017-
if (has_curves) {
1017+
if (has_codes) {
10181018
curve_t curve(clipped);
10191019
_draw_path(curve, has_clippath, face, gc);
10201020
} else {

‎src/_path.h‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ inline void points_in_path(PointArray &points,
259259
}
260260

261261
transformed_path_t trans_path(path, trans);
262-
no_nans_t no_nans_path(trans_path, true, path.has_curves());
262+
no_nans_t no_nans_path(trans_path, true, path.has_codes());
263263
curve_t curved_path(no_nans_path);
264264
if (r != 0.0) {
265265
contour_t contoured_path(curved_path);
@@ -305,7 +305,7 @@ void points_on_path(PointArray &points,
305305
}
306306

307307
transformed_path_t trans_path(path, trans);
308-
no_nans_t nan_removed_path(trans_path, true, path.has_curves());
308+
no_nans_t nan_removed_path(trans_path, true, path.has_codes());
309309
curve_t curved_path(nan_removed_path);
310310
stroke_t stroked_path(curved_path);
311311
stroked_path.width(r * 2.0);
@@ -378,7 +378,7 @@ void update_path_extents(PathIterator &path, agg::trans_affine &trans, extent_li
378378
unsigned code;
379379

380380
transformed_path_t tpath(path, trans);
381-
nan_removed_t nan_removed(tpath, true, path.has_curves());
381+
nan_removed_t nan_removed(tpath, true, path.has_codes());
382382

383383
nan_removed.rewind(0);
384384

@@ -512,7 +512,7 @@ bool path_in_path(PathIterator1 &a,
512512
}
513513

514514
transformed_path_t b_path_trans(b, btrans);
515-
no_nans_t b_no_nans(b_path_trans, true, b.has_curves());
515+
no_nans_t b_no_nans(b_path_trans, true, b.has_codes());
516516
curve_t b_curved(b_no_nans);
517517

518518
double x, y;
@@ -884,8 +884,8 @@ bool path_intersects_path(PathIterator1 &p1, PathIterator2 &p2)
884884
return false;
885885
}
886886

887-
no_nans_t n1(p1, true, p1.has_curves());
888-
no_nans_t n2(p2, true, p2.has_curves());
887+
no_nans_t n1(p1, true, p1.has_codes());
888+
no_nans_t n2(p2, true, p2.has_codes());
889889

890890
curve_t c1(n1);
891891
curve_t c2(n2);
@@ -949,7 +949,7 @@ bool path_intersects_rectangle(PathIterator &path,
949949
return false;
950950
}
951951

952-
no_nans_t no_nans(path, true, path.has_curves());
952+
no_nans_t no_nans(path, true, path.has_codes());
953953
curve_t curve(no_nans);
954954

955955
double cx = (rect_x1 + rect_x2) * 0.5, cy = (rect_y1 + rect_y2) * 0.5;
@@ -998,7 +998,7 @@ void convert_path_to_polygons(PathIterator &path,
998998
bool simplify = path.should_simplify();
999999

10001000
transformed_path_t tpath(path, trans);
1001-
nan_removal_t nan_removed(tpath, true, path.has_curves());
1001+
nan_removal_t nan_removed(tpath, true, path.has_codes());
10021002
clipped_t clipped(nan_removed, do_clip, width, height);
10031003
simplify_t simplified(clipped, simplify, path.simplify_threshold());
10041004
curve_t curve(simplified);
@@ -1063,7 +1063,7 @@ void cleanup_path(PathIterator &path,
10631063
typedef Sketch<curve_t> sketch_t;
10641064

10651065
transformed_path_t tpath(path, trans);
1066-
nan_removal_t nan_removed(tpath, remove_nans, path.has_curves());
1066+
nan_removal_t nan_removed(tpath, remove_nans, path.has_codes());
10671067
clipped_t clipped(nan_removed, do_clip, rect);
10681068
snapped_t snapped(clipped, snap_mode, path.total_vertices(), stroke_width);
10691069
simplify_t simplified(snapped, do_simplify, path.simplify_threshold());
@@ -1222,7 +1222,7 @@ bool convert_to_string(PathIterator &path,
12221222
bool do_clip = (clip_rect.x1 < clip_rect.x2 && clip_rect.y1 < clip_rect.y2);
12231223

12241224
transformed_path_t tpath(path, trans);
1225-
nan_removal_t nan_removed(tpath, true, path.has_curves());
1225+
nan_removal_t nan_removed(tpath, true, path.has_codes());
12261226
clipped_t clipped(nan_removed, do_clip, clip_rect);
12271227
simplify_t simplified(clipped, simplify, path.simplify_threshold());
12281228

0 commit comments

Comments
 (0)