This and that (#1727)

* dont fail on satin with fill

* fill stitch error message

* convert to satin mac issue

* auto_satin: add rung for two node old style satins

* avoid divide by zero in intersect_region_with_grating

* fix for incorrect stagger in guided fill

* better rail sectioning algorithm

* fix #1780

* fix #1816

Co-authored-by: Lex Neva

10 files changed, 101 insertions, 118 deletions

diff --git a/lib/elements/empty_d_object.py b/lib/elements/empty_d_object.py
index 8f3b08de..6f010488 100644
--- a/lib/elements/empty_d_object.py
+++ b/lib/elements/empty_d_object.py
@@ -10,8 +10,8 @@ from .validation import ObjectTypeWarning
 
 
 class EmptyD(ObjectTypeWarning):
-    name = _("Empty D-Attribute")
-    description = _("There is an invalid path object in the document, the d-attribute is missing.")
+    name = _("Empty Path")
+    description = _("There is an invalid object in the document without geometry information.")
     steps_to_solve = [
         _('* Run Extensions > Ink/Stitch > Troubleshoot > Cleanup Document...')
     ]
diff --git a/lib/elements/fill_stitch.py b/lib/elements/fill_stitch.py
index a5248952..173868d4 100644
--- a/lib/elements/fill_stitch.py
+++ b/lib/elements/fill_stitch.py
@@ -272,6 +272,8 @@ class FillStitch(EmbroideryElement):
             return shgeo.MultiPolygon([valid_shape])
         if isinstance(valid_shape, shgeo.LineString):
             return shgeo.MultiPolygon([])
+        if shape.area == 0:
+            return shgeo.MultiPolygon([])
 
         polygons = []
         for polygon in valid_shape.geoms:
@@ -713,12 +715,17 @@ class FillStitch(EmbroideryElement):
 
         # for an uncaught exception, give a little more info so that they can create a bug report
         message = ""
-        message += _("Error during autofill!  This means that there is a problem with Ink/Stitch.")
+        message += _("Error during autofill! This means it is a bug in Ink/Stitch.")
         message += "\n\n"
         # L10N this message is followed by a URL: https://github.com/inkstitch/inkstitch/issues/new
-        message += _("If you'd like to help us make Ink/Stitch better, please paste this whole message into a new issue at: ")
-        message += "https://github.com/inkstitch/inkstitch/issues/new\n\n"
-        message += version.get_inkstitch_version() + "\n\n"
+        message += _("If you'd like to help please\n"
+                     "- copy the entire error message below\n"
+                     "- save your SVG file and\n"
+                     "- create a new issue at")
+        message += " https://github.com/inkstitch/inkstitch/issues/new\n\n"
+        message += _("Include the error description and also (if possible) the svg file.")
+        message += '\n\n\n'
+        message += version.get_inkstitch_version() + '\n'
         message += traceback.format_exc()
 
         self.fatal(message)
diff --git a/lib/elements/satin_column.py b/lib/elements/satin_column.py
index e4d69361..886b3f85 100644
--- a/lib/elements/satin_column.py
+++ b/lib/elements/satin_column.py
@@ -7,7 +7,6 @@ from copy import deepcopy
 from itertools import chain
 import numpy as np
 
-from shapely import affinity as shaffinity
 from shapely import geometry as shgeo
 from shapely.ops import nearest_points
 
@@ -16,23 +15,11 @@ from inkex import paths
 from ..i18n import _
 from ..stitch_plan import StitchGroup
 from ..svg import line_strings_to_csp, point_lists_to_csp
-from ..utils import Point, cache, collapse_duplicate_point, cut
+from ..utils import Point, cache, cut, cut_multiple
 from .element import EmbroideryElement, param, PIXELS_PER_MM
 from .validation import ValidationError, ValidationWarning
 
 
-class SatinHasFillError(ValidationError):
-    name = _("Satin column has fill")
-    description = _("Satin column: Object has a fill (but should not)")
-    steps_to_solve = [
-        _("* Select this object."),
-        _("* Open the Fill and Stroke panel"),
-        _("* Open the Fill tab"),
-        _("* Disable the Fill"),
-        _("* Alternative: open Params and switch this path to Stroke to disable Satin Column mode")
-    ]
-
-
 class TooFewPathsError(ValidationError):
     name = _("Too few subpaths")
     description = _("Satin column: Object has too few subpaths.  A satin column should have at least two subpaths (the rails).")
@@ -338,24 +325,6 @@ class SatinColumn(EmbroideryElement):
     @cache
     def flattened_rungs(self):
         """The rungs, as LineStrings."""
-        rungs = []
-        for rung in self._raw_rungs:
-            # make sure each rung intersects both rails
-            if not rung.intersects(self.flattened_rails[0]) or not rung.intersects(self.flattened_rails[1]):
-                # the rung does not intersect both rails
-                # get nearest points on rungs
-                start = nearest_points(rung, self.flattened_rails[0])[1]
-                end = nearest_points(rung, self.flattened_rails[1])[1]
-                # extend from the nearest points just a little bit to make sure that we get an intersection
-                rung = shaffinity.scale(shgeo.LineString([start, end]), 1.01, 1.01)
-                rungs.append(rung)
-            else:
-                rungs.append(rung)
-        return tuple(rungs)
-
-    @property
-    @cache
-    def _raw_rungs(self):
         return tuple(shgeo.LineString(self.flatten_subpath(rung)) for rung in self.rungs)
 
     @property
@@ -379,21 +348,18 @@ class SatinColumn(EmbroideryElement):
         for rail in self.rails:
             points = self.strip_control_points(rail)
 
-            # ignore the start and end
-            points = points[1:-1]
+            if len(points) > 2:
+                # Don't bother putting rungs at the start and end.
+                points = points[1:-1]
+            else:
+                # But do include one at the start if we wouldn't add one otherwise.
+                # This avoids confusing other parts of the code.
+                points = points[:-1]
 
             rung_endpoints.append(points)
 
         rungs = []
         for start, end in zip(*rung_endpoints):
-            # Expand the points just a bit to ensure that shapely thinks they
-            # intersect with the rails even with floating point inaccuracy.
-            start = Point(*start)
-            end = Point(*end)
-            start, end = self.offset_points(start, end, (0.01, 0.01), (0, 0))
-            start = list(start)
-            end = list(end)
-
             rungs.append([[start, start, start], [end, end, end]])
 
         return rungs
@@ -446,39 +412,22 @@ class SatinColumn(EmbroideryElement):
             indices_by_length = sorted(list(range(num_paths)), key=lambda index: paths[index].length, reverse=True)
             return indices_by_length[:2]
 
-    def _cut_rail(self, rail, rung):
-        for segment_index, rail_segment in enumerate(rail[:]):
-            if rail_segment is None:
-                continue
-
-            intersection = rail_segment.intersection(rung)
-
-            # If there are duplicate points in a rung-less satin, then
-            # intersection will be a GeometryCollection of multiple copies
-            # of the same point.  This reduces it that to a single point.
-            intersection = collapse_duplicate_point(intersection)
-
-            if not intersection.is_empty:
-                cut_result = cut(rail_segment, rail_segment.project(intersection))
-                rail[segment_index:segment_index + 1] = cut_result
-
-                if cut_result[1] is None:
-                    # if we were exactly at the end of one of the existing rail segments,
-                    # stop here or we'll get a spurious second intersection on the next
-                    # segment
-                    break
-
     @property
     @cache
     def flattened_sections(self):
         """Flatten the rails, cut with the rungs, and return the sections in pairs."""
 
-        rails = [[rail] for rail in self.flattened_rails]
+        rails = list(self.flattened_rails)
         rungs = self.flattened_rungs
 
-        for rung in rungs:
-            for rail in rails:
-                self._cut_rail(rail, rung)
+        for i, rail in enumerate(rails):
+            cut_points = []
+
+            for rung in rungs:
+                point_on_rung, point_on_rail = nearest_points(rung, rail)
+                cut_points.append(rail.project(point_on_rail))
+
+            rails[i] = cut_multiple(rail, cut_points)
 
         for rail in rails:
             for i in range(len(rail)):
@@ -502,19 +451,15 @@ class SatinColumn(EmbroideryElement):
         return sections
 
     def validation_warnings(self):
-        for rung in self._raw_rungs:
+        for rung in self.flattened_rungs:
             for rail in self.flattened_rails:
                 intersection = rung.intersection(rail)
                 if intersection.is_empty:
                     yield DanglingRungWarning(rung.interpolate(0.5, normalized=True))
 
     def validation_errors(self):
-        # The node should have exactly two paths with no fill.  Each
-        # path should have the same number of points, meaning that they
-        # will both be made up of the same number of bezier curves.
-
-        if self.get_style("fill") is not None:
-            yield SatinHasFillError(self.shape.centroid)
+        # The node should have exactly two paths with the same number of points - or it should
+        # have two rails and at least one rung
 
         if len(self.rails) < 2:
             yield TooFewPathsError(self.shape.centroid)
@@ -522,7 +467,7 @@ class SatinColumn(EmbroideryElement):
             if len(self.rails[0]) != len(self.rails[1]):
                 yield UnequalPointsError(self.flattened_rails[0].interpolate(0.5, normalized=True))
         else:
-            for rung in self._raw_rungs:
+            for rung in self.flattened_rungs:
                 for rail in self.flattened_rails:
                     intersection = rung.intersection(rail)
                     if not intersection.is_empty and not isinstance(intersection, shgeo.Point):
@@ -664,14 +609,10 @@ class SatinColumn(EmbroideryElement):
 
         for path_list in path_lists:
             if len(path_list) in (2, 4):
-                # Add the rung just after the start of the satin.
-                rung_start = path_list[0].interpolate(0.1)
-                rung_end = path_list[1].interpolate(0.1)
+                # Add the rung at the start of the satin.
+                rung_start = path_list[0].coords[0]
+                rung_end = path_list[1].coords[0]
                 rung = shgeo.LineString((rung_start, rung_end))
-
-                # make it a bit bigger so that it definitely intersects
-                rung = shaffinity.scale(rung, 1.1, 1.1)
-
                 path_list.append(rung)
 
     def _path_list_to_satins(self, path_list):
diff --git a/lib/elements/utils.py b/lib/elements/utils.py
index dafde759..f7ee8dbc 100644
--- a/lib/elements/utils.py
+++ b/lib/elements/utils.py
@@ -6,11 +6,11 @@
 from ..commands import is_command
 from ..marker import has_marker
 from ..svg.tags import (EMBROIDERABLE_TAGS, SVG_IMAGE_TAG, SVG_PATH_TAG,
-                        SVG_POLYLINE_TAG, SVG_TEXT_TAG)
-from .fill_stitch import FillStitch
+                        SVG_POLYGON_TAG, SVG_POLYLINE_TAG, SVG_TEXT_TAG)
 from .clone import Clone, is_clone
 from .element import EmbroideryElement
 from .empty_d_object import EmptyDObject
