From f3250059f2379795da09c3a82f8758ccd680980f Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Wed, 13 Sep 2023 09:17:07 +0530 Subject: [PATCH 01/11] fix bug + add test `_searchPossibleTargets` => `fintTargetsTraversal` --- src/canvas/Canvas.ts | 2 +- src/canvas/SelectableCanvas.ts | 117 +++++++++++------- .../__snapshots__/eventData.test.ts.snap | 59 +++++++-- src/canvas/__tests__/eventData.test.ts | 41 ++++++ 4 files changed, 165 insertions(+), 54 deletions(-) diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index db1cbb07706..d20c53eed4d 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -394,7 +394,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { */ protected findDragTargets(e: DragEvent) { this.targets = []; - const target = this._searchPossibleTargets( + const target = this.searchPossibleTargets( this._objects, this.getViewportPoint(e) ); diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 7a3fce244d9..84ff16e372e 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -30,7 +30,7 @@ import type { IText } from '../shapes/IText/IText'; import type { BaseBrush } from '../brushes/BaseBrush'; import { pick } from '../util/misc/pick'; import { sendPointToPlane } from '../util/misc/planeChange'; -import { cos, createCanvasElement, sin } from '../util'; +import { cos, createCanvasElement, invertTransform, sin } from '../util'; import { CanvasDOMManager } from './DOMManagers/CanvasDOMManager'; import { BOTTOM, CENTER, LEFT, RIGHT, TOP } from '../constants'; import type { CanvasOptions } from './CanvasOptions'; @@ -819,69 +819,96 @@ export class SelectableCanvas } /** - * Internal Function used to search inside objects an object that contains pointer in bounding box or that contains pointerOnCanvas when painted + * Search for objects containing {@link pointer}. + * Search is not greedy, returning once a hit is found. * @param {Array} [objects] objects array to look into - * @param {Object} [pointer] x,y object of point coordinates we want to check. - * @return {FabricObject} **top most object from given `objects`** that contains pointer - * @private + * @param {Object} [pointer] point coordinates to check + * @param {boolean} [param2.searchStrategy] strategy + * @returns {FabricObject[]} path of objects starting from **top most** object on screen. */ - _searchPossibleTargets( + protected findTargetsTraversal( objects: FabricObject[], - pointer: Point - ): FabricObject | undefined { - // Cache all targets where their bounding box contains point. - let i = objects.length; - // Do not check for currently grouped objects, since we check the parent group itself. - // until we call this function specifically to search inside the activeGroup - while (i--) { - const target = objects[i]; - if (this._checkTarget(target, pointer)) { + pointer: Point, + options: { searchStrategy: 'first-hit' | 'search-all' } + ): FabricObject[] { + const targets: FabricObject[] = []; + for (let index = objects.length - 1; index >= 0; index--) { + const target = objects[index]; + const pointerToUse = target.group + ? this._normalizePointer(target.group, pointer) + : pointer; + if (this._checkTarget(pointerToUse, target, pointer)) { if (isCollection(target) && target.subTargetCheck) { - const subTarget = this._searchPossibleTargets( - target._objects as FabricObject[], - pointer + targets.push( + ...this.findTargetsTraversal( + target._objects as FabricObject[], + pointer, + options + ) ); - subTarget && this.targets.push(subTarget); } - return target; + targets.push(target); + if (options.searchStrategy === 'first-hit') { + break; + } } } + return targets; } /** * Function used to search inside objects an object that contains pointer in bounding box or that contains pointerOnCanvas when painted - * @see {@link _searchPossibleTargets} + * @see {@link findTargetsTraversal} * @param {FabricObject[]} [objects] objects array to look into - * @param {Point} [pointer] coordinates from viewport to check. - * @return {FabricObject} **top most object on screen** that contains pointer + * @param {Point} [pointer] x,y object of point coordinates we want to check. + * @return {FabricObject} **top most selectable object on screen** that contains {@link pointer} */ searchPossibleTargets( objects: FabricObject[], pointer: Point ): FabricObject | undefined { - const target = this._searchPossibleTargets(objects, pointer); + const targets = this.findTargetsTraversal(objects, pointer, { + searchStrategy: 'first-hit', + }); + this.targets.push(...targets); + const found = targets.findIndex((target) => { + return !target.parent || target.parent.interactive; + }); + this.targets = targets.slice(0, found); + return targets[found]; + } - // if we found something in this.targets, and the group is interactive, return the innermost subTarget - // that is still interactive - // TODO: reverify why interactive. the target should be returned always, but selected only - // if interactive. - if ( - target && - isCollection(target) && - target.interactive && - this.targets[0] - ) { - /** targets[0] is the innermost nested target, but it could be inside non interactive groups and so not a selection target */ - const targets = this.targets; - for (let i = targets.length - 1; i > 0; i--) { - const t = targets[i]; - if (!(isCollection(t) && t.interactive)) { - // one of the subtargets was not interactive. that is the last subtarget we can return. - // we can't dig more deep; - return t; - } - } - return targets[0]; + /** + * Returns pointer coordinates without the effect of the viewport + * @param {Object} pointer with "x" and "y" number values in canvas HTML coordinates + * @return {Object} object with "x" and "y" number values in fabricCanvas coordinates + */ + restorePointerVpt(pointer: Point): Point { + return pointer.transform(invertTransform(this.viewportTransform)); + } + + /** + * Returns pointer coordinates relative to canvas. + * Can return coordinates with or without viewportTransform. + * ignoreVpt false gives back coordinates that represent + * the point clicked on canvas element. + * ignoreVpt true gives back coordinates after being processed + * by the viewportTransform ( sort of coordinates of what is displayed + * on the canvas where you are clicking. + * ignoreVpt true = HTMLElement coordinates relative to top,left + * ignoreVpt false, default = fabric space coordinates, the same used for shape position. + * To interact with your shapes top and left you want to use ignoreVpt false + * most of the time, while ignoreVpt true will give you coordinates + * compatible with the object.oCoords system. + * of the time. + * @param {Event} e + * @param {Boolean} ignoreVpt + * @return {Point} + */ + getPointer(e: TPointerEvent, ignoreVpt = false): Point { + // return cached values if we are in the event processing chain + if (this._absolutePointer && !ignoreVpt) { + return this._absolutePointer; } return target; diff --git a/src/canvas/__tests__/__snapshots__/eventData.test.ts.snap b/src/canvas/__tests__/__snapshots__/eventData.test.ts.snap index 696591c9ed2..44df9eeb89c 100644 --- a/src/canvas/__tests__/__snapshots__/eventData.test.ts.snap +++ b/src/canvas/__tests__/__snapshots__/eventData.test.ts.snap @@ -114,6 +114,33 @@ exports[`Canvas event data HTML event "dragend" should fire a corresponding canv "y": -13, }, "currentSubTargets": [], + "button": 1, + "e": MouseEvent { + "isTrusted": false, + }, + "isClick": false, + "pointer": Point { + "x": 50, + "y": 50, + }, + "subTargets": [ + "Drag Target", + ], + "target": "Drag Target", + "transform": null, + }, + ], + [ + "mouse:up", + { + "absolutePointer": Point { + "x": -30, + "y": -13, + }, + "button": 1, + "currentSubTargets": [ + "Drag Target", + ], "currentTarget": "Drag Target", "e": MouseEvent { "isTrusted": false, @@ -127,7 +154,9 @@ exports[`Canvas event data HTML event "dragend" should fire a corresponding canv "x": -30, "y": -13, }, - "subTargets": [], + "subTargets": [ + "Drag Target", + ], "target": "Drag Target", "transform": null, "viewportPoint": Point { @@ -204,7 +233,9 @@ exports[`Canvas event data HTML event "dragenter" should fire a corresponding ca "e": MouseEvent { "isTrusted": false, }, - "subTargets": [], + "subTargets": [ + "Drag Target", + ], "target": "Drag Target", }, ], @@ -228,7 +259,9 @@ exports[`Canvas event data HTML event "dragenter" should fire a corresponding ca "x": -30, "y": -13, }, - "subTargets": [], + "subTargets": [ + "Drag Target", + ], "target": "Drag Target", "viewportPoint": Point { "x": 50, @@ -284,7 +317,9 @@ exports[`Canvas event data HTML event "dragover" should fire a corresponding can "e": MouseEvent { "isTrusted": false, }, - "subTargets": [], + "subTargets": [ + "Drag Target", + ], "target": "Drag Target", }, ], @@ -310,7 +345,9 @@ exports[`Canvas event data HTML event "dragover" should fire a corresponding can "x": -30, "y": -13, }, - "subTargets": [], + "subTargets": [ + "Drag Target", + ], "target": "Drag Target", "viewportPoint": Point { "x": 50, @@ -353,7 +390,9 @@ exports[`Canvas event data HTML event "drop" should fire a corresponding canvas "x": -30, "y": -13, }, - "subTargets": [], + "subTargets": [ + "Drag Target", + ], "target": "Drag Target", "viewportPoint": Point { "x": 50, @@ -382,7 +421,9 @@ exports[`Canvas event data HTML event "drop" should fire a corresponding canvas "x": -30, "y": -13, }, - "subTargets": [], + "subTargets": [ + "Drag Target", + ], "target": "Drag Target", "viewportPoint": Point { "x": 50, @@ -411,7 +452,9 @@ exports[`Canvas event data HTML event "drop" should fire a corresponding canvas "x": -30, "y": -13, }, - "subTargets": [], + "subTargets": [ + "Drag Target", + ], "target": "Drag Target", "viewportPoint": Point { "x": 50, diff --git a/src/canvas/__tests__/eventData.test.ts b/src/canvas/__tests__/eventData.test.ts index 3d32723cdc6..4a5fb2c04e4 100644 --- a/src/canvas/__tests__/eventData.test.ts +++ b/src/canvas/__tests__/eventData.test.ts @@ -892,3 +892,44 @@ describe('Event targets', () => { expect(canvasSpy.mock.calls).toMatchSnapshot(); }); }); + +describe('Event targets', () => { + it('A selected subtarget should not fire an event twice', () => { + const target = new FabricObject(); + const group = new Group([target], { + subTargetCheck: true, + interactive: true, + }); + const canvas = new Canvas(null); + canvas.add(group); + const targetSpy = jest.fn(); + target.on('mousedown', targetSpy); + jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); + canvas.getSelectionElement().dispatchEvent( + new MouseEvent('mousedown', { + clientX: 50, + clientY: 50, + }) + ); + expect(targetSpy).toHaveBeenCalledTimes(1); + }); + + test('searchPossibleTargets', () => { + const subTarget = new FabricObject(); + const target = new Group([subTarget], { + subTargetCheck: true, + }); + const parent = new Group([target], { + subTargetCheck: true, + interactive: true, + }); + const canvas = new Canvas(null); + canvas.add(parent); + const targetSpy = jest.fn(); + target.on('mousedown', targetSpy); + jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); + const found = canvas.searchPossibleTargets([parent], new Point()); + expect(found).toBe(target); + expect(canvas.targets).toEqual([subTarget, target, parent]); + }); +}); From 33a4cbc425771e3dc9037c6318cd07f9a25514de Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 15 Sep 2023 09:07:45 +0530 Subject: [PATCH 02/11] fixes --- src/canvas/SelectableCanvas.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 84ff16e372e..db4c2d95992 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -870,12 +870,17 @@ export class SelectableCanvas const targets = this.findTargetsTraversal(objects, pointer, { searchStrategy: 'first-hit', }); - this.targets.push(...targets); const found = targets.findIndex((target) => { return !target.parent || target.parent.interactive; }); - this.targets = targets.slice(0, found); - return targets[found]; + + if (found > -1) { + this.targets = targets.slice(0, found); + return targets[found]; + } else { + this.targets = targets; + return undefined; + } } /** From 65fa732387cdfab54fe52d8fc0a1a113351eb913 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 15 Sep 2023 09:29:48 +0530 Subject: [PATCH 03/11] fix under selection --- src/canvas/Canvas.ts | 1 - src/canvas/SelectableCanvas.ts | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index d20c53eed4d..2bc354c2083 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -393,7 +393,6 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { * Override at will */ protected findDragTargets(e: DragEvent) { - this.targets = []; const target = this.searchPossibleTargets( this._objects, this.getViewportPoint(e) diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index db4c2d95992..0898e2878a6 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -704,8 +704,6 @@ export class SelectableCanvas activeObject = this._activeObject, aObjects = this.getActiveObjects(); - this.targets = []; - if (activeObject && aObjects.length >= 1) { if (activeObject.findControl(pointer, isTouchEvent(e))) { // if we hit the corner of the active object, let's return that. @@ -725,7 +723,6 @@ export class SelectableCanvas return activeObject; } else { const subTargets = this.targets; - this.targets = []; const target = this.searchPossibleTargets(this._objects, pointer); if ( e[this.altSelectionKey as ModifierKey] && @@ -870,8 +867,9 @@ export class SelectableCanvas const targets = this.findTargetsTraversal(objects, pointer, { searchStrategy: 'first-hit', }); + const found = targets.findIndex((target) => { - return !target.parent || target.parent.interactive; + return !target.group || target.group.interactive; }); if (found > -1) { From 861b7bb0c21737b8969c93864c417e18bf6ced8b Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 15 Sep 2023 10:55:59 +0530 Subject: [PATCH 04/11] fix another case --- src/canvas/Canvas.ts | 4 +++- src/canvas/SelectableCanvas.ts | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index 2bc354c2083..4e1bb781320 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -1403,7 +1403,9 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { const pointer = this.getViewportPoint(e); target = // first search active objects for a target to remove - this.searchPossibleTargets(prevActiveObjects, pointer) || + this.findTargetsTraversal(prevActiveObjects, pointer, { + searchStrategy: 'first-hit', + })[0] || // if not found, search under active selection for a target to add // `prevActiveObjects` will be searched but we already know they will not be found this.searchPossibleTargets(this._objects, pointer); diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 0898e2878a6..7ec1bced718 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -854,10 +854,10 @@ export class SelectableCanvas } /** - * Function used to search inside objects an object that contains pointer in bounding box or that contains pointerOnCanvas when painted + * Function used to search objects for a object containing {@link pointer} * @see {@link findTargetsTraversal} * @param {FabricObject[]} [objects] objects array to look into - * @param {Point} [pointer] x,y object of point coordinates we want to check. + * @param {Point} [pointer] viewport point * @return {FabricObject} **top most selectable object on screen** that contains {@link pointer} */ searchPossibleTargets( From 6c48f5a83360f1935c46bb99d58c026729fadf8b Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 15 Sep 2023 11:17:17 +0530 Subject: [PATCH 05/11] import tests --- .../__snapshots__/eventData.test.ts.snap | 16 +- src/canvas/__tests__/eventData.test.ts | 721 +++++++++++++++++- 2 files changed, 712 insertions(+), 25 deletions(-) diff --git a/src/canvas/__tests__/__snapshots__/eventData.test.ts.snap b/src/canvas/__tests__/__snapshots__/eventData.test.ts.snap index 44df9eeb89c..2ad384401ec 100644 --- a/src/canvas/__tests__/__snapshots__/eventData.test.ts.snap +++ b/src/canvas/__tests__/__snapshots__/eventData.test.ts.snap @@ -123,9 +123,7 @@ exports[`Canvas event data HTML event "dragend" should fire a corresponding canv "x": 50, "y": 50, }, - "subTargets": [ - "Drag Target", - ], + "subTargets": [], "target": "Drag Target", "transform": null, }, @@ -138,9 +136,7 @@ exports[`Canvas event data HTML event "dragend" should fire a corresponding canv "y": -13, }, "button": 1, - "currentSubTargets": [ - "Drag Target", - ], + "currentSubTargets": [], "currentTarget": "Drag Target", "e": MouseEvent { "isTrusted": false, @@ -233,9 +229,7 @@ exports[`Canvas event data HTML event "dragenter" should fire a corresponding ca "e": MouseEvent { "isTrusted": false, }, - "subTargets": [ - "Drag Target", - ], + "subTargets": [], "target": "Drag Target", }, ], @@ -317,9 +311,7 @@ exports[`Canvas event data HTML event "dragover" should fire a corresponding can "e": MouseEvent { "isTrusted": false, }, - "subTargets": [ - "Drag Target", - ], + "subTargets": [], "target": "Drag Target", }, ], diff --git a/src/canvas/__tests__/eventData.test.ts b/src/canvas/__tests__/eventData.test.ts index 4a5fb2c04e4..33195de70fe 100644 --- a/src/canvas/__tests__/eventData.test.ts +++ b/src/canvas/__tests__/eventData.test.ts @@ -914,22 +914,717 @@ describe('Event targets', () => { expect(targetSpy).toHaveBeenCalledTimes(1); }); - test('searchPossibleTargets', () => { - const subTarget = new FabricObject(); - const target = new Group([subTarget], { + test('mouseover and mouseout with subTargetCheck', () => { + const rect1 = new FabricObject({ + width: 5, + height: 5, + left: 5, + top: 0, + strokeWidth: 0, + }); + const rect2 = new FabricObject({ + width: 5, + height: 5, + left: 5, + top: 5, + strokeWidth: 0, + }); + const rect3 = new FabricObject({ + width: 5, + height: 5, + left: 0, + top: 5, + strokeWidth: 0, + }); + const rect4 = new FabricObject({ + width: 5, + height: 5, + left: 0, + top: 0, + strokeWidth: 0, + }); + const rect5 = new FabricObject({ + width: 5, + height: 5, + left: 2.5, + top: 2.5, + strokeWidth: 0, + }); + const group1 = new Group([rect1, rect2], { subTargetCheck: true, }); - const parent = new Group([target], { + const group2 = new Group([rect3, rect4], { subTargetCheck: true, - interactive: true, }); - const canvas = new Canvas(null); - canvas.add(parent); - const targetSpy = jest.fn(); - target.on('mousedown', targetSpy); - jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); - const found = canvas.searchPossibleTargets([parent], new Point()); - expect(found).toBe(target); - expect(canvas.targets).toEqual([subTarget, target, parent]); + // a group with 2 groups, with 2 rects each, one group left one group right + // each with 2 rects vertically aligned + const group = new Group([group1, group2], { + subTargetCheck: true, + }); + + const enter = jest.fn(); + const exit = jest.fn(); + + const getTargetsFromEventStream = (mock: jest.Mock) => + mock.mock.calls.map((args) => args[0].target); + + registerTestObjects({ + rect1, + rect2, + rect3, + rect4, + rect5, + group1, + group2, + group, + }); + + Object.values({ + rect1, + rect2, + rect3, + rect4, + rect5, + group1, + group2, + group, + }).forEach((object) => { + object.on('mouseover', enter); + object.on('mouseout', exit); + }); + + const canvas = new Canvas(); + canvas.add(group, rect5); + + const fire = (x: number, y: number) => { + enter.mockClear(); + exit.mockClear(); + canvas + .getSelectionElement() + .dispatchEvent(new MouseEvent('mousemove', { clientX: x, clientY: y })); + }; + + fire(1, 1); + expect(getTargetsFromEventStream(enter)).toEqual([group, rect4, group2]); + expect(getTargetsFromEventStream(exit)).toEqual([]); + + fire(5, 5); + expect(getTargetsFromEventStream(enter)).toEqual([rect5]); + expect(getTargetsFromEventStream(exit)).toEqual([group, rect4, group2]); + + fire(9, 9); + expect(getTargetsFromEventStream(enter)).toEqual([group, rect2, group1]); + expect(getTargetsFromEventStream(exit)).toEqual([rect5]); + + fire(9, 1); + expect(getTargetsFromEventStream(enter)).toEqual([rect1]); + expect(getTargetsFromEventStream(exit)).toEqual([rect2]); + }); + + describe('findTarget', () => { + const mockEvent = ({ + canvas, + ...init + }: MouseEventInit & { canvas: Canvas }) => { + const e = new MouseEvent('mousedown', { + ...init, + }); + jest + .spyOn(e, 'target', 'get') + .mockReturnValue(canvas.getSelectionElement()); + return e; + }; + + const findTarget = (canvas: Canvas, ev?: MouseEventInit) => { + const target = canvas.findTarget( + mockEvent({ canvas, clientX: 0, clientY: 0, ...ev }) + ); + const targets = canvas.targets; + canvas.targets = []; + return { target, targets }; + }; + + test.each([true, false])( + 'findTargetsTraversal: search all is %s', + (searchAll) => { + const subTarget1 = new FabricObject(); + const target1 = new Group([subTarget1], { + subTargetCheck: true, + interactive: true, + }); + const subTarget2 = new FabricObject(); + const target2 = new Group([subTarget2], { + subTargetCheck: true, + }); + const parent = new Group([target1, target2], { + subTargetCheck: true, + interactive: true, + }); + registerTestObjects({ + subTarget1, + target1, + subTarget2, + target2, + parent, + }); + + const canvas = new Canvas(null); + canvas.add(parent); + + jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); + const found = canvas['findTargetsTraversal']([parent], new Point(), { + searchStrategy: searchAll ? 'search-all' : 'first-hit', + }); + expect(found).toEqual( + searchAll + ? [subTarget2, target2, subTarget1, target1, parent] + : [subTarget2, target2, parent] + ); + } + ); + + test('searchPossibleTargets', () => { + const subTarget = new FabricObject(); + const target = new Group([subTarget], { + subTargetCheck: true, + }); + const parent = new Group([target], { + subTargetCheck: true, + interactive: true, + }); + registerTestObjects({ subTarget, target, parent }); + + const canvas = new Canvas(null); + canvas.add(parent); + + jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); + const found = canvas.searchPossibleTargets([parent], new Point()); + expect(found).toBe(target); + expect(canvas.targets).toEqual([subTarget]); + }); + + test('searchPossibleTargets with selection', () => { + const subTarget = new FabricObject(); + const target = new Group([subTarget], { + subTargetCheck: true, + }); + const other = new FabricObject(); + const activeSelection = new ActiveSelection(); + registerTestObjects({ subTarget, target, other, activeSelection }); + + const canvas = new Canvas(null, { activeSelection }); + canvas.add(other, target); + activeSelection.add(target, other); + canvas.setActiveObject(activeSelection); + + jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); + const found = canvas.searchPossibleTargets( + [activeSelection], + new Point() + ); + expect(found).toBe(activeSelection); + expect(canvas.targets).toEqual([]); + }); + + test('findTarget clears prev targets', () => { + const canvas = new Canvas(); + canvas.targets = [new FabricObject()]; + expect(findTarget(canvas, { clientX: 0, clientY: 0 })).toEqual({ + target: undefined, + targets: [], + }); + }); + + test('findTarget preserveObjectStacking false', () => { + const rect = new FabricObject({ + left: 0, + top: 0, + width: 10, + height: 10, + controls: {}, + }); + const rectOver = new FabricObject({ + left: 0, + top: 0, + width: 10, + height: 10, + controls: {}, + }); + registerTestObjects({ rect, rectOver }); + + const canvas = new Canvas(null, { preserveObjectStacking: false }); + canvas.add(rect, rectOver); + canvas.setActiveObject(rect); + + expect(findTarget(canvas, { clientX: 5, clientY: 5 })).toEqual({ + target: rect, + targets: [], + }); + }); + + test('findTarget preserveObjectStacking true', () => { + const rect = new FabricObject({ left: 0, top: 0, width: 30, height: 30 }); + const rectOver = new FabricObject({ + left: 0, + top: 0, + width: 30, + height: 30, + }); + registerTestObjects({ rect, rectOver }); + + const canvas = new Canvas(null, { preserveObjectStacking: true }); + canvas.add(rect, rectOver); + + const e = { + clientX: 15, + clientY: 15, + shiftKey: true, + }; + const e2 = { clientX: 4, clientY: 4 }; + + expect(findTarget(canvas, e)).toEqual( + { target: rectOver, targets: [] } + // 'Should return the rectOver, rect is not considered' + ); + + canvas.setActiveObject(rect); + expect(findTarget(canvas, e)).toEqual( + { target: rectOver, targets: [] } + // 'Should still return rectOver because is above active object' + ); + + expect(findTarget(canvas, e2)).toEqual( + { target: rect, targets: [] } + // 'Should rect because a corner of the activeObject has been hit' + ); + + canvas.altSelectionKey = 'shiftKey'; + expect(findTarget(canvas, e)).toEqual( + { target: rect, targets: [] } + // 'Should rect because active and altSelectionKey is pressed' + ); + }); + + test('findTarget with subTargetCheck', () => { + const canvas = new Canvas(); + const rect = new FabricObject({ left: 0, top: 0, width: 10, height: 10 }); + const rect2 = new FabricObject({ + left: 30, + top: 30, + width: 10, + height: 10, + }); + const group = new Group([rect, rect2]); + registerTestObjects({ rect, rect2, group }); + canvas.add(group); + + expect(findTarget(canvas, { clientX: 5, clientY: 5 })).toEqual({ + target: group, + targets: [], + }); + + expect(findTarget(canvas, { clientX: 35, clientY: 35 })).toEqual({ + target: group, + targets: [], + }); + + group.subTargetCheck = true; + group.setCoords(); + + expect(findTarget(canvas, { clientX: 5, clientY: 5 })).toEqual({ + target: group, + targets: [rect], + }); + + expect(findTarget(canvas, { clientX: 15, clientY: 15 })).toEqual({ + target: group, + targets: [], + }); + + expect(findTarget(canvas, { clientX: 35, clientY: 35 })).toEqual({ + target: group, + targets: [rect2], + }); + }); + + test('findTarget with subTargetCheck and canvas zoom', () => { + const nested1 = new FabricObject({ + width: 100, + height: 100, + fill: 'yellow', + }); + const nested2 = new FabricObject({ + width: 100, + height: 100, + left: 100, + top: 100, + fill: 'purple', + }); + const nestedGroup = new Group([nested1, nested2], { + scaleX: 0.5, + scaleY: 0.5, + top: 100, + left: 0, + subTargetCheck: true, + }); + const rect1 = new FabricObject({ + width: 100, + height: 100, + fill: 'red', + }); + const rect2 = new FabricObject({ + width: 100, + height: 100, + left: 100, + top: 100, + fill: 'blue', + }); + const group = new Group([rect1, rect2, nestedGroup], { + top: -150, + left: -50, + subTargetCheck: true, + }); + registerTestObjects({ + rect1, + rect2, + nested1, + nested2, + nestedGroup, + group, + }); + + const canvas = new Canvas(null, { + viewportTransform: [0.1, 0, 0, 0.1, 100, 200], + }); + canvas.add(group); + + expect(findTarget(canvas, { clientX: 96, clientY: 186 })).toEqual({ + target: group, + targets: [rect1], + }); + + expect(findTarget(canvas, { clientX: 98, clientY: 188 })).toEqual({ + target: group, + targets: [rect1], + }); + + expect(findTarget(canvas, { clientX: 100, clientY: 190 })).toEqual({ + target: group, + targets: [rect1], + }); + + expect(findTarget(canvas, { clientX: 102, clientY: 192 })).toEqual({ + target: group, + targets: [rect1], + }); + + expect(findTarget(canvas, { clientX: 104, clientY: 194 })).toEqual({ + target: group, + targets: [rect1], + }); + + expect(findTarget(canvas, { clientX: 106, clientY: 196 })).toEqual({ + target: group, + targets: [rect2], + }); + }); + + test.each([true, false])( + 'findTarget on activeObject with subTargetCheck and preserveObjectStacking %s', + (preserveObjectStacking) => { + const rect = new FabricObject({ + left: 0, + top: 0, + width: 10, + height: 10, + }); + const rect2 = new FabricObject({ + left: 30, + top: 30, + width: 10, + height: 10, + }); + const group = new Group([rect, rect2], { subTargetCheck: true }); + registerTestObjects({ rect, rect2, group }); + + const canvas = new Canvas(null, { preserveObjectStacking }); + canvas.add(group); + canvas.setActiveObject(group); + + expect(findTarget(canvas, { clientX: 9, clientY: 9 })).toEqual({ + target: group, + targets: [rect], + }); + } + ); + + test('findTarget with perPixelTargetFind', () => { + const triangle = new Triangle({ width: 30, height: 30 }); + registerTestObjects({ triangle }); + + const canvas = new Canvas(); + canvas.add(triangle); + + expect(findTarget(canvas, { clientX: 5, clientY: 5 })).toEqual({ + target: triangle, + targets: [], + }); + + canvas.perPixelTargetFind = true; + + expect(findTarget(canvas, { clientX: 5, clientY: 5 })).toEqual({ + target: undefined, + targets: [], + }); + expect(findTarget(canvas, { clientX: 15, clientY: 15 })).toEqual({ + target: triangle, + targets: [], + }); + }); + + describe('findTarget with perPixelTargetFind in nested group', () => { + const prepareTest = () => { + const deepTriangle = new Triangle({ + left: 0, + top: 0, + width: 30, + height: 30, + fill: 'yellow', + }); + const triangle2 = new Triangle({ + left: 100, + top: 120, + width: 30, + height: 30, + angle: 100, + fill: 'pink', + }); + const deepCircle = new Circle({ + radius: 30, + top: 0, + left: 30, + fill: 'blue', + }); + const circle2 = new Circle({ + scaleX: 2, + scaleY: 2, + radius: 10, + top: 120, + left: -20, + fill: 'purple', + }); + const deepRect = new Rect({ + width: 50, + height: 30, + top: 10, + left: 110, + fill: 'red', + skewX: 40, + skewY: 20, + }); + const rect2 = new Rect({ + width: 100, + height: 80, + top: 50, + left: 60, + fill: 'green', + }); + const deepGroup = new Group([deepTriangle, deepCircle, deepRect], { + subTargetCheck: true, + }); + const group2 = new Group([deepGroup, circle2, rect2, triangle2], { + subTargetCheck: true, + }); + const group3 = new Group([group2], { subTargetCheck: true }); + + registerTestObjects({ + deepTriangle, + triangle2, + deepCircle, + circle2, + rect2, + deepRect, + deepGroup, + group2, + group3, + }); + + const canvas = new Canvas(null, { perPixelTargetFind: true }); + canvas.add(group3); + + return { + canvas, + deepTriangle, + triangle2, + deepCircle, + circle2, + rect2, + deepRect, + deepGroup, + group2, + group3, + }; + }; + + test.each([ + { x: 5, y: 5 }, + { x: 21, y: 9 }, + { x: 37, y: 7 }, + { x: 89, y: 47 }, + { x: 16, y: 122 }, + { x: 127, y: 37 }, + { x: 87, y: 139 }, + ])('transparent hit on %s', ({ x: clientX, y: clientY }) => { + const { canvas } = prepareTest(); + expect(findTarget(canvas, { clientX, clientY })).toEqual({ + target: undefined, + targets: [], + }); + }); + + test('findTarget with perPixelTargetFind in nested group', () => { + const { + canvas, + deepTriangle, + triangle2, + deepCircle, + circle2, + rect2, + deepRect, + deepGroup, + group2, + group3, + } = prepareTest(); + + expect(findTarget(canvas, { clientX: 15, clientY: 15 })).toEqual({ + target: group3, + targets: [deepTriangle, deepGroup, group2], + }); + + expect(findTarget(canvas, { clientX: 50, clientY: 20 })).toEqual({ + target: group3, + targets: [deepCircle, deepGroup, group2], + }); + + expect(findTarget(canvas, { clientX: 117, clientY: 16 })).toEqual({ + target: group3, + targets: [deepRect, deepGroup, group2], + }); + + expect(findTarget(canvas, { clientX: 100, clientY: 90 })).toEqual({ + target: group3, + targets: [rect2, group2], + }); + + expect(findTarget(canvas, { clientX: 9, clientY: 145 })).toEqual({ + target: group3, + targets: [circle2, group2], + }); + + expect(findTarget(canvas, { clientX: 66, clientY: 143 })).toEqual({ + target: group3, + targets: [triangle2, group2], + }); + }); + }); + + test('findTarget on active selection', () => { + const rect1 = new FabricObject({ + left: 0, + top: 0, + width: 10, + height: 10, + }); + const rect2 = new FabricObject({ + left: 20, + top: 20, + width: 10, + height: 10, + }); + const rect3 = new FabricObject({ + left: 20, + top: 0, + width: 10, + height: 10, + }); + const activeSelection = new ActiveSelection([rect1, rect2], { + subTargetCheck: true, + cornerSize: 2, + }); + registerTestObjects({ rect1, rect2, rect3, activeSelection }); + + const canvas = new Canvas(null, { activeSelection }); + canvas.add(rect1, rect2, rect3); + canvas.setActiveObject(activeSelection); + + expect(findTarget(canvas, { clientX: 5, clientY: 5 })).toEqual({ + target: activeSelection, + targets: [rect1], + }); + + expect(findTarget(canvas, { clientX: 40, clientY: 15 })).toEqual({ + target: undefined, + targets: [], + }); + expect(activeSelection.__corner).toBeUndefined(); + + expect(findTarget(canvas, { clientX: 0, clientY: 0 })).toEqual({ + target: activeSelection, + targets: [], + }); + expect(activeSelection.__corner).toBe('tl'); + + expect(findTarget(canvas, { clientX: 25, clientY: 5 })).toEqual( + { + target: activeSelection, + targets: [], + } + // 'Should not return the rect behind active selection' + ); + + canvas.discardActiveObject(); + expect(findTarget(canvas, { clientX: 25, clientY: 5 })).toEqual( + { + target: rect3, + targets: [], + } + // 'Should return the rect after clearing selection' + ); + }); + + test('findTarget on active selection with perPixelTargetFind', () => { + const rect1 = new Rect({ + left: 0, + top: 0, + width: 10, + height: 10, + }); + const rect2 = new Rect({ + left: 20, + top: 20, + width: 10, + height: 10, + }); + const activeSelection = new ActiveSelection([rect1, rect2]); + registerTestObjects({ rect1, rect2, activeSelection }); + + const canvas = new Canvas(null, { + activeSelection, + perPixelTargetFind: true, + preserveObjectStacking: true, + }); + canvas.add(rect1, rect2); + canvas.setActiveObject(activeSelection); + + expect(findTarget(canvas, { clientX: 8, clientY: 8 })).toEqual({ + target: activeSelection, + targets: [], + }); + + expect(findTarget(canvas, { clientX: 15, clientY: 15 })).toEqual({ + target: undefined, + targets: [], + }); + }); }); }); From f03dd3376649ee57f3524274c3709935d40c7aec Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 15 Sep 2023 05:35:30 +0000 Subject: [PATCH 06/11] update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 499011a1e1b..5045ab38c8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -146,6 +146,9 @@ - fix(Utils): fix exported svg color [#9408](https://github.com/fabricjs/fabric.js/pull/9408) ## [6.0.0-beta13] +- fix(): `searchPossibleTargets` `targets` value [#9343](https://github.com/fabricjs/fabric.js/pull/9343) + +## [6.0.0-b3] - fix(Textbox): implemente a fix for the style shifting issues on new lines [#9197](https://github.com/fabricjs/fabric.js/pull/9197) - Fix(Control) fix a regression in `wrap with fixed anchor`, regression from #8400 [#9326](https://github.com/fabricjs/fabric.js/pull/9326) From 113020e75d9169483939dda8b0e23ce3d53da944 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 15 Sep 2023 11:34:40 +0530 Subject: [PATCH 07/11] add more important tests cleanup typo --- src/canvas/SelectableCanvas.ts | 5 +- src/canvas/__tests__/eventData.test.ts | 64 ++++++++++++++++++-------- 2 files changed, 48 insertions(+), 21 deletions(-) diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 7ec1bced718..621e2a91bcc 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -854,8 +854,11 @@ export class SelectableCanvas } /** - * Function used to search objects for a object containing {@link pointer} + * Search objects for an object containing {@link pointer} + * depending on the tree's configuration (`subTargetCheck`, `interactive`, `selectable`) + * * @see {@link findTargetsTraversal} + * * @param {FabricObject[]} [objects] objects array to look into * @param {Point} [pointer] viewport point * @return {FabricObject} **top most selectable object on screen** that contains {@link pointer} diff --git a/src/canvas/__tests__/eventData.test.ts b/src/canvas/__tests__/eventData.test.ts index 33195de70fe..71748312843 100644 --- a/src/canvas/__tests__/eventData.test.ts +++ b/src/canvas/__tests__/eventData.test.ts @@ -1103,28 +1103,52 @@ describe('Event targets', () => { expect(canvas.targets).toEqual([subTarget]); }); - test('searchPossibleTargets with selection', () => { - const subTarget = new FabricObject(); - const target = new Group([subTarget], { - subTargetCheck: true, - }); - const other = new FabricObject(); - const activeSelection = new ActiveSelection(); - registerTestObjects({ subTarget, target, other, activeSelection }); + test.only.each([true, false])( + 'searchPossibleTargets with selection and subTargetCheck %s', + (subTargetCheck) => { + const subTarget = new FabricObject(); + const target = new Group([subTarget], { + subTargetCheck: true, + }); + const other = new FabricObject(); + const activeSelection = new ActiveSelection([], { + subTargetCheck, + }); + registerTestObjects({ subTarget, target, other, activeSelection }); - const canvas = new Canvas(null, { activeSelection }); - canvas.add(other, target); - activeSelection.add(target, other); - canvas.setActiveObject(activeSelection); + const canvas = new Canvas(null, { activeSelection }); + canvas.add(other, target); + activeSelection.add(target, other); + canvas.setActiveObject(activeSelection); - jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); - const found = canvas.searchPossibleTargets( - [activeSelection], - new Point() - ); - expect(found).toBe(activeSelection); - expect(canvas.targets).toEqual([]); - }); + jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); + + const foundTargets = canvas['findTargetsTraversal']( + [activeSelection], + new Point(), + { searchStrategy: 'search-all' } + ); + expect(foundTargets).toEqual( + subTargetCheck + ? [other, subTarget, target, activeSelection] + : [activeSelection] + ); + + const found = canvas.searchPossibleTargets( + [activeSelection], + new Point() + ); + expect(found).toBe(activeSelection); + expect(canvas.targets).toEqual(subTargetCheck ? [other] : []); + + const notFound = canvas.searchPossibleTargets( + canvas.getActiveObjects(), + new Point() + ); + expect(notFound).toBeUndefined(); + expect(canvas.targets).toEqual([other]); + } + ); test('findTarget clears prev targets', () => { const canvas = new Canvas(); From 0045876f1b970b10efa540e379ce3de4899d7ffa Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 15 Sep 2023 13:08:30 +0530 Subject: [PATCH 08/11] fix `searchPossibleTargets` --- src/canvas/Canvas.ts | 10 ++-- src/canvas/SelectableCanvas.ts | 76 +++++++++++++++++--------- src/canvas/__tests__/eventData.test.ts | 35 ++++++------ 3 files changed, 72 insertions(+), 49 deletions(-) diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index 4e1bb781320..5188335093d 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -393,13 +393,13 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { * Override at will */ protected findDragTargets(e: DragEvent) { - const target = this.searchPossibleTargets( + const { target, targets } = this.searchPossibleTargets( this._objects, this.getViewportPoint(e) ); return { target, - targets: [...this.targets], + targets: target ? targets.slice(0, targets.indexOf(target)) : targets, }; } @@ -1403,12 +1403,10 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { const pointer = this.getViewportPoint(e); target = // first search active objects for a target to remove - this.findTargetsTraversal(prevActiveObjects, pointer, { - searchStrategy: 'first-hit', - })[0] || + this.searchPossibleTargets(prevActiveObjects, pointer).target || // if not found, search under active selection for a target to add // `prevActiveObjects` will be searched but we already know they will not be found - this.searchPossibleTargets(this._objects, pointer); + this.searchPossibleTargets(this._objects, pointer).target; // if nothing is found bail out if (!target || !target.selectable) { return false; diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 621e2a91bcc..4e13ae17509 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -689,11 +689,11 @@ export class SelectableCanvas } /** - * Method that determines what object we are clicking on + * Determines what object and its sub targets {@link e} should target * 11/09/2018 TODO: would be cool if findTarget could discern between being a full target * or the outside part of the corner. * @param {Event} e mouse event - * @return {FabricObject | null} the target found + * @return {FabricObject | undefined} the target found */ findTarget(e: TPointerEvent): FabricObject | undefined { if (this.skipTargetFind) { @@ -708,38 +708,58 @@ export class SelectableCanvas if (activeObject.findControl(pointer, isTouchEvent(e))) { // if we hit the corner of the active object, let's return that. return activeObject; - } else if ( - aObjects.length > 1 && - // check pointer is over active selection and possibly perform `subTargetCheck` - this.searchPossibleTargets([activeObject], pointer) - ) { + } + + // check pointer is over active selection and possibly perform `subTargetCheck` + const { target: selectedTarget, targets: selectedTargets } = + this.searchPossibleTargets([activeObject], pointer); + + if (selectedTarget && aObjects.length > 1) { // active selection does not select sub targets like normal groups + // remove active selection for the array + this.targets = selectedTargets.slice(0, -1); return activeObject; - } else if ( - activeObject === this.searchPossibleTargets([activeObject], pointer) - ) { + } else if (activeObject === selectedTarget) { // active object is not an active selection if (!this.preserveObjectStacking) { + this.targets = selectedTargets.slice( + 0, + selectedTargets.indexOf(activeObject) + ); return activeObject; } else { - const subTargets = this.targets; - const target = this.searchPossibleTargets(this._objects, pointer); + const { target, targets: canvasTargets } = this.searchPossibleTargets( + this._objects, + pointer + ); + if ( e[this.altSelectionKey as ModifierKey] && target && target !== activeObject ) { // alt selection: select active object even though it is not the top most target - // restore targets - this.targets = subTargets; + this.targets = canvasTargets.slice( + 0, + canvasTargets.indexOf(activeObject) + ); return activeObject; } + + this.targets = target + ? canvasTargets.slice(0, canvasTargets.indexOf(target)) + : canvasTargets; return target; } } } - return this.searchPossibleTargets(this._objects, pointer); + const { target, targets } = this.searchPossibleTargets( + this._objects, + pointer + ); + this.targets = target ? targets.slice(0, targets.indexOf(target)) : targets; + return target; } /** @@ -865,23 +885,25 @@ export class SelectableCanvas */ searchPossibleTargets( objects: FabricObject[], - pointer: Point - ): FabricObject | undefined { + pointer: Point, + { + searchStrategy = 'first-hit', + }: { searchStrategy?: 'first-hit' | 'search-all' } = {} + ) { const targets = this.findTargetsTraversal(objects, pointer, { - searchStrategy: 'first-hit', + searchStrategy, }); - const found = targets.findIndex((target) => { - return !target.group || target.group.interactive; + const target = targets.find((target) => { + return ( + !target.group || target.group.interactive || objects.includes(target) + ); }); - if (found > -1) { - this.targets = targets.slice(0, found); - return targets[found]; - } else { - this.targets = targets; - return undefined; - } + return { + target, + targets, + }; } /** diff --git a/src/canvas/__tests__/eventData.test.ts b/src/canvas/__tests__/eventData.test.ts index 71748312843..93565263126 100644 --- a/src/canvas/__tests__/eventData.test.ts +++ b/src/canvas/__tests__/eventData.test.ts @@ -1098,12 +1098,13 @@ describe('Event targets', () => { canvas.add(parent); jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); - const found = canvas.searchPossibleTargets([parent], new Point()); - expect(found).toBe(target); - expect(canvas.targets).toEqual([subTarget]); + expect(canvas.searchPossibleTargets([parent], new Point())).toEqual({ + target, + targets: [subTarget, target, parent], + }); }); - test.only.each([true, false])( + test.each([true, false])( 'searchPossibleTargets with selection and subTargetCheck %s', (subTargetCheck) => { const subTarget = new FabricObject(); @@ -1134,19 +1135,21 @@ describe('Event targets', () => { : [activeSelection] ); - const found = canvas.searchPossibleTargets( - [activeSelection], - new Point() - ); - expect(found).toBe(activeSelection); - expect(canvas.targets).toEqual(subTargetCheck ? [other] : []); + expect( + canvas.searchPossibleTargets([activeSelection], new Point()) + ).toEqual({ + target: activeSelection, + targets: subTargetCheck + ? [other, activeSelection] + : [activeSelection], + }); - const notFound = canvas.searchPossibleTargets( - canvas.getActiveObjects(), - new Point() - ); - expect(notFound).toBeUndefined(); - expect(canvas.targets).toEqual([other]); + expect( + canvas.searchPossibleTargets(canvas.getActiveObjects(), new Point()) + ).toEqual({ + target: other, + targets: [other], + }); } ); From 2d3344dca2b0fd509bddedb88dec0f451cebcb89 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 15 Sep 2023 13:17:54 +0530 Subject: [PATCH 09/11] dep(): `searchPossibleTargets` => `findTargets` jsdoc Update SelectableCanvas.ts --- src/canvas/Canvas.ts | 6 +++--- src/canvas/SelectableCanvas.ts | 28 +++++++++++++++----------- src/canvas/__tests__/eventData.test.ts | 12 +++++------ 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index 5188335093d..9a55c2933d9 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -393,7 +393,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { * Override at will */ protected findDragTargets(e: DragEvent) { - const { target, targets } = this.searchPossibleTargets( + const { target, targets } = this.findTargets( this._objects, this.getViewportPoint(e) ); @@ -1403,10 +1403,10 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { const pointer = this.getViewportPoint(e); target = // first search active objects for a target to remove - this.searchPossibleTargets(prevActiveObjects, pointer).target || + this.findTargets(prevActiveObjects, pointer).target || // if not found, search under active selection for a target to add // `prevActiveObjects` will be searched but we already know they will not be found - this.searchPossibleTargets(this._objects, pointer).target; + this.findTargets(this._objects, pointer).target; // if nothing is found bail out if (!target || !target.selectable) { return false; diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 4e13ae17509..d5496f43865 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -712,7 +712,7 @@ export class SelectableCanvas // check pointer is over active selection and possibly perform `subTargetCheck` const { target: selectedTarget, targets: selectedTargets } = - this.searchPossibleTargets([activeObject], pointer); + this.findTargets([activeObject], pointer); if (selectedTarget && aObjects.length > 1) { // active selection does not select sub targets like normal groups @@ -728,7 +728,7 @@ export class SelectableCanvas ); return activeObject; } else { - const { target, targets: canvasTargets } = this.searchPossibleTargets( + const { target, targets: canvasTargets } = this.findTargets( this._objects, pointer ); @@ -754,10 +754,7 @@ export class SelectableCanvas } } - const { target, targets } = this.searchPossibleTargets( - this._objects, - pointer - ); + const { target, targets } = this.findTargets(this._objects, pointer); this.targets = target ? targets.slice(0, targets.indexOf(target)) : targets; return target; } @@ -837,9 +834,9 @@ export class SelectableCanvas /** * Search for objects containing {@link pointer}. - * Search is not greedy, returning once a hit is found. - * @param {Array} [objects] objects array to look into - * @param {Object} [pointer] point coordinates to check + * + * @param {FabricObject[]} objects objects array to look into + * @param {Point} pointer point canvas element plane coordinates to check * @param {boolean} [param2.searchStrategy] strategy * @returns {FabricObject[]} path of objects starting from **top most** object on screen. */ @@ -877,13 +874,13 @@ export class SelectableCanvas * Search objects for an object containing {@link pointer} * depending on the tree's configuration (`subTargetCheck`, `interactive`, `selectable`) * - * @see {@link findTargetsTraversal} + * @see {@link findTarget} and {@link findTargetsTraversal} * * @param {FabricObject[]} [objects] objects array to look into - * @param {Point} [pointer] viewport point + * @param {Point} pointer viewport point * @return {FabricObject} **top most selectable object on screen** that contains {@link pointer} */ - searchPossibleTargets( + findTargets( objects: FabricObject[], pointer: Point, { @@ -906,6 +903,13 @@ export class SelectableCanvas }; } + /** + * @deprecated use {@link findTargets} instead + */ + searchPossibleTargets(objects: FabricObject[], pointer: Point) { + return this.findTargets(objects, pointer).target; + } + /** * Returns pointer coordinates without the effect of the viewport * @param {Object} pointer with "x" and "y" number values in canvas HTML coordinates diff --git a/src/canvas/__tests__/eventData.test.ts b/src/canvas/__tests__/eventData.test.ts index 93565263126..4404e220838 100644 --- a/src/canvas/__tests__/eventData.test.ts +++ b/src/canvas/__tests__/eventData.test.ts @@ -1083,7 +1083,7 @@ describe('Event targets', () => { } ); - test('searchPossibleTargets', () => { + test('findTargets', () => { const subTarget = new FabricObject(); const target = new Group([subTarget], { subTargetCheck: true, @@ -1098,14 +1098,14 @@ describe('Event targets', () => { canvas.add(parent); jest.spyOn(canvas, '_checkTarget').mockReturnValue(true); - expect(canvas.searchPossibleTargets([parent], new Point())).toEqual({ + expect(canvas.findTargets([parent], new Point())).toEqual({ target, targets: [subTarget, target, parent], }); }); test.each([true, false])( - 'searchPossibleTargets with selection and subTargetCheck %s', + 'findTargets with selection and subTargetCheck %s', (subTargetCheck) => { const subTarget = new FabricObject(); const target = new Group([subTarget], { @@ -1135,9 +1135,7 @@ describe('Event targets', () => { : [activeSelection] ); - expect( - canvas.searchPossibleTargets([activeSelection], new Point()) - ).toEqual({ + expect(canvas.findTargets([activeSelection], new Point())).toEqual({ target: activeSelection, targets: subTargetCheck ? [other, activeSelection] @@ -1145,7 +1143,7 @@ describe('Event targets', () => { }); expect( - canvas.searchPossibleTargets(canvas.getActiveObjects(), new Point()) + canvas.findTargets(canvas.getActiveObjects(), new Point()) ).toEqual({ target: other, targets: [other], From 031b6524e9e851919e28ea61777f3f6ed24b006a Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 15 Sep 2023 13:32:26 +0530 Subject: [PATCH 10/11] a final test addition --- src/canvas/__tests__/eventData.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/canvas/__tests__/eventData.test.ts b/src/canvas/__tests__/eventData.test.ts index 4404e220838..ea2ad93fe38 100644 --- a/src/canvas/__tests__/eventData.test.ts +++ b/src/canvas/__tests__/eventData.test.ts @@ -1148,6 +1148,15 @@ describe('Event targets', () => { target: other, targets: [other], }); + + expect( + canvas.findTargets(canvas.getActiveObjects(), new Point(), { + searchStrategy: 'search-all', + }) + ).toEqual({ + target: other, + targets: [other, subTarget, target], + }); } ); From 070f94ea18fbb4de1545144dbbee0b809b71ee5c Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Mon, 6 May 2024 12:40:59 +0300 Subject: [PATCH 11/11] cleanup --- src/canvas/SelectableCanvas.ts | 38 +--------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index d5496f43865..f4ff3361977 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -30,7 +30,7 @@ import type { IText } from '../shapes/IText/IText'; import type { BaseBrush } from '../brushes/BaseBrush'; import { pick } from '../util/misc/pick'; import { sendPointToPlane } from '../util/misc/planeChange'; -import { cos, createCanvasElement, invertTransform, sin } from '../util'; +import { cos, createCanvasElement, sin } from '../util'; import { CanvasDOMManager } from './DOMManagers/CanvasDOMManager'; import { BOTTOM, CENTER, LEFT, RIGHT, TOP } from '../constants'; import type { CanvasOptions } from './CanvasOptions'; @@ -910,42 +910,6 @@ export class SelectableCanvas return this.findTargets(objects, pointer).target; } - /** - * Returns pointer coordinates without the effect of the viewport - * @param {Object} pointer with "x" and "y" number values in canvas HTML coordinates - * @return {Object} object with "x" and "y" number values in fabricCanvas coordinates - */ - restorePointerVpt(pointer: Point): Point { - return pointer.transform(invertTransform(this.viewportTransform)); - } - - /** - * Returns pointer coordinates relative to canvas. - * Can return coordinates with or without viewportTransform. - * ignoreVpt false gives back coordinates that represent - * the point clicked on canvas element. - * ignoreVpt true gives back coordinates after being processed - * by the viewportTransform ( sort of coordinates of what is displayed - * on the canvas where you are clicking. - * ignoreVpt true = HTMLElement coordinates relative to top,left - * ignoreVpt false, default = fabric space coordinates, the same used for shape position. - * To interact with your shapes top and left you want to use ignoreVpt false - * most of the time, while ignoreVpt true will give you coordinates - * compatible with the object.oCoords system. - * of the time. - * @param {Event} e - * @param {Boolean} ignoreVpt - * @return {Point} - */ - getPointer(e: TPointerEvent, ignoreVpt = false): Point { - // return cached values if we are in the event processing chain - if (this._absolutePointer && !ignoreVpt) { - return this._absolutePointer; - } - - return target; - } - /** * @returns point existing in the same plane as the {@link HTMLCanvasElement}, * `(0, 0)` being the top left corner of the {@link HTMLCanvasElement}.