fix(): regression of `getPointer` usages + BREAKING: drop event data #9186

ShaMan123 · 2023-08-30T07:58:06Z

Motivation

prepare for #9175

fixes bug from b6f076c

Description

Somewehre in v6 _pointer and _abolutePointer were changed to getPointer and many ~~done WRONG~~ were swapped in values.
Not a single test catch it so I have added tests

pointer => getPointer(e, true)
absolutePointer => getPointer(e)

Changes

Gist

In Action

commit 71d898d Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Aug 29 15:33:14 2023 +0530 fix jest.extend commit c5c5d38 Merge: 19fab4e a445362 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Aug 29 13:57:47 2023 +0530 Merge branch 'master' into rm-vpt-methods commit 19fab4e Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Aug 29 13:49:14 2023 +0530 dep commit be1dfd0 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Aug 29 13:48:19 2023 +0530 fix round snapshots Update jest.extend.ts commit 1dfcefc Author: ShaMan123 <shacharnen@gmail.com> Date: Sat Aug 26 08:40:58 2023 +0530 Update eventData.test.ts commit 0b47e20 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 20:50:09 2023 +0530 fix mouseup test leanup Update eventData.test.ts commit ab38a5d Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 19:12:19 2023 +0530 tests commit e567b27 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 17:19:01 2023 +0530 cleanup commit d201b34 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 15:21:31 2023 +0530 comment Update SelectableCanvas.ts Update SelectableCanvas.ts commit 3731da4 Merge: 2636773 7de49ff Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 15:14:58 2023 +0530 Merge branch 'rm-vpt-methods' of https://github.com/fabricjs/fabric.js into rm-vpt-methods commit 2636773 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 15:12:17 2023 +0530 revert commit 7de49ff Author: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Date: Fri Aug 25 09:35:04 2023 +0000 update CHANGELOG.md commit 7824e2f Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 15:03:24 2023 +0530 fix dramatic bug not a single test to account for commit 017beb3 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 14:30:56 2023 +0530 very weird comment commit 23d3ec4 Author: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Date: Fri Aug 25 08:44:37 2023 +0000 update CHANGELOG.md commit ccf1e5a Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 14:10:56 2023 +0530 patch(Canvas): rm `_normalizePointer`, `restorePointerVpt` Update dom_misc.ts

github-actions · 2023-08-30T07:59:57Z

Build Stats

file / KB (diff)	bundled	minified
fabric	915.042 (+0.239)	305.411 (+0.050)

commit 71d898d Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Aug 29 15:33:14 2023 +0530 fix jest.extend commit c5c5d38 Merge: 19fab4e a445362 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Aug 29 13:57:47 2023 +0530 Merge branch 'master' into rm-vpt-methods commit 19fab4e Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Aug 29 13:49:14 2023 +0530 dep commit be1dfd0 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Aug 29 13:48:19 2023 +0530 fix round snapshots Update jest.extend.ts commit 1dfcefc Author: ShaMan123 <shacharnen@gmail.com> Date: Sat Aug 26 08:40:58 2023 +0530 Update eventData.test.ts commit 0b47e20 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 20:50:09 2023 +0530 fix mouseup test leanup Update eventData.test.ts commit ab38a5d Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 19:12:19 2023 +0530 tests commit e567b27 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 17:19:01 2023 +0530 cleanup commit d201b34 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 15:21:31 2023 +0530 comment Update SelectableCanvas.ts Update SelectableCanvas.ts commit 3731da4 Merge: 2636773 7de49ff Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 15:14:58 2023 +0530 Merge branch 'rm-vpt-methods' of https://github.com/fabricjs/fabric.js into rm-vpt-methods commit 2636773 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 15:12:17 2023 +0530 revert commit 7de49ff Author: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Date: Fri Aug 25 09:35:04 2023 +0000 update CHANGELOG.md commit 7824e2f Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 15:03:24 2023 +0530 fix dramatic bug not a single test to account for commit 017beb3 Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 14:30:56 2023 +0530 very weird comment commit 23d3ec4 Author: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Date: Fri Aug 25 08:44:37 2023 +0000 update CHANGELOG.md commit ccf1e5a Author: ShaMan123 <shacharnen@gmail.com> Date: Fri Aug 25 14:10:56 2023 +0530 patch(Canvas): rm `_normalizePointer`, `restorePointerVpt` Update dom_misc.ts Update canvas.js