+from .fill_stitch import FillStitch
 from .image import ImageObject
 from .marker import MarkerObject
 from .polyline import Polyline
@@ -27,7 +27,8 @@ def node_to_elements(node, clone_to_element=False):  # noqa: C901
         # clone_to_element: get an actual embroiderable element once a clone has been defined as a clone
         return [Clone(node)]
 
-    elif node.tag == SVG_PATH_TAG and not node.get('d', ''):
+    elif ((node.tag == SVG_PATH_TAG and not node.get('d', None)) or
+          (node.tag in [SVG_POLYLINE_TAG, SVG_POLYGON_TAG] and not node.get('points', None))):
         return [EmptyDObject(node)]
 
     elif has_marker(node):
@@ -36,18 +37,17 @@ def node_to_elements(node, clone_to_element=False):  # noqa: C901
     elif node.tag in EMBROIDERABLE_TAGS or is_clone(node):
         element = EmbroideryElement(node)
 
-        if element.get_boolean_param("satin_column") and element.get_style("stroke"):
-            return [SatinColumn(node)]
-        else:
-            elements = []
-            if element.get_style("fill", "black") and not element.get_style('fill-opacity', 1) == "0":
-                elements.append(FillStitch(node))
-            if element.get_style("stroke"):
-                if not is_command(element.node):
-                    elements.append(Stroke(node))
-            if element.get_boolean_param("stroke_first", False):
-                elements.reverse()
-            return elements
+        elements = []
+        if element.get_style("fill", "black") and not element.get_style('fill-opacity', 1) == "0":
+            elements.append(FillStitch(node))
+        if element.get_style("stroke"):
+            if element.get_boolean_param("satin_column"):
+                elements.append(SatinColumn(node))
+            elif not is_command(element.node):
+                elements.append(Stroke(node))
+        if element.get_boolean_param("stroke_first", False):
+            elements.reverse()
+        return elements
 
     elif node.tag == SVG_IMAGE_TAG:
         return [ImageObject(node)]
