Merge pull request #17885 from brunobeltran/ignore_closepoly_after_nan

QuLogic · web-flow · commit b7ba9e49b07c · 2020-07-15T17:57:29.000-04:00
BF: ignore CLOSEPOLY after NaN in PathNanRemover
diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
@@ -1404,6 +1404,11 @@ def test_bar_tick_label_single():
     ax.bar("a", "b", align='edge', tick_label='0', data=data)
 
 
+def test_nan_bar_values():
+    fig, ax = plt.subplots()
+    ax.bar([0, 1], [np.nan, 4])
+
+
 def test_bar_ticklabel_fail():
     fig, ax = plt.subplots()
     ax.bar([], [])
diff --git a/lib/matplotlib/tests/test_path.py b/lib/matplotlib/tests/test_path.py
@@ -452,3 +452,26 @@ def test_intersect_zero_length_segment():
 
     assert outline_path.intersects_path(this_path)
     assert this_path.intersects_path(outline_path)
+
+
+def test_cleanup_closepoly():
+    # if the first connected component of a Path ends in a CLOSEPOLY, but that
+    # component contains a NaN, then Path.cleaned should ignore not just the
+    # control points but also the CLOSEPOLY, since it has nowhere valid to
+    # point.
+    paths = [
+        Path([[np.nan, np.nan], [np.nan, np.nan]],
+             [Path.MOVETO, Path.CLOSEPOLY]),
+        # we trigger a different path in the C++ code if we don't pass any
+        # codes explicitly, so we must also make sure that this works
+        Path([[np.nan, np.nan], [np.nan, np.nan]]),
+        # we should also make sure that this cleanup works if there's some
+        # multi-vertex curves
+        Path([[np.nan, np.nan], [np.nan, np.nan], [np.nan, np.nan],
+              [np.nan, np.nan]],
+             [Path.MOVETO, Path.CURVE3, Path.CURVE3, Path.CLOSEPOLY])
+    ]
+    for p in paths:
+        cleaned = p.cleaned(remove_nans=True)
+        assert len(cleaned) == 1
+        assert cleaned.codes[0] == Path.STOP
diff --git a/src/path_converters.h b/src/path_converters.h
@@ -163,6 +163,7 @@ class PathNanRemover : protected EmbeddedQueue<4>
     VertexSource *m_source;
     bool m_remove_nans;
     bool m_has_curves;
+    bool valid_segment_exists;
 
   public:
     /* has_curves should be true if the path contains bezier curve
@@ -172,7 +173,9 @@ class PathNanRemover : protected EmbeddedQueue<4>
     PathNanRemover(VertexSource &source, bool remove_nans, bool has_curves)
         : m_source(&source), m_remove_nans(remove_nans), m_has_curves(has_curves)
     {
-        // empty
+        // ignore all close/end_poly commands until after the first valid
+        // (nan-free) command is encountered
+        valid_segment_exists = false;
     }
 
     inline void rewind(unsigned path_id)
@@ -202,8 +205,13 @@ class PathNanRemover : protected EmbeddedQueue<4>
                    are found along the way, the queue is emptied, and
                    the next curve segment is handled. */
                 code = m_source->vertex(x, y);
+                /* The vertices attached to STOP and CLOSEPOLY left are never
+                   used, so we leave them as-is even if NaN. However, CLOSEPOLY
+                   only makes sense if a valid MOVETO command has already been
+                   emitted. */
                 if (code == agg::path_cmd_stop ||
-                    code == (agg::path_cmd_end_poly | agg::path_flags_close)) {
+                    (code == (agg::path_cmd_end_poly | agg::path_flags_close) &&
+                     valid_segment_exists)) {
                     return code;
                 }
 
@@ -224,6 +232,7 @@ class PathNanRemover : protected EmbeddedQueue<4>
                 }
 
                 if (!has_nan) {
+                    valid_segment_exists = true;
                     break;
                 }
 
@@ -251,21 +260,23 @@ class PathNanRemover : protected EmbeddedQueue<4>
             code = m_source->vertex(x, y);
 
             if (code == agg::path_cmd_stop ||
-                code == (agg::path_cmd_end_poly | agg::path_flags_close)) {
+                (code == (agg::path_cmd_end_poly | agg::path_flags_close) &&
+                 valid_segment_exists)) {
                 return code;
             }
 
             if (!(std::isfinite(*x) && std::isfinite(*y))) {
                 do {
                     code = m_source->vertex(x, y);
                     if (code == agg::path_cmd_stop ||
-                        code == (agg::path_cmd_end_poly | agg::path_flags_close)) {
+                        (code == (agg::path_cmd_end_poly | agg::path_flags_close) &&
+                         valid_segment_exists)) {
                         return code;
                     }
                 } while (!(std::isfinite(*x) && std::isfinite(*y)));
                 return agg::path_cmd_move_to;
             }
-
+            valid_segment_exists = true;
             return code;
         }
     }