ShaMan123

step

…abric.js into fix-getPointer-usages

ShaMan123

ready for reviewing and fixing

github-actions · 2023-08-30T08:07:25Z

Coverage after merging fix-getPointer-usages into master will be

82.97%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
index.node.ts	7.69%	100%	0%	14.29%	17, 20, 23, 35, 38, 41
src
ClassRegistry.ts	100%	100%	100%	100%
Collection.ts	94.71%	94.64%	86.67%	97.09%	101, 104, 207–208, 233–234
CommonMethods.ts	96.55%	87.50%	100%	100%	9
Intersection.ts	100%	100%	100%	100%
Observable.ts	87.76%	85.29%	87.50%	89.58%	134–135, 160–161, 32–33, 41, 50, 78, 89
Point.ts	100%	100%	100%	100%
Shadow.ts	98.36%	95.65%	100%	100%	178
cache.ts	97.06%	90%	100%	100%	57
config.ts	75%	66.67%	66.67%	82.76%	130, 138, 138, 138, 138, 138–140, 151–153
constants.ts	100%	100%	100%	100%
src/Pattern
Pattern.ts	92.31%	91.89%	90%	93.55%	118, 129, 138, 31, 94
src/brushes
BaseBrush.ts	100%	100%	100%	100%
CircleBrush.ts	0%	0%	0%	0%	108, 108, 108, 110, 112, 114–116, 118–121, 128–129, 13, 136, 138, 23–24, 32–36, 40–44, 51–54, 62–66, 68, 76, 76, 76, 76, 76–77, 79, 79, 79–82, 84, 92–93, 95, 97–99
PatternBrush.ts	97.06%	87.50%	100%	100%	21
PencilBrush.ts	91.06%	82.35%	100%	93.81%	122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
SprayBrush.ts	0%	0%	0%	0%	107, 107, 107, 107, 107–108, 110–111, 118–119, 121, 123–127, 136, 140–141, 141, 149, 149, 149–152, 154–157, 161–162, 164, 166–169, 17, 172, 179, 18, 180, 182, 184–185, 187, 194–195, 197–198, 20, 201, 201, 208, 208, 21, 212, 22, 22, 22–24, 28, 32, 39, 46, 53, 60, 67, 84–86, 94–96, 98–99
src/canvas
Canvas.ts	79.12%	77.68%	83.05%	79.61%	1000, 1003–1004, 1004, 1004, 1006, 1014, 1014, 1014–1016, 1016, 1016, 1023–1024, 1032–1033, 1033, 1033–1034, 1040, 1042, 1073–1075, 1078–1079, 1083–1084, 1197–1199, 1202–1203, 1276, 1395, 1517, 157, 182, 292–293, 296–300, 305, 328–329, 334–339, 359, 359, 359–360, 360, 360–361, 369, 37, 374–375, 375, 375–376, 378, 387, 393–394, 394, 394, 41, 437, 445, 449, 449, 449–450, 452, 535–536, 536, 536–537, 543, 543, 543–545, 565, 567, 567, 567–568, 568, 568, 571, 571, 571–572, 575, 584–585, 587–588, 590, 590–591, 593–594, 606–607, 607, 607–608, 610–615, 621, 628, 665, 665, 665, 667, 669–674, 680, 686, 686, 686–687, 689, 692, 697, 710, 738, 738, 796–797, 797, 797–798, 800, 803–804, 804, 804–805, 807–808, 811, 811–813, 816–817, 887, 899, 906, 927, 959, 980–981, 997–998, 998, 998–999
CanvasOptions.ts	100%	100%	100%	100%
SelectableCanvas.ts	94.44%	92.28%	94.23%	96.17%	1098, 1100, 1102–1103, 301, 477–478, 480–481, 481, 481, 530–531, 592–593, 646–648, 680, 685–686, 713–714, 898, 898–899, 902, 922, 922, 971, 979
StaticCanvas.ts	96.78%	93.09%	100%	98.53%	1030, 1040, 1092–1093, 1096, 1131–1132, 1208, 1217, 1217, 1221, 1221, 1268–1269, 185–186, 202, 570, 582–583, 913–914, 914, 914–915
StaticCanvasOptions.ts	100%	100%	100%	100%
TextEditingManager.ts	84.31%	71.43%	91.67%	88%	17–18, 18, 18–19, 19, 19
src/canvas/DOMManagers
CanvasDOMManager.ts	95.52%	70%	100%	100%	21–22, 29
StaticCanvasDOMManager.ts	100%	100%	100%	100%
util.ts	86.67%	80.56%	83.33%	93.94%	14, 26, 63–64, 67, 67, 74, 93–94
src/color
Color.ts	94.96%	91.67%	96.30%	96.05%	233, 258–259, 267–268, 48
color_map.ts	100%	100%	100%	100%
constants.ts	100%	100%	100%	100%
util.ts	85.71%	76.92%	100%	89.74%	55–56, 56, 58, 58, 58–59, 61–62, 89
src/controls
Control.ts	93.33%	87.88%	91.67%	97.78%	175, 240, 327, 327, 362
changeWidth.ts	100%	100%	100%	100%
commonControls.ts	100%	100%	100%	100%
controlRendering.ts	81.63%	78%	100%	84.78%	106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
drag.ts	100%	100%	100%	100%
fireEvent.ts	88.89%	75%	100%	100%	13
polyControl.ts	6.06%	0%	0%	11.43%	103, 117, 117, 117, 117, 117, 119–122, 122, 125, 132, 17, 25–29, 29, 29, 29, 29, 29, 29, 29, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–84, 84, 86, 89–90, 90, 90, 90, 90, 92, 97
rotate.ts	19.57%	12.50%	50%	21.43%	41, 45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73, 73–74, 76, 78, 80–81
scale.ts	93.57%	92.94%	100%	93.67%	129–130, 132–134, 148–149, 181–183, 42
scaleSkew.ts	78.79%	64.29%	100%	85.71%	27, 29, 29, 29, 31, 33, 35
skew.ts	91.03%	79.31%	100%	97.67%	131–132, 163–164, 171, 177, 179
util.ts	100%	100%	100%	100%
wrapWithFireEvent.ts	100%	100%	100%	100%
wrapWithFixedAnchor.ts	100%	100%	100%	100%
src/env
browser.ts</

