From 8cfd9b2e927508d8e3a1f93114cca88fe35efa5a Mon Sep 17 00:00:00 2001 From: CapellanCitizen Date: Sun, 25 Aug 2024 22:51:31 -0400 Subject: [PATCH] Fixed hidden objects being stitched out when cloned (Fix #3167) --- lib/elements/clone.py | 17 ++++-------- lib/elements/utils.py | 26 ++++++++++++++++++- tests/test_clone.py | 1 + tests/test_elements_utils.py | 50 ++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 tests/test_elements_utils.py diff --git a/lib/elements/clone.py b/lib/elements/clone.py index ce9c2162d..cf3e0c329 100644 --- a/lib/elements/clone.py +++ b/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 diff --git a/lib/elements/utils.py b/lib/elements/utils.py index a340f838b..0318797ca 100644 --- a/lib/elements/utils.py +++ b/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: diff --git a/tests/test_clone.py b/tests/test_clone.py index 1c84f3e9f..ba3aa3aa6 100644 --- a/tests/test_clone.py +++ b/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() diff --git a/tests/test_elements_utils.py b/tests/test_elements_utils.py new file mode 100644 index 000000000..bbedfb72b --- /dev/null +++ b/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)