diff --git a/lib/extensions/convert_to_satin.py b/lib/extensions/convert_to_satin.py
index 93850789..5512a095 100644
--- a/lib/extensions/convert_to_satin.py
+++ b/lib/extensions/convert_to_satin.py
@@ -129,11 +129,16 @@ class ConvertToSatin(InkstitchExtension):
         if Point(*path[0]).distance(Point(*path[-1])) < 1:
             raise SelfIntersectionError()
 
+        # Shapely is supposed to return right sided offsets in reversed direction, which it does, except for macOS.
+        # To avoid direction checking, we are going to rely on left side offsets only.
+        # Therefore we need to reverse the original path.
+        reversed_path = shgeo.LineString(reversed(path))
         path = shgeo.LineString(path)
+        distance = stroke_width / 2.0
 
         try:
-            left_rail = path.parallel_offset(stroke_width / 2.0, 'left', **style_args)
-            right_rail = path.parallel_offset(stroke_width / 2.0, 'right', **style_args)
+            left_rail = path.parallel_offset(distance, 'left', **style_args)
+            right_rail = reversed_path.parallel_offset(distance, 'left', **style_args)
         except ValueError:
             # TODO: fix this error automatically
             # Error reference: https://github.com/inkstitch/inkstitch/issues/964
@@ -149,7 +154,6 @@ class ConvertToSatin(InkstitchExtension):
             #   https://shapely.readthedocs.io/en/latest/manual.html#object.parallel_offset
             raise SelfIntersectionError()
 