asturur · 2023-08-30T13:02:17Z

@ShaMan123 those are the only swap instances i could find. Do you think there are more?

I removed the identity matrix test and i added a more generic matrix that doesn't put the absolute pointer in 0,0

asturur · 2023-08-30T13:15:12Z

src/canvas/Canvas.ts

@@ -512,6 +512,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
      target,
      subTargets: targets,
      dragSource: this._dragSource,
+      // this is an absolute pointer, the name convention is wrong.
+      // such was in 5.x


What do we want to do with this? do we make consistency and we fix it?

update: i normalized it. Do not merge if you don't agree.

Yes
I think we should expose a protected method that build the event data so everything calls it

asturur

let with my proposal of normalization. If this is good for you is approved you can merge.

ShaMan123

Brush API also uses the wrong naming... but that was like that before v6 => to be done in #8476 IMO
Group selector also uses the absolute pointer, which is weird cause it is not affected by vpt

getPointer usages seem correct

ShaMan123 · 2023-08-30T15:31:28Z

src/canvas/Canvas.ts

@@ -513,8 +513,8 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
      subTargets: targets,
      dragSource: this._dragSource,
      // this is an absolute pointer, the name convention is wrong.


Suggested change

// this is an absolute pointer, the name convention is wrong.

ShaMan123 · 2023-08-30T15:38:17Z

you anyways need to approve it since I created it, so merge at will

ShaMan123

done?

asturur · 2023-08-30T16:28:31Z

I can't find any getPointer usage in the brush files, can you point to me?
with this PR any usage of getPointer function is associated to the correct name, or at least that seems to me

asturur · 2023-08-30T16:31:32Z

ok i found them and added comments.

asturur · 2023-08-30T16:37:10Z

At least the pointers in brush are exposed to the brush functionality only, that is an issue for who is creating custom brushes that should be way less than who is just listening to events

ShaMan123 force-pushed the fix-getPointer-usages branch from 3de8c04 to ab07ee8 Compare August 30, 2023 07:58

ShaMan123 requested a review from asturur August 30, 2023 07:58

update CHANGELOG.md

6f718c8

ShaMan123 commented Aug 30, 2023

View reviewed changes

