From 2bbebe56fdcf3fb351df1c636d349946e548e505 Mon Sep 17 00:00:00 2001 From: capellancitizen Date: Thu, 28 Mar 2024 17:21:42 -0400 Subject: [PATCH] Fixed clones of group elements not appearing. (#2766) --- bin/style-check | 2 +- lib/elements/clone.py | 217 +++++++++++++++----------- lib/elements/element.py | 4 +- tests/__init__.py | 0 tests/test_clone.py | 337 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 469 insertions(+), 91 deletions(-) create mode 100644 tests/__init__.py create mode 100644 tests/test_clone.py diff --git a/bin/style-check b/bin/style-check index bafd6e74e..81d237c40 100755 --- a/bin/style-check +++ b/bin/style-check @@ -5,4 +5,4 @@ # Instead of files, "--diff" may be passed to check only the lines changed # by a diff piped to standard input. -flake8 --count --max-complexity=10 --max-line-length=150 --statistics --exclude=pyembroidery,__init__.py,electron,build,src,dist "${@:-.}" +flake8 --count --max-complexity=10 --max-line-length=150 --statistics --exclude=pyembroidery,__init__.py,electron,build,src,dist,./*-metadata.py,./pyembroidery-format-descriptions.py "${@:-.}" diff --git a/lib/elements/clone.py b/lib/elements/clone.py index fdcd68355..6f667836f 100644 --- a/lib/elements/clone.py +++ b/lib/elements/clone.py @@ -3,19 +3,24 @@ # Copyright (c) 2010 Authors # Licensed under the GNU GPL version 3.0 or later. See the file LICENSE for details. -from math import atan2, degrees, radians +from math import degrees +from copy import deepcopy +from contextlib import contextmanager +from typing import Generator, List, Tuple -from inkex import CubicSuperPath, Path, Transform +from inkex import Transform, BaseElement from shapely import MultiLineString +from ..stitch_plan.stitch_group import StitchGroup + from ..commands import is_command_symbol from ..i18n import _ from ..svg.path import get_node_transform from ..svg.tags import (EMBROIDERABLE_TAGS, INKSTITCH_ATTRIBS, SVG_USE_TAG, - XLINK_HREF) + XLINK_HREF, SVG_GROUP_TAG) from ..utils import cache from .element import EmbroideryElement, param -from .validation import ObjectTypeWarning, ValidationWarning +from .validation import ValidationWarning class CloneWarning(ValidationWarning): @@ -29,21 +34,7 @@ class CloneWarning(ValidationWarning): ] -class CloneSourceWarning(ObjectTypeWarning): - name = _("Clone is not embroiderable") - description = _("There are one ore more clone objects in this document. A clone must be a direct child of an embroiderable element. " - "Ink/Stitch cannot embroider clones of groups or other not embroiderable elements (text or image).") - steps_to_solve = [ - _("Convert the clone into a real element:"), - _("* Select the clone."), - _("* Run: Edit > Clone > Unlink Clone (Alt+Shift+D)") - ] - - class Clone(EmbroideryElement): - # A clone embroidery element is linked to an embroiderable element. - # It will be ignored if the source element is not a direct child of the xlink attribute. - element_name = "Clone" def __init__(self, *args, **kwargs): @@ -62,86 +53,125 @@ class Clone(EmbroideryElement): type='float') @cache def clone_fill_angle(self): - return self.get_float_param('angle') or None + return self.get_float_param('angle') @property @param('flip_angle', _('Flip angle'), - tooltip=_("Flip automatically calucalted angle if it appears to be wrong."), + tooltip=_( + "Flip automatically calculated angle if it appears to be wrong."), type='boolean') @cache def flip_angle(self): - return self.get_boolean_param('flip_angle') + return self.get_boolean_param('flip_angle', False) def get_cache_key_data(self, previous_stitch): - source_node = get_clone_source(self.node) - source_elements = self.clone_to_element(source_node) + source_node = self.node.href + source_elements = self.clone_to_elements(source_node) return [element.get_cache_key(previous_stitch) for element in source_elements] - def clone_to_element(self, node): + def clone_to_elements(self, node): from .utils import node_to_elements - return node_to_elements(node, True) + 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 - def to_stitch_groups(self, last_patch=None): - patches = [] + def to_stitch_groups(self, last_patch=None) -> List[StitchGroup]: + with self.clone_elements() as elements: + patches = [] - source_node = get_clone_source(self.node) - if source_node.tag not in EMBROIDERABLE_TAGS: - return [] + for element in elements: + stitch_groups = element.to_stitch_groups(last_patch) + if len(stitch_groups): + last_patch = stitch_groups[-1] + patches.extend(stitch_groups) - old_transform = source_node.get('transform', '') - source_transform = source_node.composed_transform() - source_path = Path(source_node.get_path()).transform(source_transform) - transform = Transform(source_node.get('transform', '')) @ -source_transform - transform @= self.node.composed_transform() @ Transform(source_node.get('transform', '')) - source_node.set('transform', transform) + return patches - old_angle = float(source_node.get(INKSTITCH_ATTRIBS['angle'], 0)) - if self.clone_fill_angle is None: - rot = transform.add_rotate(-old_angle) - angle = self._get_rotation(rot, source_node, source_path) - if angle is not None: - source_node.set(INKSTITCH_ATTRIBS['angle'], angle) - else: - source_node.set(INKSTITCH_ATTRIBS['angle'], self.clone_fill_angle) + @contextmanager + def clone_elements(self) -> Generator[List[EmbroideryElement], None, None]: + """ + A context manager method which yields a set of elements representing the cloned element(s) href'd by this clone's element. + Cleans up after itself afterwards. + This is broken out from to_stitch_groups for testing convenience, primarily. + 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) + """ + source_node, local_transform = get_concrete_source(self.node) - elements = self.clone_to_element(source_node) - for element in elements: - stitch_groups = element.to_stitch_groups(last_patch) - patches.extend(stitch_groups) + if source_node.tag not in EMBROIDERABLE_TAGS and source_node.tag != SVG_GROUP_TAG: + yield [] + return - source_node.set('transform', old_transform) - source_node.set(INKSTITCH_ATTRIBS['angle'], old_angle) - return patches - - def _get_rotation(self, transform, source_node, source_path): + # Effectively, manually clone the href'd element: Place it into the tree at the same location + # as the use element this Clone represents, with the same transform + parent: BaseElement = self.node.getparent() + cloned_node = deepcopy(source_node) + cloned_node.set('transform', local_transform) + parent.add(cloned_node) try: - rotation = transform.rotation_degrees() - except ValueError: - source_path = CubicSuperPath(source_path)[0] - clone_path = Path(source_node.get_path()).transform(source_node.composed_transform()) - clone_path = CubicSuperPath(clone_path)[0] + # 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. + self.resolve_all_clones(cloned_node) - angle_source = atan2(source_path[1][1][1] - source_path[0][1][1], source_path[1][1][0] - source_path[0][1][0]) - angle_clone = atan2(clone_path[1][1][1] - clone_path[0][1][1], clone_path[1][1][0] - clone_path[0][1][0]) - angle_embroidery = radians(-float(source_node.get(INKSTITCH_ATTRIBS['angle'], 0))) + source_parent_transform = source_node.getparent().composed_transform() + clone_transform = cloned_node.composed_transform() + global_transform = clone_transform @ -source_parent_transform + self.apply_angles(cloned_node, global_transform) - diff = angle_source - angle_embroidery - rotation = degrees(diff + angle_clone) + yield self.clone_to_elements(cloned_node) + finally: + # Remove the "manually cloned" tree. + parent.remove(cloned_node) + + def resolve_all_clones(self, node: BaseElement) -> None: + """ + For a subtree, recursively replace all `use` tags with the elements they href. + """ + clones: List[BaseElement] = [n for n in node.iterdescendants() if n.tag == SVG_USE_TAG] + for clone in clones: + parent: BaseElement = clone.getparent() + source_node, local_transform = get_concrete_source(clone) + cloned_node = deepcopy(source_node) + parent.add(cloned_node) + cloned_node.set('transform', local_transform) + parent.remove(clone) + self.resolve_all_clones(cloned_node) + self.apply_angles(cloned_node, local_transform) + + def apply_angles(self, cloned_node: BaseElement, transform: Transform) -> None: + """ + Adjust angles on a cloned tree based on their transform. + """ + if self.clone_fill_angle is None: + # Strip out the translation component to simplify the fill vector rotation angle calculation. + angle_transform = Transform((transform.a, transform.b, transform.c, transform.d, 0.0, 0.0)) + + elements = self.clone_to_elements(cloned_node) + for element in elements: + # We manipulate the element's node directly here instead of using get/set param methods, because otherwise + # we may run into issues due to those methods' use of caching not updating if the underlying param value is changed. + + # Normally, rotate the cloned element's angle by the clone's rotation. + if self.clone_fill_angle is None: + element_angle = float(element.node.get(INKSTITCH_ATTRIBS['angle'], 0)) + # We have to negate the angle because SVG/Inkscape's definition of rotation is clockwise, while Inkstitch uses counter-clockwise + fill_vector = (angle_transform @ Transform(f"rotate(${-element_angle})")).apply_to_point((1, 0)) + # Same reason for negation here. + element_angle = -degrees(fill_vector.angle) + else: # If clone_fill_angle is specified, override the angle instead. + element_angle = self.clone_fill_angle if self.flip_angle: - rotation = -degrees(diff - angle_clone) + element_angle = -element_angle - return -rotation + element.node.set(INKSTITCH_ATTRIBS['angle'], element_angle) - def get_clone_style(self, style_name, node, default=None): - style = node.style[style_name] or default - return style - - def center(self, source_node): - transform = get_node_transform(self.node.getparent()) - center = self.node.bounding_box(transform).center - return center + return elements @property def shape(self): @@ -151,14 +181,15 @@ class Clone(EmbroideryElement): path = path.to_superpath() return MultiLineString(path) + def center(self, source_node): + transform = get_node_transform(self.node.getparent()) + center = self.node.bounding_box(transform).center + return center + def validation_warnings(self): - source_node = get_clone_source(self.node) - if source_node.tag not in EMBROIDERABLE_TAGS: - point = self.center(source_node) - yield CloneSourceWarning(point) - else: - point = self.center(source_node) - yield CloneWarning(point) + source_node = self.node.href + point = self.center(source_node) + yield CloneWarning(point) def is_clone(node): @@ -167,11 +198,21 @@ def is_clone(node): return False -def is_embroiderable_clone(node): - if is_clone(node) and get_clone_source(node).tag in EMBROIDERABLE_TAGS: - return True - return False - - -def get_clone_source(node): - return node.href +def get_concrete_source(node: BaseElement) -> Tuple[BaseElement, Transform]: + """ + Given a use element, follow hrefs until finding an element that is not a use. + Returns that non-use element, and a transform to apply to a copy of that element + which will place that copy in the same position as the use if added as a sibling of the use. + """ + # Compute the transform that will be applied to the cloned element, which is based off of the cloned element. + # This makes intuitive sense: The clone of a scaled element will also be scaled, the clone of a rotated element will also + # be rotated, etc. Any transforms from the use element will be applied on top of that. + transform = Transform(node.get('transform')) + source_node: BaseElement = node.href + while source_node.tag == SVG_USE_TAG: + # In case the source_node href's a use (and that href's a use...), iterate up the chain until we get a source node, + # applying the transforms as we go. + transform @= Transform(source_node.get('transform')) + source_node = source_node.href + transform @= Transform(source_node.get('transform')) + return (source_node, transform) diff --git a/lib/elements/element.py b/lib/elements/element.py index 6c2fa15ab..071f8a90f 100644 --- a/lib/elements/element.py +++ b/lib/elements/element.py @@ -8,7 +8,7 @@ from copy import deepcopy import inkex import numpy as np -from inkex import bezier +from inkex import bezier, BaseElement from ..commands import find_commands from ..debug import debug @@ -58,7 +58,7 @@ def param(*args, **kwargs): class EmbroideryElement(object): - def __init__(self, node): + def __init__(self, node: BaseElement): self.node = node @property diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/test_clone.py b/tests/test_clone.py new file mode 100644 index 000000000..4a920e9e7 --- /dev/null +++ b/tests/test_clone.py @@ -0,0 +1,337 @@ +from lib.elements import Clone, EmbroideryElement +from lib.svg.tags import INKSTITCH_ATTRIBS, SVG_RECT_TAG +from inkex import SvgDocumentElement, Rectangle, Circle, Group, Use, Transform, TextElement +from inkex.tester import TestCase +from inkex.tester.svg import svg + +from typing import Optional + +from math import sqrt + + +def element_fill_angle(element: EmbroideryElement) -> Optional[float]: + angle = element.node.get(INKSTITCH_ATTRIBS['angle']) + if angle is not None: + angle = float(angle) + return angle + + +class CloneElementTest(TestCase): + def assertAngleAlmostEqual(self, a, b): + # Take the mod 180 of the returned angles, because e.g. -130deg and 50deg produce fills along the same angle. + # We have to use a precision of 4 decimal digits because of the precision of the matrices as they are stored in the svg trees + # generated by these tests. + self.assertAlmostEqual(a % 180, b % 180, 4) + + def test_not_embroiderable(self): + root: SvgDocumentElement = svg() + text = root.add(TextElement()) + text.text = "Can't embroider this!" + use = root.add(Use()) + use.href = text + + clone = Clone(use) + stitch_groups = clone.to_stitch_groups(None) + self.assertEqual(len(stitch_groups), 0) + + # These tests make sure the element cloning works as expected, using the `clone_elements` method. + + def test_basic(self): + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + use = root.add(Use()) + use.href = rect + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + self.assertAlmostEqual(element_fill_angle(elements[0]), 30) + + def test_angle_rotated(self): + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + use = root.add(Use()) + use.href = rect + use.set('transform', Transform().add_rotate(20)) + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + self.assertAngleAlmostEqual(element_fill_angle(elements[0]), 10) + + def test_angle_flipped(self): + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + use = root.add(Use()) + use.href = rect + use.set('transform', Transform().add_scale(-1, 1)) + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + self.assertAngleAlmostEqual(element_fill_angle(elements[0]), -30) + + def test_angle_flipped_rotated(self): + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + use = root.add(Use()) + use.href = rect + use.set('transform', Transform().add_rotate(20).add_scale(-1, 1)) + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + # Fill angle goes from 30 -> -30 after flip -> -50 after rotate + self.assertAngleAlmostEqual(element_fill_angle(elements[0]), -50) + + def test_angle_non_uniform_scale(self): + """ + The angle isn't *as* well-defined for non-rotational scales, but we try to follow how the slope will be altered. + """ + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + use = root.add(Use()) + use.href = rect + use.set('transform', Transform().add_rotate(10).add_scale(1, -sqrt(3))) + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + # Slope of the stitching goes from tan(30deg) = 1/sqrt(3) to -sqrt(3)/sqrt(3) = tan(-45deg), + # then rotated another -10 degrees to -55 + self.assertAngleAlmostEqual(element_fill_angle(elements[0]), -55) + + def test_angle_inherits_down_tree(self): + """ + The stitching angle of a clone is based in part on the relative transforms of the source and clone. + """ + root: SvgDocumentElement = svg() + g1 = root.add(Group()) + g1.set('transform', Transform().add_rotate(3)) + rect = g1.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + g2 = root.add(Group()) + g2.set('transform', Transform().add_translate((20, 0)).add_rotate(-7)) + use = g2.add(Use()) + use.href = rect + use.set('transform', Transform().add_rotate(11)) + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + # Angle goes from 30 -> 40 (g1 -> g2) -> 29 (use) + self.assertAngleAlmostEqual(element_fill_angle(elements[0]), 29) + + def test_transform_inherits_from_cloned_element(self): + """ + Elements cloned by cloned_elements need to inherit their transform from their href'd element and their use to match what's shown. + """ + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30", + })) + rect.set('transform', Transform().add_scale(2, 2)) + use = root.add(Use()) + use.href = rect + use.set('transform', Transform().add_translate((5, 10))) + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + self.assertTransformEqual( + elements[0].node.composed_transform(), + Transform().add_translate((5, 10)).add_scale(2, 2)) + + def test_transform_inherits_from_tree(self): + root: SvgDocumentElement = svg() + g1 = root.add(Group()) + g1.set('transform', Transform().add_translate((0, 5)).add_rotate(5)) + rect = g1.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30", + })) + rect.set('transform', Transform().add_scale(2, 2)) + use = root.add(Use()) + use.href = g1 + use.set('transform', Transform().add_translate((5, 10))) + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + self.assertTransformEqual( + elements[0].node.composed_transform(), + Transform().add_translate((5, 10)) # use + .add_translate((0, 5)).add_rotate(5) # g1 + .add_scale(2, 2), # rect + 5) + + def test_transform_inherits_from_tree_up_tree(self): + root: SvgDocumentElement = svg() + g1 = root.add(Group()) + g1.set('transform', Transform().add_translate((0, 5)).add_rotate(5)) + rect = g1.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30", + })) + rect.set('transform', Transform().add_scale(2, 2)) + circ = g1.add(Circle()) + circ.radius = 5 + g2 = root.add(Group()) + g2.set('transform', Transform().add_translate((1, 2)).add_scale(0.5, 1)) + use = g2.add(Use()) + use.href = g1 + use.set('transform', Transform().add_translate((5, 10))) + + clone = Clone(use) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 2) + self.assertTransformEqual( + elements[0].node.composed_transform(), + Transform().add_translate((1, 2)).add_scale(0.5, 1) # g2 + .add_translate((5, 10)) # use + .add_translate((0, 5)).add_rotate(5) # g1 + .add_scale(2, 2), # rect + 5) + self.assertTransformEqual( + elements[1].node.composed_transform(), + Transform().add_translate((1, 2)).add_scale(0.5, 1) # g2 + .add_translate((5, 10)) # use + .add_translate((0, 5)).add_rotate(5), # g1 + 5) + + def test_clone_fill_angle_not_specified(self): + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + use = root.add(Use()) + use.href = rect + use.set('transform', Transform().add_rotate(20)) + + clone = Clone(use) + self.assertEqual(clone.clone_fill_angle, None) + + def test_clone_fill_angle(self): + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + use = root.add(Use()) + use.href = rect + use.set(INKSTITCH_ATTRIBS["angle"], 42) + use.set('transform', Transform().add_rotate(20)) + + clone = Clone(use) + self.assertEqual(clone.clone_fill_angle, 42) + + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + self.assertAngleAlmostEqual(element_fill_angle(elements[0]), 42) + + def test_angle_manually_flipped(self): + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + use = root.add(Use()) + use.href = rect + use.set('transform', Transform().add_rotate(20)) + use.set(INKSTITCH_ATTRIBS["flip_angle"], True) + + clone = Clone(use) + self.assertTrue(clone.flip_angle) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + self.assertAngleAlmostEqual(element_fill_angle(elements[0]), -10) + + def test_recursive_uses(self): + root: SvgDocumentElement = svg() + g1 = root.add(Group()) + rect = g1.add(Rectangle(attrib={ + "width": "10", + "height": "10", + })) + u1 = g1.add(Use()) + u1.set('transform', Transform().add_translate((20, 0))) + u1.href = rect + u2 = root.add(Use()) + u2.set('transform', Transform().add_translate((0, 20)).add_scale(0.5, 0.5)) + u2.href = g1 + u3 = root.add(Use()) + u3.set('transform', Transform().add_translate((0, 30))) + u3.href = u2 + + clone = Clone(u3) + with clone.clone_elements() as elements: + # There should be two elements cloned from u3, two rects, one corresponding to rect and one corresponding to u1. + # Their transforms should derive from the elements they href. + self.assertEqual(len(elements), 2) + self.assertEqual(elements[0].node.tag, SVG_RECT_TAG) + self.assertTransformEqual(elements[0].node.composed_transform(), + Transform().add_translate((0, 30)) # u3 + .add_translate(0, 20).add_scale(0.5, 0.5) # u2 + ) + + self.assertEqual(elements[1].node.tag, SVG_RECT_TAG) + self.assertTransformEqual(elements[1].node.composed_transform(), + Transform().add_translate((0, 30)) # u3 + .add_translate((0, 20)).add_scale(0.5, 0.5) # u2 + .add_translate((20, 0)) # u1 + ) + + def test_recursive_uses_angle(self): + root: SvgDocumentElement = svg() + rect = root.add(Rectangle(attrib={ + "width": "10", + "height": "10", + INKSTITCH_ATTRIBS["angle"]: "30" + })) + u1 = root.add(Use()) + u1.set('transform', Transform().add_rotate(60)) + u1.href = rect + g = root.add(Group()) + g.set('transform', Transform().add_rotate(-10)) + u2 = g.add(Use()) + u2.href = u1 + u3 = root.add(Use()) + u3.set('transform', Transform().add_rotate(7)) + u3.href = g + + clone = Clone(u3) + with clone.clone_elements() as elements: + self.assertEqual(len(elements), 1) + # Angle goes from 30 -> -30 (u1) -> -20 (g -> u2) -> -27 (u3) + self.assertAngleAlmostEqual(element_fill_angle(elements[0]), -27)