svg-flatten: Fix fill-rule handling for filled open paths

3 files changed, 47 insertions, 30 deletions

diff --git a/svg-flatten/src/svg_doc.cpp b/svg-flatten/src/svg_doc.cpp
index 0ae9b68..f387bcc 100644
--- a/svg-flatten/src/svg_doc.cpp
+++ b/svg-flatten/src/svg_doc.cpp
@@ -234,29 +234,32 @@ void gerbolyze::SVGDocument::export_svg_path(const RenderSettings &rset, const p
     cairo_save(cr);
     apply_cairo_transform_from_svg(cr, node.attribute("transform").value());
 
+    PolyTree ptree_stroke;
+    PolyTree ptree_fill;
     PolyTree ptree;
-    load_svg_path(cr, node, ptree);
+    load_svg_path(cr, node, ptree_stroke, ptree_fill);
 
     double _y = 0;
     cairo_user_to_device_distance(cr, &stroke_width, &_y);
 
     cairo_restore (cr);
 
-    Paths open_paths, closed_paths;
-    OpenPathsFromPolyTree(ptree, open_paths);
-    ClosedPathsFromPolyTree(ptree, closed_paths);
+    Paths open_paths, closed_paths, fill_paths;
+    OpenPathsFromPolyTree(ptree_stroke, open_paths);
+    ClosedPathsFromPolyTree(ptree_stroke, closed_paths);
+    PolyTreeToPaths(ptree_fill, fill_paths);
 
     /* Skip filling for transparent fills */
     if (fill_color) {
         /* Clip paths. Consider all paths closed for filling. */
         if (!clip_path.empty()) {
             Clipper c;
-            c.AddPaths(open_paths, ptSubject, /* closed */ false);
-            c.AddPaths(closed_paths, ptSubject, /* closed */ true);
+            c.AddPaths(fill_paths, ptSubject, /* closed */ true);
             c.AddPaths(clip_path, ptClip, /* closed */ true);
             c.StrictlySimple(true);
             /* fill rules are nonzero since both subject and clip have already been normalized by clipper. */ 
             c.Execute(ctIntersection, ptree, pftNonZero, pftNonZero);
+            PolyTreeToPaths(ptree, fill_paths);
         }
 
         /* Call out to pattern tiler for pattern fills. The path becomes the clip here. */
@@ -267,16 +270,14 @@ void gerbolyze::SVGDocument::export_svg_path(const RenderSettings &rset, const p
                 cerr << "Warning: Fill pattern with id \"" << fill_pattern_id << "\" not found." << endl;
 
             } else {
-                Paths clip;
-                PolyTreeToPaths(ptree, clip);
-                pattern->tile(rset, clip);
+                pattern->tile(rset, fill_paths);
             }
 
         } else { /* solid fill */
             Paths f_polys;
             /* Important for gerber spec compliance and also for reliable rendering results irrespective of board house
              * and gerber viewer. */
-            dehole_polytree(ptree, f_polys);
+            dehole_polytree(ptree_fill, f_polys);
 
             /* Export SVG */
             cairo_save(cr);
@@ -475,15 +476,16 @@ void gerbolyze::SVGDocument::load_clips() {
          * rendering, and the only way a group might stay is if it affects rasterization (e.g. through mask, clipPath).
          */
         for (const auto &child : node.children("path")) {
-            PolyTree ptree;
+            PolyTree ptree_stroke; /* discarded */
+            PolyTree ptree_fill;
             cairo_save(cr);
             /* TODO: we currently only support clipPathUnits="userSpaceOnUse", not "objectBoundingBox". */
             apply_cairo_transform_from_svg(cr, child.attribute("transform").value());
-            load_svg_path(cr, child, ptree);
+            load_svg_path(cr, child, ptree_stroke, ptree_fill);
             cairo_restore (cr);
 
             Paths paths;
-            PolyTreeToPaths(ptree, paths);
+            PolyTreeToPaths(ptree_fill, paths);
             c.AddPaths(paths, ptSubject, /* closed */ false);
         }
 
diff --git a/svg-flatten/src/svg_path.cpp b/svg-flatten/src/svg_path.cpp
index e45a0c9..68003ad 100644
--- a/svg-flatten/src/svg_path.cpp
+++ b/svg-flatten/src/svg_path.cpp
@@ -32,7 +32,7 @@ static void clipper_add_cairo_path(cairo_t *cr, ClipperLib::Clipper &c, bool clo
     c.AddPaths(in_poly, ClipperLib::ptSubject, closed);
 }
 
-static bool path_to_clipper_via_cairo(cairo_t *cr, ClipperLib::Clipper &c, const pugi::char_t *path_data) {
+static pair<bool, bool> path_to_clipper_via_cairo(cairo_t *cr, ClipperLib::Clipper &c_stroke, ClipperLib::Clipper &c_fill, const pugi::char_t *path_data) {
     istringstream d(path_data);
 
     string cmd;
@@ -41,6 +41,7 @@ static bool path_to_clipper_via_cairo(cairo_t *cr, ClipperLib::Clipper &c, const
     bool first = true;
     bool has_closed = false;
     bool path_is_empty = true;
+    int num_subpaths = 0;
     while (!d.eof()) {
         d >> cmd;
         assert (!d.fail());
@@ -48,12 +49,23 @@ static bool path_to_clipper_via_cairo(cairo_t *cr, ClipperLib::Clipper &c, const
         
         if (cmd == "Z") { /* Close path */
             cairo_close_path(cr);
-            clipper_add_cairo_path(cr, c, /* closed= */ true);
+            clipper_add_cairo_path(cr, c_stroke, /* closed= */ true);
+            clipper_add_cairo_path(cr, c_fill, /* closed= */ true);
             has_closed = true;
             cairo_new_path(cr);
             path_is_empty = true;
+            num_subpaths += 1;
 
         } else if (cmd == "M") { /* Move to */
+            if (!first && !path_is_empty) {
+                cairo_close_path(cr);
+                clipper_add_cairo_path(cr, c_stroke, /* closed= */ false);
+                clipper_add_cairo_path(cr, c_fill, /* closed= */ true);
+                num_subpaths += 1;
+            }
+
+            cairo_new_path (cr);
+
             d >> x >> y;
             /* We need to transform all points ourselves here, and cannot use the transform feature of cairo_to_clipper:
              * Our transform may contain offsets, and clipper only passes its data into cairo's transform functions
@@ -63,10 +75,6 @@ static bool path_to_clipper_via_cairo(cairo_t *cr, ClipperLib::Clipper &c, const
              */
             cairo_user_to_device(cr, &x, &y);
             assert (!d.fail());
-            if (!first) {
-                clipper_add_cairo_path(cr, c, /* closed= */ false);
-            }
-            cairo_new_path (cr);
             path_is_empty = true;
             cairo_move_to(cr, x, y);
 
@@ -94,13 +102,15 @@ static bool path_to_clipper_via_cairo(cairo_t *cr, ClipperLib::Clipper &c, const
     }
     if (!path_is_empty) {
         cairo_close_path(cr);
-        clipper_add_cairo_path(cr, c, /* closed= */ false);
+        clipper_add_cairo_path(cr, c_stroke, /* closed= */ false);
+        clipper_add_cairo_path(cr, c_fill, /* closed= */ true);
+        num_subpaths += 1;
     }
 
-    return has_closed;
+    return {has_closed, num_subpaths > 1};
 }
 
-void gerbolyze::load_svg_path(cairo_t *cr, const pugi::xml_node &node, ClipperLib::PolyTree &ptree) {
+void gerbolyze::load_svg_path(cairo_t *cr, const pugi::xml_node &node, ClipperLib::PolyTree &ptree_stroke, ClipperLib::PolyTree &ptree_fill) {
     auto *path_data = node.attribute("d").value();
     auto fill_rule = clipper_fill_rule(node);
 
@@ -110,11 +120,14 @@ void gerbolyze::load_svg_path(cairo_t *cr, const pugi::xml_node &node, ClipperLi
     cairo_set_tolerance (cr, 0.1); /* FIXME make configurable, scale properly for units */
     cairo_set_fill_rule(cr, CAIRO_FILL_RULE_WINDING);
 
-    ClipperLib::Clipper c;
-    c.StrictlySimple(true);
-    bool has_closed = path_to_clipper_via_cairo(cr, c, path_data);
+    ClipperLib::Clipper c_stroke;
+    ClipperLib::Clipper c_fill;
+    c_stroke.StrictlySimple(true);
+    c_fill.StrictlySimple(true);
+    auto res = path_to_clipper_via_cairo(cr, c_stroke, c_fill, path_data);
+    bool has_closed = res.first, has_multiple = res.second;
 
-    if (!has_closed) {
+    if (!has_closed && !has_multiple) {
         /* FIXME: Workaround!
          *
          * When we render silkscreen layers from gerbv's output, we get a lot of two-point paths (lines). Many of these are
@@ -132,12 +145,14 @@ void gerbolyze::load_svg_path(cairo_t *cr, const pugi::xml_node &node, ClipperLi
         auto le_min = -ClipperLib::loRange;
         auto le_max = ClipperLib::hiRange;
         ClipperLib::Path p = {{le_min, le_min}, {le_max, le_min}, {le_max, le_max}, {le_min, le_max}};
-        c.AddPath(p, ClipperLib::ptClip, /* closed= */ true);
-        c.Execute(ClipperLib::ctIntersection, ptree, fill_rule, ClipperLib::pftNonZero);
+        c_stroke.AddPath(p, ClipperLib::ptClip, /* closed= */ true);
+        c_stroke.Execute(ClipperLib::ctIntersection, ptree_stroke, fill_rule, ClipperLib::pftNonZero);
+        ptree_fill.Clear();
 
     } else {
         /* We cannot clip the polygon here since that would produce incorrect results for our stroke. */
-        c.Execute(ClipperLib::ctUnion, ptree, fill_rule, ClipperLib::pftNonZero);
+        c_stroke.Execute(ClipperLib::ctUnion, ptree_stroke, fill_rule, ClipperLib::pftNonZero);
+        c_fill.Execute(ClipperLib::ctUnion, ptree_fill, fill_rule, ClipperLib::pftNonZero);
     }
 }
 
diff --git a/svg-flatten/src/svg_path.h b/svg-flatten/src/svg_path.h
index 9c65fdd..689af63 100644
--- a/svg-flatten/src/svg_path.h
+++ b/svg-flatten/src/svg_path.h
@@ -23,7 +23,7 @@
 #include "svg_geom.h"
 
 namespace gerbolyze {
-void load_svg_path(cairo_t *cr, const pugi::xml_node &node, ClipperLib::PolyTree &ptree);
+void load_svg_path(cairo_t *cr, const pugi::xml_node &node, ClipperLib::PolyTree &ptree_stroke, ClipperLib::PolyTree &ptree_fill);
 void parse_dasharray(const pugi::xml_node &node, std::vector<double> &out);
 void dash_path(const ClipperLib::Path &in, ClipperLib::Paths &out, const std::vector<double> dasharray, double dash_offset=0.0);
 }