Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Intersection): bug causing selection edge case #8735

Merged
merged 30 commits into from
Feb 25, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [next]

- fix(Intersection): bug causing selection edge case [#8735](https://github.com/fabricjs/fabric.js/pull/8735)
- chore(TS): class interface for options/brevity [#8674](https://github.com/fabricjs/fabric.js/issues/8674)
- ci(): fix import autocomplete in dev mode #8725
- chore(): remove deprecated class util [#8731](https://github.com/fabricjs/fabric.js/pull/8731)
Expand Down
71 changes: 47 additions & 24 deletions src/Intersection.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,10 @@
import { Point } from './Point';
import { createVector } from './util/misc/vectors';

/* Adaptation of work of Kevin Lindsey (kevin@kevlindev.com) */

export type IntersectionType = 'Intersection' | 'Coincident' | 'Parallel';

/**
* **Assuming `T`, `A`, `B` are points on the same line**,
* check if `T` is contained in `[A, B]` by comparing the direction of the vectors from `T` to `A` and `B`
* @param T
* @param A
* @param B
* @returns true if `T` is contained
*/
const isContainedInInterval = (T: Point, A: Point, B: Point) => {
const TA = new Point(T).subtract(A);
const TB = new Point(T).subtract(B);
return (
Math.sign(TA.x) !== Math.sign(TB.x) || Math.sign(TA.y) !== Math.sign(TB.y)
);
};

export class Intersection {
declare points: Point[];

Expand Down Expand Up @@ -54,8 +39,46 @@ export class Intersection {
return this;
}

/**
* check if `T ∈ [A, B]`
*
* Solving the linear equation `A + s * AB = T` will give us `s`\
* If `s` has a solution and `s ∈ [0, 1]` then `T ∈ [A, B]`
* If `AB` has a zero component we revert to a simple coordinate check
*
* @param T
* @param A
* @param B
* @param [infinite] if true checks if `T` is on the line defined by `A` and `B`
* @returns true if `T` is contained
*/
static isPointContained(T: Point, A: Point, B: Point, infinite = false) {
if (A.eq(B)) {
return T.eq(A);
} else if (A.x === B.x) {
return (
T.x === A.x &&
(infinite || (T.y >= Math.min(A.y, B.y) && T.y <= Math.max(A.y, B.y)))
);
} else if (A.y === B.y) {
return (
T.y === A.y &&
(infinite || (T.x >= Math.min(A.x, B.x) && T.x <= Math.max(A.x, B.x)))
);
} else {
const AB = createVector(A, B);
const AT = createVector(A, T);
const s = AT.divide(AB);
return infinite
? Math.abs(s.x) === Math.abs(s.y)
: s.x === s.y && s.x >= 0 && s.x <= 1;
}
}
ShaMan123 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Checks if a line intersects another
* @see {@link https://en.wikipedia.org/wiki/Line%E2%80%93line_intersection line intersection}
* @see {@link https://en.wikipedia.org/wiki/Cramer%27s_rule Cramer's rule}
* @static
* @param {Point} a1
* @param {Point} a2
Expand All @@ -73,12 +96,12 @@ export class Intersection {
aInfinite = true,
bInfinite = true
): Intersection {
const b2xb1x = b2.x - b1.x,
a1yb1y = a1.y - b1.y,
const a2xa1x = a2.x - a1.x,
a2ya1y = a2.y - a1.y,
b2xb1x = b2.x - b1.x,
b2yb1y = b2.y - b1.y,
a1xb1x = a1.x - b1.x,
a2ya1y = a2.y - a1.y,
a2xa1x = a2.x - a1.x,
a1yb1y = a1.y - b1.y,
Comment on lines +107 to +112
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed order to be less confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and found the algebra behind it and documented it

uaT = b2xb1x * a1yb1y - b2yb1y * a1xb1x,
ubT = a2xa1x * a1yb1y - a2ya1y * a1xb1x,
uB = b2yb1y * a2xa1x - b2xb1x * a2ya1y;
Expand All @@ -100,10 +123,10 @@ export class Intersection {
const segmentsCoincide =
aInfinite ||
bInfinite ||
isContainedInInterval(a1, b1, b2) ||
isContainedInInterval(a2, b1, b2) ||
isContainedInInterval(b1, a1, a2) ||
isContainedInInterval(b2, a1, a2);
Intersection.isPointContained(a1, b1, b2) ||
Intersection.isPointContained(a2, b1, b2) ||
Intersection.isPointContained(b1, a1, a2) ||
Intersection.isPointContained(b2, a1, a2);
return new Intersection(segmentsCoincide ? 'Coincident' : undefined);
} else {
return new Intersection('Parallel');
Expand Down
7 changes: 3 additions & 4 deletions src/Point.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ export interface IPoint {
*/
export class Point implements IPoint {
declare x: number;

declare y: number;

static ZERO = new Point();

constructor();
constructor(x: number, y: number);
constructor(point: IPoint);
Expand Down Expand Up @@ -356,7 +357,7 @@ export class Point implements IPoint {
* @param {TRadian} radians The radians of the angle for the rotation
* @return {Point} The new rotated point
*/
rotate(radians: TRadian, origin: IPoint = originZero): Point {
rotate(radians: TRadian, origin: IPoint = Point.ZERO): Point {
// TODO benchmark and verify the add and subtract how much cost
// and then in case early return if no origin is passed
const sinus = sin(radians),
Expand Down Expand Up @@ -384,5 +385,3 @@ export class Point implements IPoint {
);
}
}

const originZero = new Point(0, 0);
ShaMan123 marked this conversation as resolved.
Show resolved Hide resolved
Loading