From eded9e9d3a6d1f0c42d1970057fb99bb7a9a4fa8 Mon Sep 17 00:00:00 2001
From: joostdecock <joost@joost.at>
Date: Wed, 12 Jun 2024 13:24:42 +0200
Subject: [PATCH] fix(core): Handle path splits on start or end points

In some edge cases, like when splitting a path on or very close to its
start or end point, `Path.split()` will return an array in which one of
the elements is not a Path object, but rather an empty array.

That is rather combersome to check for, so this changes that behaviour
and will return null for such cases.

I've also updated the documentation to clarify this behaviour.

Fixes #6816

This also fixes a bug when a path was being split on its end point.
---
 markdown/dev/reference/api/path/split/en.md | 22 +++++++++++++
 packages/core/src/path.mjs                  | 12 +++++--
 packages/core/tests/path.test.mjs           | 36 ++++++++++++++++++---
 3 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/markdown/dev/reference/api/path/split/en.md b/markdown/dev/reference/api/path/split/en.md
index 179ae5467b9..154bf9c5c3e 100644
--- a/markdown/dev/reference/api/path/split/en.md
+++ b/markdown/dev/reference/api/path/split/en.md
@@ -45,3 +45,25 @@ array path.split(Point splitPoint)
 }
 ```
 </Example>
+
+## Notes
+
+### The returned array will hold null for edge cases
+
+Typically, the returned array will hold a `Path` object for each half.
+But in some cases, one of the array entries can hold `null` if the split failed to find a path.
+For example because you are splitting a `Path` on its start or end point.
+
+```mjs
+// Return value for a normal case
+[Path, Path]
+// Return value when calling Path.split() on/near the path's start point
+[null, Path]
+// Return value when calling Path.split() on/near the path's end point
+[Path, null]
+```
+
+### This method will snap the split point to start or end points
+This method will also _snap_ to the start or end point if you are splitting a path
+(very) close to it, as it checks with [`Point.sitsRoughlyOn()`](/reference/api/point/sitsroughlyon).
+
diff --git a/packages/core/src/path.mjs b/packages/core/src/path.mjs
index 08a4972fce1..5341e3ab836 100644
--- a/packages/core/src/path.mjs
+++ b/packages/core/src/path.mjs
@@ -898,7 +898,12 @@ Path.prototype.split = function (point) {
       break
     }
     if (path.ops[1].type === 'line') {
-      if (pointOnLine(path.ops[0].to, path.ops[1].to, point)) {
+      if (path.ops[1].to.sitsRoughlyOn(point)) {
+        pi++
+        firstHalf = divided.slice(0, pi)
+        secondHalf = divided.slice(pi)
+        break
+      } else if (pointOnLine(path.ops[0].to, path.ops[1].to, point)) {
         firstHalf = divided.slice(0, pi)
         firstHalf.push(new Path().__withLog(this.log).move(path.ops[0].to).line(point))
         pi++
@@ -953,8 +958,9 @@ Path.prototype.split = function (point) {
     }
   }
 
-  if (firstHalf.length > 0) firstHalf = __joinPaths(firstHalf)
-  if (secondHalf.length > 0) secondHalf = __joinPaths(secondHalf)
+  firstHalf = firstHalf.length > 0 && firstHalf[0].ops.length > 1 ? __joinPaths(firstHalf) : null
+  secondHalf =
+    secondHalf.length > 0 && secondHalf[0].ops.length > 1 ? __joinPaths(secondHalf) : null
 
   return [firstHalf, secondHalf]
 }
diff --git a/packages/core/tests/path.test.mjs b/packages/core/tests/path.test.mjs
index 50562bd6f47..8eefcda4fd1 100644
--- a/packages/core/tests/path.test.mjs
+++ b/packages/core/tests/path.test.mjs
@@ -661,10 +661,10 @@ describe('Path', () => {
     const test = new Path().move(a).line(b).line(c)
 
     let halves = test.split(new Point(10.1, 29.9))
-    expect(halves[0].ops[1].to.x).to.equal(10.1)
-    expect(halves[0].ops[1].to.y).to.equal(29.9)
-    expect(halves[1].ops[0].to.x).to.equal(10.1)
-    expect(halves[1].ops[0].to.y).to.equal(29.9)
+    expect(halves[0].ops[1].to.x).to.equal(10)
+    expect(halves[0].ops[1].to.y).to.equal(30)
+    expect(halves[1].ops[0].to.x).to.equal(10)
+    expect(halves[1].ops[0].to.y).to.equal(30)
   })
 
   it('Should split a path on a curve joint', () => {
@@ -680,6 +680,34 @@ describe('Path', () => {
     expect(halves[1].ops[0].to.y).to.equal(30)
   })
 
+  // Issue 6816: https://github.com/freesewing/freesewing/issues/6816
+  it('Should split a path on the start of that same path', () => {
+    const A = new Point(45, 60)
+    const B = new Point(10, 30)
+
+    const test = new Path().move(A).line(B)
+    let halves = test.split(A)
+    expect(halves[0]).to.equal(null)
+    expect(halves[1].ops[0].to.x).to.equal(45)
+    expect(halves[1].ops[0].to.y).to.equal(60)
+    expect(halves[1].ops[1].to.x).to.equal(10)
+    expect(halves[1].ops[1].to.y).to.equal(30)
+  })
+
+  // Issue 6816: https://github.com/freesewing/freesewing/issues/6816
+  it('Should split a path on the end of that same path', () => {
+    const A = new Point(45, 60)
+    const B = new Point(10, 30)
+
+    const test = new Path().move(A).line(B)
+    let halves = test.split(B)
+    expect(halves[1]).to.equal(null)
+    expect(halves[0].ops[0].to.x).to.equal(45)
+    expect(halves[0].ops[0].to.y).to.equal(60)
+    expect(halves[0].ops[1].to.x).to.equal(10)
+    expect(halves[0].ops[1].to.y).to.equal(30)
+  })
+
   it('Should determine the angle on a path', () => {
     const a = new Point(0, 0)
     const b = new Point(0, 40)