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

Extracted Base.descendants into a util function

3 files changed, 81 insertions, 64 deletions

diff --git a/lib/elements/clone.py b/lib/elements/clone.py
index ce9c2162..73f3c6c0 100644
--- a/lib/elements/clone.py
+++ b/lib/elements/clone.py
@@ -72,7 +72,8 @@ class Clone(EmbroideryElement):
         source_elements = self.clone_to_elements(source_node)
         return [element.get_cache_key(previous_stitch) for element in source_elements]
 
-    def clone_to_elements(self, node):
+    def clone_to_elements(self, node) -> List[EmbroideryElement]:
+        # Only used in get_cache_key_data, actual embroidery uses nodes_to_elements+iterate_nodes
         from .utils import node_to_elements
         elements = []
         if node.tag in EMBROIDERABLE_TAGS:
@@ -107,11 +108,13 @@ class Clone(EmbroideryElement):
         Could possibly be refactored into just a generator - being a context manager is mainly to control the lifecycle of the elements
         that are cloned (again, for testing convenience primarily)
         """
+        from .utils import nodes_to_elements, iterate_nodes
+
         cloned_nodes = self.resolve_clone()
         try:
             # In a try block so we can ensure that the cloned_node is removed from the tree in the event of an exception.
             # Otherwise, it might be left around on the document if we throw for some reason.
-            yield self.clone_to_elements(cloned_nodes[0])
+            yield nodes_to_elements(iterate_nodes(cloned_nodes[0]))
         finally:
             # Remove the "manually cloned" tree.
             for cloned_node in cloned_nodes:
@@ -163,7 +166,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 +248,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
diff --git a/lib/elements/utils.py b/lib/elements/utils.py
index a340f838..79d5ac93 100644
--- a/lib/elements/utils.py
+++ b/lib/elements/utils.py
@@ -3,9 +3,15 @@
 # Copyright (c) 2010 Authors
 # Licensed under the GNU GPL version 3.0 or later.  See the file LICENSE for details.
 
-from ..commands import is_command
+from lxml.etree import Comment
+from typing import List, Optional
+from inkex import BaseElement
+
+from ..commands import is_command, layer_commands
 from ..marker import has_marker
-from ..svg.tags import EMBROIDERABLE_TAGS, SVG_IMAGE_TAG, SVG_TEXT_TAG
+from ..svg.tags import (CONNECTOR_TYPE, EMBROIDERABLE_TAGS, INKSCAPE_GROUPMODE,
+                        NOT_EMBROIDERABLE_TAGS, SVG_CLIPPATH_TAG, SVG_DEFS_TAG,
+                        SVG_GROUP_TAG, SVG_MASK_TAG, SVG_IMAGE_TAG, SVG_TEXT_TAG)
 from .clone import Clone, is_clone
 from .element import EmbroideryElement
 from .empty_d_object import EmptyDObject
@@ -17,7 +23,7 @@ from .stroke import Stroke
 from .text import TextObject
 
 
-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)]
@@ -59,3 +65,60 @@ def nodes_to_elements(nodes):
         elements.extend(node_to_elements(node))
 
     return elements
+
+
+def iterate_nodes(node: BaseElement,  # noqa: C901
+                  selection: Optional[List[BaseElement]] = None,
+                  troubleshoot=False) -> List[BaseElement]:
+    # Postorder traversal of selected nodes and their descendants.
+    # Returns all nodes if there is no selection.
+    def walk(node: BaseElement, selected: bool) -> List[BaseElement]:
+        nodes = []
+
+        if node.tag == Comment:
+            return []
+
+        element = EmbroideryElement(node)
+
+        if element.has_command('ignore_object'):
+            return []
+
+        if node.tag == SVG_GROUP_TAG and node.get(INKSCAPE_GROUPMODE) == "layer":
+            if len(list(layer_commands(node, "ignore_layer"))):
+                return []
+
+        if (node.tag in EMBROIDERABLE_TAGS or node.tag == SVG_GROUP_TAG) and element.get_style('display', 'inline') is None:
+            return []
+
+        # defs, masks and clippaths can contain embroiderable elements
+        # but should never be rendered directly.
+        if node.tag in [SVG_DEFS_TAG, SVG_MASK_TAG, SVG_CLIPPATH_TAG]:
+            return []
+
+        # command connectors with a fill color set, will glitch into the elements list
+        if is_command(node) or node.get(CONNECTOR_TYPE):
+            return []
+
+        if not selected:
+            if selection:
+                if node in selection:
+                    selected = True
+            else:
+                # if the user didn't select anything that means we process everything
+                selected = True
+
+        for child in node:
+            nodes.extend(walk(child, selected))
+
+        if selected:
+            if node.tag == SVG_GROUP_TAG:
+                pass
+            elif (node.tag in EMBROIDERABLE_TAGS or is_clone(node)) and not has_marker(node):
+                nodes.append(node)
+            # add images, text and elements with a marker for the troubleshoot extension
+            elif troubleshoot and (node.tag in NOT_EMBROIDERABLE_TAGS or has_marker(node)):
+                nodes.append(node)
+
+        return nodes
+
+    return walk(node, False)
diff --git a/lib/extensions/base.py b/lib/extensions/base.py
index 4d5aacfe..5f840417 100644
--- a/lib/extensions/base.py
+++ b/lib/extensions/base.py
@@ -6,18 +6,12 @@
 import os
 
 import inkex
-from lxml.etree import Comment
 
-from ..commands import is_command, layer_commands
-from ..elements import EmbroideryElement, nodes_to_elements
-from ..elements.clone import is_clone
 from ..i18n import _
-from ..marker import has_marker
 from ..metadata import InkStitchMetadata
 from ..svg import generate_unique_id
-from ..svg.tags import (CONNECTOR_TYPE, EMBROIDERABLE_TAGS, INKSCAPE_GROUPMODE,
-                        NOT_EMBROIDERABLE_TAGS, SVG_CLIPPATH_TAG, SVG_DEFS_TAG,
-                        SVG_GROUP_TAG, SVG_MASK_TAG)
+from ..svg.tags import INKSCAPE_GROUPMODE, SVG_GROUP_TAG
+from ..elements.utils import iterate_nodes, nodes_to_elements
 from ..update import update_inkstitch_document
 
 
@@ -62,58 +56,15 @@ class InkstitchExtension(inkex.EffectExtension):
 
         inkex.errormsg(_("Tip: Run Extensions > Ink/Stitch > Troubleshoot > Troubleshoot Objects") + "\n")
 
-    def descendants(self, node, selected=False, troubleshoot=False):  # noqa: C901
-        nodes = []
-
-        if node.tag == Comment:
-            return []
-
-        element = EmbroideryElement(node)
-
-        if element.has_command('ignore_object'):
-            return []
-
-        if node.tag == SVG_GROUP_TAG and node.get(INKSCAPE_GROUPMODE) == "layer":
-            if len(list(layer_commands(node, "ignore_layer"))):
-                return []
-
-        if (node.tag in EMBROIDERABLE_TAGS or node.tag == SVG_GROUP_TAG) and element.get_style('display', 'inline') is None:
-            return []
-
-        # defs, masks and clippaths can contain embroiderable elements
-        # but should never be rendered directly.
-        if node.tag in [SVG_DEFS_TAG, SVG_MASK_TAG, SVG_CLIPPATH_TAG]:
-            return []
-
-        # command connectors with a fill color set, will glitch into the elements list
-        if is_command(node) or node.get(CONNECTOR_TYPE):
-            return []
-
-        if self.svg.selection:
-            if node in list(self.svg.selection):
-                selected = True
-        else:
-            # if the user didn't select anything that means we process everything
-            selected = True
-
-        for child in node:
-            nodes.extend(self.descendants(child, selected, troubleshoot))
-
-        if selected:
-            if node.tag == SVG_GROUP_TAG:
-                pass
-            elif (node.tag in EMBROIDERABLE_TAGS or is_clone(node)) and not has_marker(node):
-                nodes.append(node)
-            # add images, text and elements with a marker for the troubleshoot extension
-            elif troubleshoot and (node.tag in NOT_EMBROIDERABLE_TAGS or has_marker(node)):
-                nodes.append(node)
-
-        return nodes
-
     def get_nodes(self, troubleshoot=False):
         # Postorder traversal of selected nodes and their descendants.
         # Returns all nodes if there is no selection.
-        return self.descendants(self.document.getroot(), troubleshoot=troubleshoot)
+        if self.svg.selection:
+            selection = list(self.svg.selection)
+        else:
+            selection = None
+
+        return iterate_nodes(self.document.getroot(), selection=selection, troubleshoot=troubleshoot)
 
     def get_elements(self, troubleshoot=False):
         self.elements = nodes_to_elements(self.get_nodes(troubleshoot))