8000 Close paths only if they haven't been clipped · matplotlib/matplotlib@8adf493 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8adf493

Browse files
QuLogicQuLogic
committed
Close paths only if they haven't been clipped
Otherwise, a random spurious line connecting the last clipped point with the initial point is drawn unintentionally. Fixes #21300
1 parent 483c01c commit 8adf493

File tree

3 files changed

+73
-9
lines changed

3 files changed

+73
-9
lines changed

‎lib/matplotlib/tests/test_simplification.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,47 @@ def test_closed_path_nan_removal(fig_test, fig_ref):
354354
remove_ticks_and_titles(fig_ref)
355355

356356

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+
357398
@image_comparison(['hatch_simplify'], remove_text=True)
358399
def test_hatch():
359400
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/path_converters.h

Lines changed: 31 additions & 8 deletions
< 7D35 td data-grid-cell-id="diff-36d69dfa667beb67da44e7bc61c9e0ca35deea173c1cb43e77637c7cbcb4a939-425-446-0" data-selected="false" role="gridcell" style="background-color:var(--diffBlob-additionNum-bgColor, var(--diffBlob-addition-bgColor-num));text-align:center" tabindex="-1" valign="top" class="focusable-grid-cell diff-line-number position-relative left-side">
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ class PathClipper : public EmbeddedQueue<3>
336336
double m_initX;
337337
double m_initY;
338338
bool m_has_init;
339+
bool m_was_clipped;
339340

340341
public:
341342
PathClipper(VertexSource &source, bool do_clipping, double width, double height)
@@ -347,7 +348,8 @@ class PathClipper : public EmbeddedQueue<3>
347348
m_moveto(true),
348349
m_initX(nan("")),
349350
m_initY(nan("")),
350-
m_has_init(false)
351+
m_has_init(false),
352+
m_was_clipped(false)
351353
{
352354
// empty
353355
}
@@ -361,7 +363,8 @@ class PathClipper : public EmbeddedQueue<3>
361363
m_moveto(true),
362364
m_initX(nan("")),
363365
m_initY(nan("")),
364-
m_has_init(false)
366+
m_has_init(false),
367+
m_was_clipped(false)
365368
{
366369
m_cliprect.x1 -= 1.0;
367370
m_cliprect.y1 -= 1.0;
@@ -372,21 +375,29 @@ class PathClipper : public EmbeddedQueue<3>
372375
inline void rewind(unsigned path_id)
373376
{
374377
m_has_init = false;
378+
m_was_clipped = false;
375379
m_moveto = true;
376380
m_source->rewind(path_id);
377381
}
378382

379-
int draw_clipped_line(double x0, double y0, double x1, double y1)
383+
int draw_clipped_line(double x0, double y0, double x1, double y1,
384+
bool closed=false)
380385
{
381386
unsigned moved = agg::clip_line_segment(&x0, &y0, &x1, &y1, m_cliprect);
382387
// moved >= 4 - Fully clipped
383388
// moved & 1 != 0 - First point has been moved
384389
// moved & 2 != 0 - Second point has been moved
390+
m_was_clipped = m_was_clipped || (moved != 0);
385391
if (moved < 4) {
386392
if (moved & 1 || m_moveto) {
387393
queue_push(agg::path_cmd_move_to, x0, y0);
388394
}
389395
queue_push(agg::path_cmd_line_to, x1, y1);
396+
if (closed && !m_was_clipped) {
397+
// Close the path only if the end point hasn't moved.
398+
queue_push(agg::path_cmd_end_poly | agg::path_flags_close,
399+
x1, y1);
400+
}
390401

391402
m_moveto = false;
392403
return 1;
@@ -417,12 +428,23 @@ class PathClipper : public EmbeddedQueue<3>
417428
switch (code) {
418429
case (agg::path_cmd_end_poly | agg::path_flags_close):
419430
if (m_has_init) {
420-
draw_clipped_line(m_lastX, m_lastY, m_initX, m_initY);
431+
// Queue the line from last point to the initial point, and
432+
// if never clipped, add a close code.
433+
draw_clipped_line(m_lastX, m_lastY, m_initX, m_initY,
434+
true);
435+
} else {
436+
// An empty path that is immediately closed.
437+
queue_push(
438+
agg::path_cmd_end_poly | agg::path_flags_close,
439+
m_lastX, m_lastY);
421440
}
422-
queue_push(
423-
agg::path_cmd_end_poly | agg::path_flags_close,
424-
m_lastX, m_lastY);
425-
goto exit_loop;
441+
// If paths were not clipped, then the above code queued
442+
// something, and we should exit the loop. Otherwise, continue
443+
// to the next point, as there may be a new subpath.
444+
if (queue_nonempty()) {
445+
goto exit_loop;
446+
}
447+
break;
426448

427449
case agg::path_cmd_move_to:
428450

@@ -444,6 +466,7 @@ class PathClipper : public EmbeddedQueue<3>
444466
m_initY = m_lastY = *y;
445467
m_has_init = true;
446468
m_moveto = true;
469+
m_was_clipped = false;
447470
// if the last command was moveto exit the loop to emit the code
448471
if (emit_moveto) {
449472
goto exit_loop;

0 commit comments

Comments
 (0)
0