-        # for whatever reason, shapely returns a right-side offset's coordinates in reverse
         left_rail = list(left_rail.coords)
         right_rail = list(reversed(right_rail.coords))
 
diff --git a/lib/stitches/auto_satin.py b/lib/stitches/auto_satin.py
index 93bffd57..6d792b4e 100644
--- a/lib/stitches/auto_satin.py
+++ b/lib/stitches/auto_satin.py
@@ -81,6 +81,8 @@ class SatinSegment(object):
 
         satin = satin.apply_transform()
 
+        _ensure_even_repeats(satin)
+
         return satin
 
     to_element = to_satin
@@ -507,7 +509,6 @@ def name_elements(new_elements, preserve_order):
     for element in new_elements:
         if isinstance(element, SatinColumn):
             element.node.set("id", generate_unique_id(element.node, "autosatin"))
-            _ensure_even_repeats(element)
         else:
             element.node.set("id", generate_unique_id(element.node, "autosatinrun"))
 
@@ -515,7 +516,6 @@ def name_elements(new_elements, preserve_order):
             if isinstance(element, SatinColumn):
                 # L10N Label for a satin column created by Auto-Route Satin Columns and Lettering extensions
                 element.node.set(INKSCAPE_LABEL, _("AutoSatin %d") % index)
