From fed705411455a7b68124840a7548153d955e4593 Mon Sep 17 00:00:00 2001
From: Christopher Chedeau <vjeuxx@gmail.com>
Date: Sun, 15 Mar 2020 13:42:18 -0700
Subject: [PATCH] Fix hit testing threshold (#969)

* Fix hit testing threshold

- The bounding box was not correctly extended to take into account the threshold. It was only for y axis but not x.
- The bezier threshold was using 20 instead of 10 and not taking into account zoom level.

Both those issues are fixed and now the behavior looks good on all the shapes I can test.

* fix_tests
---
 src/element/collision.ts     | 31 +++++++++++++++++++++++++------
 src/tests/selection.test.tsx | 36 +++++++++++++++++++++++++++++++-----
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/src/element/collision.ts b/src/element/collision.ts
index e5b4e3ec..2da8091a 100644
--- a/src/element/collision.ts
+++ b/src/element/collision.ts
@@ -165,7 +165,12 @@ export function hitTest(
     const shape = getShapeForElement(element) as Drawable[];
 
     const [x1, y1, x2, y2] = getLinearElementAbsoluteBounds(element);
-    if (x < x1 || y < y1 - 10 || x > x2 || y > y2 + 10) {
+    if (
+      x < x1 - lineThreshold ||
+      y < y1 - lineThreshold ||
+      x > x2 + lineThreshold ||
+      y > y2 + lineThreshold
+    ) {
       return false;
     }
 
@@ -173,7 +178,9 @@ export function hitTest(
     const relY = y - element.y;
 
     // hit thest all "subshapes" of the linear element
-    return shape.some(subshape => hitTestRoughShape(subshape.sets, relX, relY));
+    return shape.some(subshape =>
+      hitTestRoughShape(subshape.sets, relX, relY, lineThreshold),
+    );
   } else if (element.type === "text") {
     const [x1, y1, x2, y2] = getElementAbsoluteCoords(element);
 
@@ -191,6 +198,7 @@ const pointInBezierEquation = (
   p2: Point,
   p3: Point,
   [mx, my]: Point,
+  lineThreshold: number,
 ) => {
   // B(t) = p0 * (1-t)^3 + 3p1 * t * (1-t)^2 + 3p2 * t^2 * (1-t) + p3 * t^3
   const equation = (t: number, idx: number) =>
@@ -199,7 +207,6 @@ const pointInBezierEquation = (
     3 * Math.pow(t, 2) * (1 - t) * p1[idx] +
     p0[idx] * Math.pow(t, 3);
 
-  const epsilon = 20;
   // go through t in increments of 0.01
   let t = 0;
   while (t <= 1.0) {
@@ -208,7 +215,7 @@ const pointInBezierEquation = (
 
     const diff = Math.sqrt(Math.pow(tx - mx, 2) + Math.pow(ty - my, 2));
 
-    if (diff < epsilon) {
+    if (diff < lineThreshold) {
       return true;
     }
 
@@ -218,7 +225,12 @@ const pointInBezierEquation = (
   return false;
 };
 
-const hitTestRoughShape = (opSet: OpSet[], x: number, y: number) => {
+const hitTestRoughShape = (
+  opSet: OpSet[],
+  x: number,
+  y: number,
+  lineThreshold: number,
+) => {
   // read operations from first opSet
   const ops = opSet[0].ops;
 
@@ -248,7 +260,14 @@ const hitTestRoughShape = (opSet: OpSet[], x: number, y: number) => {
       // check if points are on the curve
       // cubic bezier curves require four parameters
       // the first parameter is the last stored position (p0)
-      const retVal = pointInBezierEquation(p0, p1, p2, p3, [x, y]);
+      const retVal = pointInBezierEquation(
+        p0,
+        p1,
+        p2,
+        p3,
+        [x, y],
+        lineThreshold,
+      );
 
       // set end point of bezier curve as the new starting point for
       // upcoming operations as each operation is based on the last drawn
diff --git a/src/tests/selection.test.tsx b/src/tests/selection.test.tsx
index 5e8e17f0..f98bc597 100644
--- a/src/tests/selection.test.tsx
+++ b/src/tests/selection.test.tsx
@@ -163,10 +163,23 @@ describe("select single element on the scene", () => {
       fireEvent.keyDown(document, { key: KEYS.ESCAPE });
     }
 
+    /*
+        1 2 3 4 5 6 7 8 9
+      1
+      2     x
+      3
+      4       .
+      5
+      6
+      7           x
+      8
+      9
+    */
+
     const tool = getByToolName("selection");
     fireEvent.click(tool);
-    // click on a line on the rectangle
-    fireEvent.pointerDown(canvas, { clientX: 45, clientY: 20 });
+    // click on a line on the arrow
+    fireEvent.pointerDown(canvas, { clientX: 40, clientY: 40 });
     fireEvent.pointerUp(canvas);
 
     expect(renderScene).toHaveBeenCalledTimes(7);
@@ -175,7 +188,7 @@ describe("select single element on the scene", () => {
     expect(h.appState.selectedElementIds[h.elements[0].id]).toBeTruthy();
   });
 
-  it("arrow", () => {
+  it("arrow escape", () => {
     const { getByToolName, container } = render(<App />);
     const canvas = container.querySelector("canvas")!;
     {
@@ -188,10 +201,23 @@ describe("select single element on the scene", () => {
       fireEvent.keyDown(document, { key: KEYS.ESCAPE });
     }
 
+    /*
+        1 2 3 4 5 6 7 8 9
+      1
+      2     x
+      3
+      4       .
+      5
+      6
+      7           x
+      8
+      9
+    */
+
     const tool = getByToolName("selection");
     fireEvent.click(tool);
-    // click on a line on the rectangle
-    fireEvent.pointerDown(canvas, { clientX: 45, clientY: 20 });
+    // click on a line on the arrow
+    fireEvent.pointerDown(canvas, { clientX: 40, clientY: 40 });
     fireEvent.pointerUp(canvas);
 
     expect(renderScene).toHaveBeenCalledTimes(7);