-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat(Canvas): provide correctly named canvas pointer methods. BREAKING: rm _normalizePointer
, restorePointerVpt
#9175
Merged
Merged
Changes from 31 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
ccf1e5a
patch(Canvas): rm `_normalizePointer`, `restorePointerVpt`
ShaMan123 23d3ec4
update CHANGELOG.md
github-actions[bot] 017beb3
very weird comment
ShaMan123 7824e2f
fix dramatic bug
ShaMan123 7de49ff
update CHANGELOG.md
github-actions[bot] 2636773
revert
ShaMan123 3731da4
Merge branch 'rm-vpt-methods' of https://github.com/fabricjs/fabric.j…
ShaMan123 d201b34
comment
ShaMan123 e567b27
cleanup
ShaMan123 ab38a5d
tests
ShaMan123 0b47e20
fix mouseup test
ShaMan123 1dfcefc
Update eventData.test.ts
ShaMan123 be1dfd0
fix round snapshots
ShaMan123 19fab4e
dep
ShaMan123 c5c5d38
Merge branch 'master' into rm-vpt-methods
ShaMan123 71d898d
fix jest.extend
ShaMan123 1894bc9
Merge branch 'fix-getPointer-usages' into rm-vpt-methods
ShaMan123 8eed9fb
Merge branch 'master' into rm-vpt-methods
asturur 971f6dd
Update dom_misc.ts
asturur be66636
snapshots from master
asturur 3405f6a
no empty changes
asturur d34b62c
rm `transformPointRelativeToCanvas`
ShaMan123 747965e
Merge branch 'master' into rm-vpt-methods
ShaMan123 b74e593
rename
ShaMan123 be60b9b
add a weird test
ShaMan123 8c6bdce
Merge branch 'master' into rm-vpt-methods
ShaMan123 b88a731
jsdoc
ShaMan123 d2e54be
rename
ShaMan123 fdb1f0e
Merge branch 'master' into rm-vpt-methods
ShaMan123 6e3a2b5
rename 2
ShaMan123 251a2de
fix - from merge conflict
ShaMan123 a2ab0dc
just the rename and grouping
asturur e33548b
this test is not great
asturur 10d462f
Update CHANGELOG.md
asturur File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -236,8 +236,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
const shared = { | ||
e, | ||
isClick: false, | ||
pointer: this.getPointer(e, true), | ||
absolutePointer: this.getPointer(e), | ||
pointer: this.getViewportPoint(e), | ||
absolutePointer: this.getScenePoint(e), | ||
}; | ||
this.fire('mouse:out', { ...shared, target }); | ||
this._hoveredTarget = undefined; | ||
|
@@ -263,8 +263,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
if (!this._currentTransform && !this.findTarget(e)) { | ||
this.fire('mouse:over', { | ||
e, | ||
pointer: this.getPointer(e, true), | ||
absolutePointer: this.getPointer(e), | ||
pointer: this.getViewportPoint(e), | ||
absolutePointer: this.getScenePoint(e), | ||
}); | ||
this._hoveredTarget = undefined; | ||
this._hoveredTargets = []; | ||
|
@@ -392,7 +392,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
this.targets = []; | ||
const target = this._searchPossibleTargets( | ||
this._objects, | ||
this.getPointer(e, true) | ||
this.getViewportPoint(e) | ||
); | ||
return { | ||
target, | ||
|
@@ -502,8 +502,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
target, | ||
subTargets: targets, | ||
dragSource: this._dragSource, | ||
pointer: this.getPointer(e, true), | ||
absolutePointer: this.getPointer(e), | ||
pointer: this.getViewportPoint(e), | ||
absolutePointer: this.getScenePoint(e), | ||
}); | ||
// will be set by the drop target | ||
options.didDrop = false; | ||
|
@@ -819,7 +819,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
let pointer, corner; | ||
if (target) { | ||
corner = target._findTargetCorner( | ||
this.getPointer(e, true), | ||
this.getViewportPoint(e), | ||
isTouchEvent(e) | ||
); | ||
if ( | ||
|
@@ -834,7 +834,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
const mouseUpHandler = | ||
control && control.getMouseUpHandler(e, target, control); | ||
if (mouseUpHandler) { | ||
pointer = this.getPointer(e); | ||
pointer = this.getScenePoint(e); | ||
mouseUpHandler.call(control, e, transform!, pointer.x, pointer.y); | ||
} | ||
} | ||
|
@@ -855,7 +855,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
transform.target, | ||
originalControl | ||
); | ||
pointer = pointer || this.getPointer(e); | ||
pointer = pointer || this.getScenePoint(e); | ||
originalMouseUpHandler && | ||
originalMouseUpHandler.call( | ||
originalControl, | ||
|
@@ -913,13 +913,14 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
e, | ||
target, | ||
subTargets: targets, | ||
pointer: this.getPointer(e, true), | ||
absolutePointer: this.getPointer(e), | ||
pointer: this.getViewportPoint(e), | ||
absolutePointer: this.getScenePoint(e), | ||
transform: this._currentTransform, | ||
...(eventType === 'up:before' || eventType === 'up' | ||
? { | ||
isClick: this._isClick, | ||
currentTarget: this.findTarget(e), | ||
// set by the preceding `findTarget` call | ||
currentSubTargets: this.targets, | ||
} | ||
: {}), | ||
|
@@ -984,8 +985,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
this.discardActiveObject(e); | ||
this.requestRenderAll(); | ||
} | ||
// this is an absolute pointer, the naming is wrong | ||
const pointer = this.getPointer(e); | ||
// TODO: this is a scene point so it should be renamed | ||
const pointer = this.getScenePoint(e); | ||
this.freeDrawingBrush && | ||
this.freeDrawingBrush.onMouseDown(pointer, { e, pointer }); | ||
this._handleEvent(e, 'down'); | ||
|
@@ -997,7 +998,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
*/ | ||
_onMouseMoveInDrawingMode(e: TPointerEvent) { | ||
if (this._isCurrentlyDrawing) { | ||
const pointer = this.getPointer(e); | ||
const pointer = this.getScenePoint(e); | ||
this.freeDrawingBrush && | ||
this.freeDrawingBrush.onMouseMove(pointer, { | ||
e, | ||
|
@@ -1014,12 +1015,12 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
* @param {Event} e Event object fired on mouseup | ||
*/ | ||
_onMouseUpInDrawingMode(e: TPointerEvent) { | ||
const pointer = this.getPointer(e); | ||
const pointer = this.getScenePoint(e); | ||
if (this.freeDrawingBrush) { | ||
this._isCurrentlyDrawing = !!this.freeDrawingBrush.onMouseUp({ | ||
e: e, | ||
// this is an absolute pointer, the naming is wrong | ||
pointer: pointer, | ||
pointer, | ||
}); | ||
} else { | ||
this._isCurrentlyDrawing = false; | ||
|
@@ -1089,7 +1090,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
!(target as IText).isEditing && | ||
target !== this._activeObject)) | ||
) { | ||
const p = this.getPointer(e); | ||
const p = this.getScenePoint(e); | ||
this._groupSelector = { | ||
x: p.x, | ||
y: p.y, | ||
|
@@ -1104,13 +1105,13 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
this.setActiveObject(target, e); | ||
} | ||
const corner = target._findTargetCorner( | ||
this.getPointer(e, true), | ||
this.getViewportPoint(e), | ||
isTouchEvent(e) | ||
); | ||
if (target === this._activeObject && (corner || !grouped)) { | ||
this._setupCurrentTransform(e, target, alreadySelected); | ||
const control = target.controls[corner], | ||
pointer = this.getPointer(e), | ||
pointer = this.getScenePoint(e), | ||
mouseDownHandler = | ||
control && control.getMouseDownHandler(e, target, control); | ||
if (mouseDownHandler) { | ||
|
@@ -1150,8 +1151,12 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
_cacheTransformEventData(e: TPointerEvent) { | ||
// reset in order to avoid stale caching | ||
this._resetTransformEventData(); | ||
this._pointer = this.getPointer(e, true); | ||
this._absolutePointer = this.restorePointerVpt(this._pointer); | ||
this._pointer = this.getViewportPoint(e); | ||
this._absolutePointer = sendPointToPlane( | ||
Comment on lines
+1161
to
+1162
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also these should be renamed and maybe put into an object |
||
this._pointer, | ||
undefined, | ||
this.viewportTransform | ||
); | ||
this._target = this._currentTransform | ||
? this._currentTransform.target | ||
: this.findTarget(e); | ||
|
@@ -1195,7 +1200,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
|
||
// We initially clicked in an empty area, so we draw a box for multiple selection | ||
if (groupSelector) { | ||
const pointer = this.getPointer(e); | ||
const pointer = this.getScenePoint(e); | ||
|
||
groupSelector.deltaX = pointer.x - groupSelector.x; | ||
groupSelector.deltaY = pointer.y - groupSelector.y; | ||
|
@@ -1307,8 +1312,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
target: oldTarget, | ||
nextTarget: target, | ||
isClick: false, | ||
pointer: this.getPointer(e, true), | ||
absolutePointer: this.getPointer(e), | ||
pointer: this.getViewportPoint(e), | ||
absolutePointer: this.getScenePoint(e), | ||
}; | ||
fireCanvas && this.fire(canvasOut, outOpt); | ||
oldTarget.fire(targetOut, outOpt); | ||
|
@@ -1320,8 +1325,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
target, | ||
previousTarget: oldTarget, | ||
isClick: false, | ||
pointer: this.getPointer(e, true), | ||
absolutePointer: this.getPointer(e), | ||
pointer: this.getViewportPoint(e), | ||
absolutePointer: this.getScenePoint(e), | ||
}; | ||
fireCanvas && this.fire(canvasIn, inOpt); | ||
target.fire(targetIn, inOpt); | ||
|
@@ -1343,18 +1348,18 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
* @param {Event} e Event fired on mousemove | ||
*/ | ||
_transformObject(e: TPointerEvent) { | ||
const pointer = this.getPointer(e), | ||
const scenePoint = this.getScenePoint(e), | ||
transform = this._currentTransform!, | ||
target = transform.target, | ||
// transform pointer to target's containing coordinate plane | ||
// both pointer and object should agree on every point | ||
localPointer = target.group | ||
? sendPointToPlane( | ||
pointer, | ||
scenePoint, | ||
undefined, | ||
target.group.calcTransformMatrix() | ||
) | ||
: pointer; | ||
: scenePoint; | ||
// seems used only here. | ||
// @TODO: investigate; | ||
transform.reset = false; | ||
|
@@ -1412,7 +1417,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
// here we call findTargetCorner always with undefined for the touch parameter. | ||
// we assume that if you are using a cursor you do not need to interact with | ||
// the bigger touch area. | ||
target._findTargetCorner(this.getPointer(e, true)); | ||
target._findTargetCorner(this.getViewportPoint(e)); | ||
|
||
if (!corner) { | ||
if ((target as Group).subTargetCheck) { | ||
|
@@ -1473,7 +1478,7 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { | |
const prevActiveObjects = | ||
activeSelection.getObjects() as FabricObject[]; | ||
if (target === activeSelection) { | ||
const pointer = this.getPointer(e, true); | ||
const pointer = this.getViewportPoint(e); | ||
target = | ||
// first search active objects for a target to remove | ||
this.searchPossibleTargets(prevActiveObjects, pointer) || | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,6 @@ import { | |
import type { TCanvasSizeOptions } from './StaticCanvas'; | ||
import { StaticCanvas } from './StaticCanvas'; | ||
import { isCollection } from '../util/typeAssertions'; | ||
import { invertTransform } from '../util/misc/matrix'; | ||
import { isTransparent } from '../util/misc/isTransparent'; | ||
import type { | ||
TMat2D, | ||
|
@@ -589,11 +588,11 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents> | |
const pointer = target.group | ||
? // transform pointer to target's containing coordinate plane | ||
sendPointToPlane( | ||
this.getPointer(e), | ||
this.getScenePoint(e), | ||
undefined, | ||
target.group.calcTransformMatrix() | ||
) | ||
: this.getPointer(e); | ||
: this.getScenePoint(e); | ||
const corner = target.getActiveControl() || '', | ||
control = !!corner && target.controls[corner], | ||
actionHandler = | ||
|
@@ -708,7 +707,7 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents> | |
return undefined; | ||
} | ||
|
||
const pointer = this.getPointer(e, true), | ||
const pointer = this.getViewportPoint(e), | ||
activeObject = this._activeObject, | ||
aObjects = this.getActiveObjects(); | ||
|
||
|
@@ -765,7 +764,10 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents> | |
obj && | ||
obj.visible && | ||
obj.evented && | ||
obj.containsPoint(this.restorePointerVpt(pointer), true) | ||
obj.containsPoint( | ||
sendPointToPlane(pointer, undefined, this.viewportTransform), | ||
true | ||
Comment on lines
+767
to
+769
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keep this in mind for conflict resolution with #9395 |
||
) | ||
) { | ||
if ( | ||
(this.perPixelTargetFind || obj.perPixelTargetFind) && | ||
|
@@ -815,7 +817,7 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents> | |
* Function used to search inside objects an object that contains pointer in bounding box or that contains pointerOnCanvas when painted | ||
* @see {@link _searchPossibleTargets} | ||
* @param {FabricObject[]} [objects] objects array to look into | ||
* @param {Object} [pointer] x,y object of point coordinates we want to check. | ||
* @param {Point} [pointer] coordinates from viewport to check. | ||
* @return {FabricObject} **top most object on screen** that contains pointer | ||
*/ | ||
searchPossibleTargets( | ||
|
@@ -835,42 +837,56 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents> | |
} | ||
|
||
/** | ||
* Returns pointer coordinates without the effect of the viewport | ||
* Takes a point in html canvas space and gives you back a point of the scene. | ||
* @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 | ||
* @returns point existing in the same plane as the {@link HTMLCanvasElement}, | ||
* `(0, 0)` being the top left corner of the {@link HTMLCanvasElement}. | ||
* This means that changes to the {@link viewportTransform} do not change the values of the point | ||
* and it remains unchanged from the viewer's perspective. | ||
* | ||
* @example | ||
* const scenePoint = sendPointToPlane( | ||
* this.getViewportPoint(e), | ||
* undefined, | ||
* canvas.viewportTransform | ||
* ); | ||
* | ||
*/ | ||
restorePointerVpt(pointer: Point): Point { | ||
return pointer.transform(invertTransform(this.viewportTransform)); | ||
getViewportPoint(e: TPointerEvent) { | ||
if (this._pointer) { | ||
return this._pointer; | ||
} | ||
return this.getPointer(e, true); | ||
} | ||
|
||
/** | ||
* 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} | ||
* @returns point existing in the scene (the same plane as the plane {@link FabricObject#getCenterPoint} exists in). | ||
* This means that changes to the {@link viewportTransform} do not change the values of the point, | ||
* however, from the viewer's perspective, the point is changed. | ||
* | ||
* @example | ||
* const viewportPoint = sendPointToPlane( | ||
* this.getScenePoint(e), | ||
* canvas.viewportTransform | ||
* ); | ||
* | ||
*/ | ||
getPointer(e: TPointerEvent, ignoreVpt = false): Point { | ||
// return cached values if we are in the event processing chain | ||
if (this._absolutePointer && !ignoreVpt) { | ||
getScenePoint(e: TPointerEvent) { | ||
if (this._absolutePointer) { | ||
return this._absolutePointer; | ||
ShaMan123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
if (this._pointer && ignoreVpt) { | ||
return this._pointer; | ||
} | ||
return this.getPointer(e); | ||
} | ||
|
||
/** | ||
* Returns pointer relative to canvas. | ||
* | ||
* @deprecated This method is deprecated since v6 to protect you from misuse. | ||
* Use {@link getViewportPoint} or {@link getScenePoint} instead. | ||
* | ||
* @param {Event} e | ||
* @param {Boolean} [fromViewport] whether to return the point from the viewport or in the scene | ||
* @return {Point} | ||
*/ | ||
getPointer(e: TPointerEvent, fromViewport = false): Point { | ||
const upperCanvasEl = this.upperCanvasEl, | ||
bounds = upperCanvasEl.getBoundingClientRect(); | ||
let pointer = getPointer(e), | ||
|
@@ -889,8 +905,8 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents> | |
this.calcOffset(); | ||
pointer.x = pointer.x - this._offset.left; | ||
pointer.y = pointer.y - this._offset.top; | ||
if (!ignoreVpt) { | ||
pointer = this.restorePointerVpt(pointer); | ||
if (!fromViewport) { | ||
pointer = sendPointToPlane(pointer, undefined, this.viewportTransform); | ||
} | ||
|
||
const retinaScaling = this.getRetinaScaling(); | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems redundant since we cache
getScenePoint