svg-flatten: Fix include/exclude logic

5 files changed, 214 insertions, 29 deletions

diff --git a/svg-flatten/include/gerbolyze.hpp b/svg-flatten/include/gerbolyze.hpp
index 2c21173..9311a98 100644
--- a/svg-flatten/include/gerbolyze.hpp
+++ b/svg-flatten/include/gerbolyze.hpp
@@ -143,15 +143,15 @@ namespace gerbolyze {
 
     class ElementSelector {
     public:
-        virtual bool match(const pugi::xml_node &node, bool included, bool is_root) const {
-            (void) node, (void) included, (void) is_root;
+        virtual bool match(const pugi::xml_node &node, bool is_toplevel, bool parent_include) const {
+            (void) node, (void) is_toplevel, (void) parent_include;
             return true;
         }
     };
 
     class IDElementSelector : public ElementSelector {
     public:
-        virtual bool match(const pugi::xml_node &node, bool included, bool is_root) const;
+        virtual bool match(const pugi::xml_node &node, bool is_toplevel, bool parent_include) const;
 
         std::vector<std::string> include;
         std::vector<std::string> exclude;
@@ -196,7 +196,8 @@ namespace gerbolyze {
                     xform2d transform);
             RenderContext(RenderContext &parent,
                     xform2d transform,
-                    ClipperLib::Paths &clip);
+                    ClipperLib::Paths &clip,
+                    bool included);
 
             PolygonSink &sink() { return m_sink; }
             const ElementSelector &sel() { return m_sel; }
@@ -209,7 +210,7 @@ namespace gerbolyze {
                 m_mat.transform(transform);
             }
             bool match(const pugi::xml_node &node) {
-                return m_sel.match(node, m_included, m_root);
+                return m_sel.match(node, m_root, m_included);
             }
 
         private:
diff --git a/svg-flatten/src/main.cpp b/svg-flatten/src/main.cpp
index 54ce896..8512547 100644
--- a/svg-flatten/src/main.cpp
+++ b/svg-flatten/src/main.cpp
@@ -377,6 +377,15 @@ int main(int argc, char **argv) {
         return EXIT_FAILURE;
     }
 
+    /*
+    cerr << "Selectors:" << endl;
+    for (auto &elem : sel.include) {
+        cerr << " + " << elem << endl;
+    }
+    for (auto &elem : sel.exclude) {
+        cerr << " - " << elem << endl;
+    }
+    */
     doc.render(rset, *top_sink, sel);
 
     remove(frob.c_str());
diff --git a/svg-flatten/src/svg_doc.cpp b/svg-flatten/src/svg_doc.cpp
index d90e00d..5a27163 100644
--- a/svg-flatten/src/svg_doc.cpp
+++ b/svg-flatten/src/svg_doc.cpp
@@ -108,9 +108,10 @@ double gerbolyze::SVGDocument::doc_units_to_mm(double px) const {
     return px / (vb_w / page_w_mm);
 }
 
-bool IDElementSelector::match(const pugi::xml_node &node, bool included, bool is_root) const {
+bool IDElementSelector::match(const pugi::xml_node &node, bool is_toplevel, bool parent_include) const {
     string id = node.attribute("id").value();
-    if (is_root && layers) {
+    cerr << "match id=" << id << " toplevel=" << is_toplevel << " parent=" << parent_include << endl;
+    if (is_toplevel && layers) {
         bool layer_match = std::find(layers->begin(), layers->end(), id) != layers->end();
         if (!layer_match) {
             cerr << "Rejecting layer \"" << id << "\"" << endl;
@@ -123,12 +124,24 @@ bool IDElementSelector::match(const pugi::xml_node &node, bool included, bool is
 
     bool include_match = std::find(include.begin(), include.end(), id) != include.end();
     bool exclude_match = std::find(exclude.begin(), exclude.end(), id) != exclude.end();
+    cerr << "  excl=" << exclude_match << " incl=" << include_match << endl;
 
-    if (exclude_match || (!included && !include_match)) {
+    if (is_toplevel) {
+        if (!include.empty())
+            parent_include = false;
+        else
+            parent_include = true;
+    }
+
+    if (exclude_match) {
         return false;
     }
 
-    return true;
+    if (include_match) {
+        return true;
+    }
+
+    return parent_include;
 }
 
 /* Recursively export all SVG elements in the given group. */
@@ -164,11 +177,10 @@ void gerbolyze::SVGDocument::export_svg_group(RenderContext &ctx, const pugi::xm
 
     /* Iterate over the group's children, exporting them one by one. */
     for (const auto &node : group.children()) {
-        if (!ctx.match(node))
-            continue;
-
         string name(node.name());
-        RenderContext elem_ctx(ctx, xform2d(node.attribute("transform").value()), clip_path);
+        bool match = ctx.match(node);
+        RenderContext elem_ctx(ctx, xform2d(node.attribute("transform").value()), clip_path, match);
+
         if (name == "g") {
             if (ctx.root()) { /* Treat top-level groups as "layers" like inkscape does. */
                 cerr << "Forwarding layer name to sink: \"" << node.attribute("id").value() << "\"" << endl;
@@ -184,9 +196,15 @@ void gerbolyze::SVGDocument::export_svg_group(RenderContext &ctx, const pugi::xm
             }
 
         } else if (name == "path") {
+            if (!match)
+                continue;
+
             export_svg_path(elem_ctx, node);
 
         } else if (name == "image") {
+            if (!match)
+                continue;
+
             ImageVectorizer *vec = ctx.settings().m_vec_sel.select(node);
             if (!vec) {
                 cerr << "Cannot resolve vectorizer for node \"" << node.attribute("id").value() << "\"" << endl;
@@ -261,7 +279,7 @@ void gerbolyze::SVGDocument::export_svg_path(RenderContext &ctx, const pugi::xml
 
             } else {
                 PolyTreeToPaths(ptree_fill, fill_paths);
-                RenderContext local_ctx(ctx, xform2d(), fill_paths);
+                RenderContext local_ctx(ctx, xform2d(), fill_paths, true);
                 pattern->tile(local_ctx);
             }
 
@@ -366,7 +384,7 @@ void gerbolyze::SVGDocument::export_svg_path(RenderContext &ctx, const pugi::xml
             } else {
                 Paths clip;
                 PolyTreeToPaths(ptree, clip);
-                RenderContext local_ctx(ctx, xform2d(), clip);
+                RenderContext local_ctx(ctx, xform2d(), clip, true);
                 pattern->tile(local_ctx);
             }
 
@@ -490,16 +508,16 @@ gerbolyze::RenderContext::RenderContext(const RenderSettings &settings,
 }
 
 gerbolyze::RenderContext::RenderContext(RenderContext &parent, xform2d transform) :
-    RenderContext(parent, transform, parent.clip())
+    RenderContext(parent, transform, parent.clip(), parent.included())
 {
 }
 
-gerbolyze::RenderContext::RenderContext(RenderContext &parent, xform2d transform, ClipperLib::Paths &clip) :
+gerbolyze::RenderContext::RenderContext(RenderContext &parent, xform2d transform, ClipperLib::Paths &clip, bool included) :
     m_sink(parent.sink()),
     m_settings(parent.settings()),
     m_mat(parent.mat()),
     m_root(false),
-    m_included(parent.included()),
+    m_included(included),
     m_sel(parent.sel()),
     m_clip(clip)
 {
diff --git a/svg-flatten/src/test/svg_tests.py b/svg-flatten/src/test/svg_tests.py
index c2e5d50..4db827e 100644
--- a/svg-flatten/src/test/svg_tests.py
+++ b/svg-flatten/src/test/svg_tests.py
@@ -82,7 +82,7 @@ class SVGRoundTripTests(unittest.TestCase):
         'pattern_stroke_dashed'
     }
 
-    def compare_images(self, reference, output, test_name, mean, vectorizer_test=False, rsvg_workaround=False):
+    def compare_images(self, reference, output, test_name, mean=test_mean_default, vectorizer_test=False, rsvg_workaround=False):
         ref, out = Image.open(reference), Image.open(output)
 
         if vectorizer_test:
@@ -116,6 +116,49 @@ class SVGRoundTripTests(unittest.TestCase):
         self.assertTrue(delta.mean() < mean,
                 f'Expected mean pixel difference between images to be <{mean}, was {delta.mean():.5g}')
 
+
+    def run_svg_group_selector_test(self, mode, groups):
+        test_in_svg = 'testdata/group_test_input.svg'
+
+        with tempfile.NamedTemporaryFile(suffix='.svg') as tmp_out_svg,\
+            tempfile.NamedTemporaryFile(suffix='.svg') as tmp_ref_svg,\
+            tempfile.NamedTemporaryFile(suffix='.png') as tmp_out_png,\
+            tempfile.NamedTemporaryFile(suffix='.png') as tmp_in_png:
+
+            if mode == 'inc':
+                group_arg = { 'only_groups': ','.join(groups) }
+            elif mode == 'exc':
+                group_arg = { 'exclude_groups': ','.join(groups) }
+            run_svg_flatten(test_in_svg, tmp_out_svg.name, format='svg', **group_arg)
+
+            with open(test_in_svg, 'r') as in_f:
+                with open(tmp_ref_svg.name, 'w') as out_f:
+                    if mode == 'inc':
+                        css = '#layer1 { fill: none; }\n'
+                        css += '\n'.join(f'#{group} {{ fill: black; }}' for group in groups)
+                    elif mode == 'exc':
+                        css = '\n'.join(f'#{group} {{ fill: none; }}' for group in groups)
+                    else:
+                        raise ValueError(f'invalid mode "{mode}"')
+                    out_f.write(in_f.read().replace('/* {CSS GOES HERE} */', css))
+
+            run_cargo_cmd('resvg', [tmp_out_svg.name, tmp_out_png.name], check=True, stdout=subprocess.DEVNULL)
+            run_cargo_cmd('resvg', [tmp_ref_svg.name, tmp_in_png.name], check=True, stdout=subprocess.DEVNULL)
+
+            tc_id = f'group_sel_test_{mode}_{"_".join(groups)}'
+            try:
+                self.compare_images(tmp_in_png, tmp_out_png, tc_id, mean=0.001)
+
+            except AssertionError as e:
+                shutil.copyfile(tmp_in_png.name, f'/tmp/gerbolyze-fail-{tc_id}-in.png')
+                shutil.copyfile(tmp_out_png.name, f'/tmp/gerbolyze-fail-{tc_id}-out.png')
+                msg, *rest = e.args
+                msg += '\nFailing test renderings copied to:\n'
+                msg += f'  /tmp/gerbolyze-fail-{tc_id}-{{in|out}}.png\n'
+                e.args = (msg, *rest)
+                raise e
+
+
     def run_svg_round_trip_test(self, test_in_svg):
         with tempfile.NamedTemporaryFile(suffix='.svg') as tmp_out_svg,\
             tempfile.NamedTemporaryFile(suffix='.png') as tmp_out_png,\
@@ -128,18 +171,18 @@ class SVGRoundTripTests(unittest.TestCase):
             if not vectorizer_test:
                 run_svg_flatten(test_in_svg, tmp_out_svg.name, format='svg')
 
-            else:
-                run_svg_flatten(test_in_svg, tmp_out_svg.name, format='svg',
-                        svg_white_is_gerber_dark=True,
-                        clear_color='black', dark_color='white')
-
-            if contours_test:
+            elif contours_test:
                 run_svg_flatten(test_in_svg, tmp_out_svg.name, 
                         clear_color='black', dark_color='white',
                         svg_white_is_gerber_dark=True,
                         format='svg',
                         vectorizer='binary-contours')
 
+            else:
+                run_svg_flatten(test_in_svg, tmp_out_svg.name, format='svg',
+                        svg_white_is_gerber_dark=True,
+                        clear_color='black', dark_color='white')
+
             if not use_rsvg: # default!
                 run_cargo_cmd('resvg', [tmp_out_svg.name, tmp_out_png.name], check=True, stdout=subprocess.DEVNULL)
                 run_cargo_cmd('resvg', [test_in_svg, tmp_in_png.name], check=True, stdout=subprocess.DEVNULL)
@@ -156,10 +199,10 @@ class SVGRoundTripTests(unittest.TestCase):
             except AssertionError as e:
                 shutil.copyfile(tmp_in_png.name, f'/tmp/gerbolyze-fail-{test_in_svg.stem}-in.png')
                 shutil.copyfile(tmp_out_png.name, f'/tmp/gerbolyze-fail-{test_in_svg.stem}-out.png')
-                foo = list(e.args)
-                foo[0] += '\nFailing test renderings copied to:\n'
-                foo[0] += f'  /tmp/gerbolyze-fail-{test_in_svg.stem}-{{in|out}}.png\n'
-                e.args = tuple(foo)
+                msg, *rest = e.args
+                msg += '\nFailing test renderings copied to:\n'
+                msg += f'  /tmp/gerbolyze-fail-{test_in_svg.stem}-{{in|out}}.png\n'
+                e.args = (msg, *rest)
                 raise e
 
 for test_in_svg in Path('testdata/svg').glob('*.svg'):
@@ -167,5 +210,11 @@ for test_in_svg in Path('testdata/svg').glob('*.svg'):
     gen = lambda testcase: lambda self: self.run_svg_round_trip_test(testcase)
     setattr(SVGRoundTripTests, f'test_{test_in_svg.stem}', gen(test_in_svg))
 
+for group in ["g0", "g00", "g000", "g0000", "g00000", "g0001", "g001", "g0010", "g002", "g01", "g010", "g0100", "g011",
+              "g02", "g020", "g03", "path846-59", "path846-3-2", "path846-5-2", "path846-3-3-8"]:
+    gen = lambda mode, group: lambda self: self.run_svg_group_selector_test(mode, group)
+    setattr(SVGRoundTripTests, f'test_group_sel_inc_{group}', gen('inc', [group]))
+    setattr(SVGRoundTripTests, f'test_group_sel_exc_{group}', gen('exc', [group]))
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/svg-flatten/testdata/group_test_input.svg b/svg-flatten/testdata/group_test_input.svg
new file mode 100644
index 0000000..8310000
--- /dev/null
+++ b/svg-flatten/testdata/group_test_input.svg
@@ -0,0 +1,108 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<!-- Created with Inkscape (http://www.inkscape.org/) -->
+
+<svg
+   width="30mm"
+   height="30mm"
+   viewBox="0 0 30 30"
+   version="1.1"
+   id="svg5"
+   inkscape:version="1.1 (c4e8f9ed74, 2021-05-24)"
+   sodipodi:docname="group_test_input.svg"
+   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
+   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
+   xmlns="http://www.w3.org/2000/svg"
+   xmlns:svg="http://www.w3.org/2000/svg">
+  <sodipodi:namedview
+     id="namedview7"
+     pagecolor="#ffffff"
+     bordercolor="#666666"
+     borderopacity="1.0"
+     inkscape:pageshadow="2"
+     inkscape:pageopacity="0.0"
+     inkscape:pagecheckerboard="0"
+     inkscape:document-units="mm"
+     showgrid="false"
+     width="30mm"
+     showguides="true"
+     inkscape:lockguides="true"
+     inkscape:zoom="4.9925495"
+     inkscape:cx="46.769691"
+     inkscape:cy="33.149396"
+     inkscape:window-width="1920"
+     inkscape:window-height="1024"
+     inkscape:window-x="0"
+     inkscape:window-y="0"
+     inkscape:window-maximized="1"
+     inkscape:current-layer="layer1" />
+  <defs id="defs2" />
+  <style>
+  /* {CSS GOES HERE} */
+  </style>
+  <g inkscape:label="Layer 1" inkscape:groupmode="layer" id="layer1">
+    <g id="g0" transform="translate(1.5,3)">
+      <g id="g00">
+        <g id="g000">
+          <g id="g0000">
+            <g id="g00000">
+              <circle id="path846" cx="3" cy="3" r="1" />
+              <circle id="path846-3" cx="6" cy="3" r="1" />
+            </g>
+            <circle id="path846-5" cx="9" cy="3" r="1" />
+            <circle id="path846-3-3" cx="12" cy="3" r="1" />
+          </g>
+          <g id="g0001" transform="translate(12)">
+            <circle id="path846-2" cx="3" cy="3" r="1" />
+            <circle id="path846-3-9" cx="6" cy="3" r="1" />
+          </g>
+          <circle id="path846-5-1" cx="21" cy="3" r="1" />
+          <circle id="path846-3-3-2" cx="24" cy="3" r="1" />
+        </g>
+        <g id="g001" transform="translate(0,3)">
+          <g id="g0010">
+            <circle id="path846-36" cx="3" cy="3" r="1" />
+            <circle id="path846-3-0" cx="6" cy="3" r="1" />
+          </g>
+          <circle id="path846-5-6" cx="9" cy="3" r="1" />
+          <circle id="path846-3-3-26" cx="12" cy="3" r="1" />
+        </g>
+        <g id="g002" transform="translate(12,3)">
+          <circle id="path846-2-8" cx="3" cy="3" r="1" />
+          <circle id="path846-3-9-7" cx="6" cy="3" r="1" />
+        </g>
+        <circle id="path846-5-1-9" cx="21" cy="6" r="1" />
+        <circle id="path846-3-3-2-2" cx="24" cy="6" r="1" />
+      </g>
+      <g id="g01" transform="translate(0,6)">
+        <g id="g010">
+          <g id="g0100">
+            <circle id="path846-59" cx="3" cy="3" r="1" />
+            <circle id="path846-3-2" cx="6" cy="3" r="1" />
+          </g>
+          <circle id="path846-5-2" cx="9" cy="3" r="1" />
+          <circle id="path846-3-3-8" cx="12" cy="3" r="1" />
+        </g>
+        <g id="g011" transform="translate(12)">
+          <circle id="path846-2-7" cx="3" cy="3" r="1" />
+          <circle id="path846-3-9-3" cx="6" cy="3" r="1" />
+        </g>
+        <circle id="path846-5-1-6" cx="21" cy="3" r="1" />
+        <circle id="path846-3-3-2-1" cx="24" cy="3" r="1" />
+      </g>
+      <g id="g02" transform="translate(0,9)">
+        <g id="g020">
+          <circle id="path846-36-3" cx="3" cy="3" r="1" />
+          <circle id="path846-3-0-1" cx="6" cy="3" r="1" />
+        </g>
+        <circle id="path846-5-6-9" cx="9" cy="3" r="1" />
+        <circle id="path846-3-3-26-4" cx="12" cy="3" r="1" />
+      </g>
+      <g id="g03" transform="translate(12,9)">
+        <circle id="path846-2-8-8" cx="3" cy="3" r="1" />
+        <circle id="path846-3-9-7-4" cx="6" cy="3" r="1" />
+      </g>
+      <circle id="path846-5-1-9-5" cx="21" cy="12" r="1" />
+      <circle id="path846-3-3-2-2-0" cx="24" cy="12" r="1" />
+    </g>
+  </g>
+</svg>