Fixed hidden objects being stitched out when cloned (Fix #3167)

lib/elements/clone.py

@@ -17,8 +17,7 @@ from ..i18n import _
 from ..svg.svg import copy_no_children
 from ..svg.path import get_node_transform
 from ..svg.tags import (EMBROIDERABLE_TAGS, INKSTITCH_ATTRIBS, SVG_USE_TAG,
-                        XLINK_HREF, CONNECTION_START, CONNECTION_END,
-                        SVG_GROUP_TAG)
+                        XLINK_HREF, CONNECTION_START, CONNECTION_END)
 from ..utils import cache
 from .element import EmbroideryElement, param
 from .validation import ValidationWarning
@@ -73,14 +72,8 @@ class Clone(EmbroideryElement):
         return [element.get_cache_key(previous_stitch) for element in source_elements]
 
     def clone_to_elements(self, node):
-        from .utils import node_to_elements
-        elements = []
-        if node.tag in EMBROIDERABLE_TAGS:
-            elements = node_to_elements(node, True)
-        elif node.tag == SVG_GROUP_TAG:
-            for child in node.iterdescendants():
-                elements.extend(node_to_elements(child, True))
-        return elements
+        from .utils import node_and_children_to_elements
+        return node_and_children_to_elements(node, True)
 
     def to_stitch_groups(self, last_stitch_group=None) -> List[StitchGroup]:
         if not self.clone:
@@ -163,7 +156,7 @@ class Clone(EmbroideryElement):
 
         ret = [cloned_node]
 
-        # For aesthetic purposes, transform allof the cloned command symbols so they're facing upwards
+        # For aesthetic purposes, transform all of the cloned command symbols so they're facing upwards
         point_command_symbols_up(cloned_node)
 
         # We need to copy all commands that were attached directly to the href'd node
@@ -245,7 +238,7 @@ def clone_with_fixup(parent: BaseElement, node: BaseElement) -> BaseElement:
         parent.append(cloned)
         id_map[f"#{node.get_id()}"] = f"#{cloned.get_id()}"
 
-        for child in node.getchildren():
+        for child in node:
             clone_children(cloned, child)
 
         return cloned

lib/elements/utils.py

@@ -15,9 +15,11 @@ from .marker import MarkerObject
 from .satin_column import SatinColumn
 from .stroke import Stroke
 from .text import TextObject
+from typing import List
+from inkex import BaseElement
 
 
-def node_to_elements(node, clone_to_element=False):  # noqa: C901
+def node_to_elements(node, clone_to_element=False) -> List[EmbroideryElement]:  # noqa: C901
     if is_clone(node) and not clone_to_element:
         # clone_to_element: get an actual embroiderable element once a clone has been defined as a clone
         return [Clone(node)]
@@ -53,6 +55,28 @@ def node_to_elements(node, clone_to_element=False):  # noqa: C901
         return []
 
 
+def node_and_children_to_elements(node: BaseElement, clone_to_element=False) -> List[EmbroideryElement]:
+    """
+    Iterate through a node and its children, and return all applicable EmbroideryElements.
+
+    Notably, does not return EmbroideryElements for hidden elements or children of hidden elements,
+    similar to `Base.descendants`.
+    """
+    elements: List[EmbroideryElement] = []
+
+    def walk(node: BaseElement):
+        if node.get_computed_style('display') == 'none':
+            return
+
+        elements.extend(node_to_elements(node, clone_to_element))
+        for child in node:
+            walk(child)
+
+    walk(node)
+
+    return elements
+
+
 def nodes_to_elements(nodes):
     elements = []
     for node in nodes:

tests/test_clone.py

@@ -19,6 +19,7 @@ def element_fill_angle(element: EmbroideryElement) -> Optional[float]:
 
 
 class CloneElementTest(TestCase):
+    # Monkey-patch the cahce to forcibly disable it: We may need to refactor this out for tests.
     def setUp(self):
         from pytest import MonkeyPatch
         self.monkeypatch = MonkeyPatch()

tests/test_elements_utils.py

@@ -0,0 +1,50 @@
+from lib.elements import utils, FillStitch
+from inkex import Rectangle, Group, Style
+from inkex.tester import TestCase
+from inkex.tester.svg import svg
+
+
+class ElementsUtilsTest(TestCase):
+    def test_node_and_children_to_elements(self):
+        root = svg()
+        g = root.add(Group())
+        rect = g.add(Rectangle(attrib={
+            "width": "10",
+            "height": "10"
+        }))
+        hidden_rect = g.add(Rectangle(attrib={  # noqa: F841
+            "width": "10",
+            "height": "10",
+            "style": "display:none"
+        }))
+        hidden_group = g.add(Group(attrib={
+            "style": "display:none"
+        }))
+        child_of_hidden = hidden_group.add(Rectangle(attrib={  # noqa: F841
+            "width": "10",
+            "height": "10",
+        }))
+
+        elements = utils.node_and_children_to_elements(g)
+        self.assertEqual(len(elements), 1)
+        self.assertEqual(type(elements[0]), FillStitch)
+        self.assertEqual(elements[0].node, rect)
+
+    def test_node_and_children_to_elements_root_embroiderable(self):
+        """ Test node_and_children_to_elements where the the node passed is directly embroiderable """
+        root = svg()
+        rect = root.add(Rectangle(attrib={
+            "width": "10",
+            "height": "10"
+        }))
+
+        elements = utils.node_and_children_to_elements(rect)
+        self.assertEqual(len(elements), 1)
+        self.assertEqual(type(elements[0]), FillStitch)
+        self.assertEqual(elements[0].node, rect)
+
+        # Now make the element hidden: It shouldn't return an element
+        rect.style = rect.style + Style({"display": "none"})
+
+        elements = utils.node_and_children_to_elements(rect)
+        self.assertEqual(len(elements), 0)