ShaMan123 added 2 commits August 30, 2023 13:31

Merge branch 'fix-getPointer-usages' of https://github.com/fabricjs/f…

40ef7c5

…abric.js into fix-getPointer-usages

checkout

588436b

ShaMan123 commented Aug 30, 2023

View reviewed changes

asturur added 3 commits August 30, 2023 14:49

swapped back

5dddc1a

tests

883c88c

Update dom_misc.ts

e1de82a

asturur added 2 commits August 30, 2023 15:10

fixed comment

51f4d85

found inconsistency

8bd06b0

asturur reviewed Aug 30, 2023

View reviewed changes

asturur added 2 commits August 30, 2023 15:17

normalize right away

d161974

tests

8b1b374

asturur previously approved these changes Aug 30, 2023

View reviewed changes

asturur changed the title ~~fix(): regression of getPointer usages~~ fix(): regression of getPointer usages + BREAKING: drop event data Aug 30, 2023

update CHANGELOG.md

a7529f8

github-actions bot dismissed asturur’s stale review via a7529f8 August 30, 2023 13:21

asturur previously approved these changes Aug 30, 2023

View reviewed changes

Merge branch 'master' into fix-getPointer-usages

ea8e504

asturur dismissed their stale review via ea8e504 August 30, 2023 14:55

asturur previously approved these changes Aug 30, 2023

View reviewed changes

asturur added 2 commits August 30, 2023 17:16

event-stream

edc8fff

event-stream

4ba153f

asturur dismissed their stale review via 4ba153f August 30, 2023 15:17

Merge branch 'master' into fix-getPointer-usages

eac1d3e

ShaMan123 commented Aug 30, 2023

View reviewed changes

test fixed

14613e4

ShaMan123 commented Aug 30, 2023

View reviewed changes

the comment

a725885

added more comments

4a3798c

asturur approved these changes Aug 30, 2023

View reviewed changes

asturur merged commit 0287646 into master Aug 30, 2023
24 checks passed

asturur deleted the fix-getPointer-usages branch August 30, 2023 16:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(): regression of `getPointer` usages + BREAKING: drop event data #9186

fix(): regression of `getPointer` usages + BREAKING: drop event data #9186

ShaMan123 commented Aug 30, 2023 •

edited by asturur

github-actions bot commented Aug 30, 2023 •

edited

ShaMan123 left a comment

ShaMan123 left a comment

github-actions bot commented Aug 30, 2023 •

edited

asturur commented Aug 30, 2023

asturur Aug 30, 2023

asturur Aug 30, 2023

ShaMan123 Aug 30, 2023

asturur left a comment

ShaMan123 left a comment

ShaMan123 Aug 30, 2023

ShaMan123 commented Aug 30, 2023

ShaMan123 left a comment

asturur commented Aug 30, 2023

asturur commented Aug 30, 2023

asturur commented Aug 30, 2023

fix(): regression of getPointer usages + BREAKING: drop event data #9186

fix(): regression of getPointer usages + BREAKING: drop event data #9186

Conversation

ShaMan123 commented Aug 30, 2023 • edited by asturur

Motivation

Description

Changes

Gist

In Action

github-actions bot commented Aug 30, 2023 • edited

ShaMan123 left a comment

Choose a reason for hiding this comment

ShaMan123 left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 30, 2023 • edited

asturur commented Aug 30, 2023

asturur Aug 30, 2023

Choose a reason for hiding this comment

asturur Aug 30, 2023

Choose a reason for hiding this comment

ShaMan123 Aug 30, 2023

Choose a reason for hiding this comment

asturur left a comment

Choose a reason for hiding this comment

ShaMan123 left a comment

Choose a reason for hiding this comment

ShaMan123 Aug 30, 2023

Choose a reason for hiding this comment

ShaMan123 commented Aug 30, 2023

ShaMan123 left a comment

Choose a reason for hiding this comment

asturur commented Aug 30, 2023

asturur commented Aug 30, 2023

asturur commented Aug 30, 2023

fix(): regression of `getPointer` usages + BREAKING: drop event data #9186

fix(): regression of `getPointer` usages + BREAKING: drop event data #9186

ShaMan123 commented Aug 30, 2023 •

edited by asturur

github-actions bot commented Aug 30, 2023 •

edited

github-actions bot commented Aug 30, 2023 •

edited