-                _ensure_even_repeats(element)
             else:
                 # L10N Label for running stitch (underpathing) created by Auto-Route Satin Columns amd Lettering extensions
                 element.node.set(INKSCAPE_LABEL, _("AutoSatin Running Stitch %d") % index)
diff --git a/lib/stitches/fill.py b/lib/stitches/fill.py
index 60a0cb7d..11c9259b 100644
--- a/lib/stitches/fill.py
+++ b/lib/stitches/fill.py
@@ -125,6 +125,9 @@ def intersect_region_with_grating(shape, angle, row_spacing, end_row_spacing=Non
     end -= center.y
 
     height = abs(end - start)
+    if height == 0:
+        # return early to avoid divide-by-zero later
+        return []
 
     # print >> dbg, "grating:", start, end, height, row_spacing, end_row_spacing
 
diff --git a/lib/stitches/guided_fill.py b/lib/stitches/guided_fill.py
index 14c58b7a..a9906bfb 100644
--- a/lib/stitches/guided_fill.py
+++ b/lib/stitches/guided_fill.py
@@ -247,7 +247,7 @@ def intersect_region_with_grating_guideline(shape, line, row_spacing, num_stagge
 
         debug.log_line_string(offset_line, f"offset {row}")
 
-        stitched_line = apply_stitches(offset_line, max_stitch_length, num_staggers, row_spacing, row * direction)
+        stitched_line = apply_stitches(offset_line, max_stitch_length, num_staggers, row_spacing, row)
         intersection = shape.intersection(stitched_line)
 
         if shape_envelope.intersects(stitched_line):
diff --git a/lib/threads/color.py b/lib/threads/color.py
index 85c8948c..458752da 100644
--- a/lib/threads/color.py
+++ b/lib/threads/color.py
@@ -7,9 +7,10 @@ import colorsys
 import re
 
 import tinycss2.color3
-from inkex import Color
 from pyembroidery.EmbThread import EmbThread
 
+from inkex import Color
+
 
 class ThreadColor(object):
     hex_str_re = re.compile('#([0-9a-z]{3}|[0-9a-z]{6})', re.I)
diff --git a/lib/utils/geometry.py b/lib/utils/geometry.py
index 30457749..98f40709 100644
--- a/lib/utils/geometry.py
+++ b/lib/utils/geometry.py
@@ -39,6 +39,40 @@ def cut(line, distance, normalized=False):
                 LineString([(cp.x, cp.y)] + coords[i:])]
 
 
+def cut_multiple(line, distances, normalized=False):
+    """Cut a LineString at multiple distances along that line.
+
+    Always returns a list of N + 1 members, where N is the number of distances
+    provided.  Some members of the list may be None, indicating an empty
+    segment.  This can happen if one of the distances is at the start or end
+    of the line, or if duplicate distances are provided.
+
+    Returns:
+        a list of LineStrings or None values"""
+
+    distances = list(sorted(distances))
+
+    segments = [line]
+    distance_so_far = 0
+    nones = []
+
+    for distance in distances:
+        segment = segments.pop()
+        before, after = cut(segment, distance - distance_so_far, normalized)
+
+        segments.append(before)
+
+        if after is None:
+            nones.append(after)
+        else:
+            if before is not None:
+                distance_so_far += before.length
+            segments.append(after)
+
+    segments.extend(nones)
+    return segments
+
+
 def roll_linear_ring(ring, distance, normalized=False):
     """Make a linear ring start at a different point.
 
@@ -113,13 +147,6 @@ def cut_path(points, length):
     return [Point(*point) for point in subpath.coords]
 
 
-def collapse_duplicate_point(geometry):
-    if geometry.area < 0.01:
-        return geometry.representative_point()
-
-    return geometry
-
-
 class Point:
     def __init__(self, x, y):
         self.x = x