feat(ICollection/Layer): Group Rewrite #7473

asturur · 2021-11-04T12:40:55Z

Motivation

#7316 #6776 #7136 #7299 #7130 #7142
Group is obsolete. We need something much better for fabric.
I think this is a successful predecessor that can replace Group entirely.
This PR aims first and foremost to enable/support nested selection under an Object.

Gist

I managed to come up with this solution thanks to this fabricjs/fabricjs.com#315:

Creating custom objects built from other objects is an extremely powerful option, but it takes you closer to fabric's edge.

You may experience problems. Let's use an example to explain.

When an object (a) renders, it transforms the rendering context to it's center point.
In order to render another object (b) as a child of (a) we need to take that into account.
We can handle (b)'s transform matrix relative to (a)'s center point. This will render (b) correctly.

However, it will cause many problems.

Let's take, for example, one of the simplest and most powerful rendering optimizations to explain.

Before an object renders it checks if it's visible on screen. If it isn't it skips the rendering process.
Checking if it's visible means the object checks it's transform matrix relative to the canvas.
But we've handled it's transform matrix relative to (a)....

This will cause (b) to skip renders in some cases and not show at all.

This gotcha affects almost everything fabric has to offer.

The solution is simple.

Handle the transform matrix relative to canvas, ALWAYS. When rendering (b), transform the rendering context from (a)'s center point back to canvas.

For more information and detailed explanation, check the dedicated pages: Using transformations, Subclassing.

This is why Layer objects are transformed relative to canvas

Impl

Expose a method that adds the transform diff from the previous call to objects.
In order to layout we need to disable transform propagation.

To Do

Additional

migrate SprayBrush from group?

* Create layer.class.js * Update build.js * Update layer.class.js * stable * Update layer.class.js * add/remove objects * Update layer.class.js * svg * typo * Update layer.class.js * fix(): remove + layout * fix(): layout * svg * Update layer.class.js * Update layer.class.js * lint * Update canvas.class.js * Update layer.class.js * Revert "svg" This reverts commit 1433f56. * perf(): `SprayBrush` case `SprayBrush` case - a huge amount of objects optimizing logic according to `subTargetCheck` * parent property * Update layer.class.js * Update layer.class.js * feat(Object#onSelect): support returning a descendant of object Return a descendant of object (`subTargets` prop of the first argument passed to `onSelect`). This makes selecting a child object of a collection possible. * _applyLayoutStrategy: add context + call on object change * fix(_setActiveObject): handle nested selection if object is already selected * fix(): call setCoords, getObjectsBoundingBox * Update canvas.class.js * ci(): lint * Update canvas_events.mixin.js * better `getObjectsBoundingBox` * Revert "Update canvas_events.mixin.js" This reverts commit 025a76f. * Update canvas.class.js * Update layer.class.js * Update canvas_events.mixin.js * Update canvas_events.mixin.js * Update layer.class.js * Update layer.class.js * Update object_stacking.mixin.js * Update active_selection.class.js * Update layer.class.js * Update layer.class.js * Update layer.class.js * Update layer.class.js * fix(): render selected on top * lint * Update layer.class.js * fix(): nested selection * Update layer.class.js * fix(): selection stack order selecting an object belonging to the same parent while under a foreign object. * Update layer.class.js * fix(): caching

ShaMan123 · 2021-11-06T15:27:02Z

Changed _applyMatrixDiff hook to setCoords. This introduced a problem with object caching.
Hooking to render is OK, hooking to setCoords only is not good because it renders a frame+ late fixed

ShaMan123 · 2021-11-13T11:55:36Z

Last commits optimized layer in such a way I think it is now more performant than group.
Instead of rendering all the time after applying the matrix diff it does the following:

keep objects up to date from setCoords but doesn't re-render
render once an object is selected/deselected (the selected object is not rendered by layer)

~~The last major thing is rotation + bounding box~~

asturur · 2021-11-30T12:50:29Z

src/parser.js

@@ -29,6 +29,7 @@
        display:              'visible',
        visibility:           'visible',
        transform:            'transformMatrix',
+        'fabric-layout':      'layout',


what is this?

asturur · 2021-12-02T11:52:03Z

i m starting to review and taking notes. Can you leave the code steady for a bit?

ShaMan123 · 2021-12-02T13:45:15Z

Yes. בתאריך יום ה׳, 2 בדצמ׳ 2021, 13:52, מאת Andrea Bogazzi ‏< ***@***.***>:

…

i m starting to review and taking notes. Can you leave the code steady for a bit? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#7473 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIGAW4P4P5SNUINKL7Q4THTUO5MW5ANCNFSM5HLK6WLQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

asturur · 2021-12-02T14:01:36Z

There is some good stuff here!
Some lesser things probably shouldn't be done. I left extensive comments in our doc instead of triggering a tons of emails back and fort with code comments.

ShaMan123 · 2021-12-02T15:24:07Z

I will work with PRs and merge into this one so you won't get overwhelmed with commits or changes while trying to review this.
Next up #7529

ebigtm · 2022-01-09T15:01:30Z

src/canvas.class.js

-    _setActiveObject: function(object, e) {
-      if (this._activeObject === object) {
+    _setActiveObject: function (object, e) {
+      var isCollection = Array.isArray(object._objects), activeObject = this._activeObject;


For fixing the drag of multiple object isCollection should be fixed and line 1151 should be

var isCollection = (Array.isArray(object._objects) && object.type != 'activeSelection'), activeObject = this._activeObject;

Thanks! I'll look into it.
Just wanted to make sure you've tested this with #7529 merged into your local branch.
This should do it

git fetch upstream git merge ShaMan123/layer-proposal-patch --squash

I didn't test it but I have a few thoughts.
If I understand correctly if we accept your proposal selecting objects nested in active selection will become impossible.
Can you confirm?
If so this case should be handled by the dev overriding onSelect of active selection.

for me its working as it should selecting in icollection when its there but doesnt cancel selection for active selection, tested on #7529 merged branch.

asturur · 2022-01-10T10:25:01Z

https://codesandbox.io/s/yiliv

https://codesandbox.io/s/fabric-app-forked-cvewn

* getAncestors * isInFrontOf * tests * fix(Canvas): `_chooseObjectsToRender` * Update object.js * Update object.js

ShaMan123 · 2022-01-10T19:27:29Z

@ebigtm says:

after the fix i presented yesterday a new issue popped, after dragging the objects it casts ray on the object the mouse up was on and selects it, the fix i found is a bit rough: var result = object.type == 'iCollection' ? object.onSelect({ e: e, object: activeObject, subTargets: this.targets.filter(function (object) { return object.isSelectable(); }), activeSubTargets: object._objects.length > 0 ? true : subTargets && subTargets.filter(function (object) { return object.isSelectable(); }) }) : object.onSelect({ e: e, object: activeObject, });

maybe it can be done differntly but generally i added a condition regarding the type, maybe we can use isCollection var instead?

fabric.js/src/canvas.class.js

Lines 1192 to 1197 in 5571a3a

var result = object.onSelect({

e: e,

object: activeObject,

subTargets: this.targets.filter(function (object) { return object.isSelectable(); }),

activeSubTargets: subTargets && subTargets.filter(function (object) { return object.isSelectable(); })

});

ShaMan123 · 2022-02-14T14:23:08Z

closing this in favor of #7669
There's still selection logic that might be useful

asturur changed the title ~~feat() Layer v2 (#7472)~~ feat(Layer): Group Rewrite Nov 4, 2021

asturur added the possible_feature label Nov 4, 2021

ShaMan123 added 5 commits November 4, 2021 14:49

Update fabric.js

4ed606e

hook matrix diff to render/setCoords

9161b00

Update fabric.js

ec228f3

fix(): toSVG

22d3cfe

Update fabric.js

3a125d9

ShaMan123 added 14 commits November 13, 2021 09:01

fix(): nested selection

12f7ef7

ci(): lint

6720af9

caching

04d6729

Update layer.class.js

71584c6

fix(): layout + child modified

6a9ce37

Update layer.class.js

f012838

Update object.class.js

f74197e

Update object.class.js

d52dd90

perf(): optimize caching

681f734

JSDOC

67b36e1

Update layer.class.js

65ef1ab

fix(): deep selection

f0e3f16

Update layer.class.js

083f4d6

Update layer.class.js

5959fd3

Update fabric.js

0fda6e5

ShaMan123 added 2 commits November 30, 2021 13:40

Update icollection.class.js

46cc386

Update icollection.js

e856f0f

asturur commented Nov 30, 2021

View reviewed changes

ShaMan123 added 8 commits November 30, 2021 14:52

Update icollection.class.js

63e9fb2

Update icollection.js

d820cef

feat(Object): isDescendantOf

ab4745e

Update icollection.class.js

7a7df24

Update icollection.js

dcaec79

Update fabric.js

9be9196

fix(): collection opacity

3c48a23

Update icollection.class.js

dd8f48d

ebigtm suggested changes Jan 9, 2022

View reviewed changes

ShaMan123 added 2 commits January 10, 2022 01:28

add/remove with update

9e93a65

svg(): revert custom layout prop

d05a4e7

Layer proposal patch (#7529)

5571a3a

* getAncestors * isInFrontOf * tests * fix(Canvas): `_chooseObjectsToRender` * Update object.js * Update object.js

ShaMan123 mentioned this pull request Jan 11, 2022

Editing text inside groups #7598

Closed

Update fabric.js

3c90553

ShaMan123 mentioned this pull request Jan 12, 2022

fix(Geometry): account for group transform #7599

Closed

4 tasks

This was referenced Feb 10, 2022

V6: Group rewrite - closed due to polluted history #7668

Closed

feat(V6): Group Rewrite + Nested Selection #7669

Closed

ShaMan123 closed this Feb 14, 2022

ShaMan123 mentioned this pull request Apr 4, 2022

V6: 🎉 Group Rewrite #7670

Closed

48 tasks

ShaMan123 deleted the layer-proposal branch September 12, 2022 00:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ICollection/Layer): Group Rewrite #7473

feat(ICollection/Layer): Group Rewrite #7473

asturur commented Nov 4, 2021 •

edited by ShaMan123

Loading

ShaMan123 commented Nov 6, 2021 •

edited

Loading

ShaMan123 commented Nov 13, 2021 •

edited

Loading

asturur Nov 30, 2021

asturur commented Dec 2, 2021

ShaMan123 commented Dec 2, 2021 via email

asturur commented Dec 2, 2021

ShaMan123 commented Dec 2, 2021

ebigtm Jan 9, 2022

ShaMan123 Jan 9, 2022 •

edited

Loading

ShaMan123 Jan 9, 2022

ebigtm Jan 10, 2022

asturur commented Jan 10, 2022

ShaMan123 commented Jan 10, 2022 •

edited

Loading

ShaMan123 commented Feb 14, 2022

feat(ICollection/Layer): Group Rewrite #7473

feat(ICollection/Layer): Group Rewrite #7473

Conversation

asturur commented Nov 4, 2021 • edited by ShaMan123 Loading

Motivation

Gist

Impl

To Do

Additional

ShaMan123 commented Nov 6, 2021 • edited Loading

ShaMan123 commented Nov 13, 2021 • edited Loading

asturur Nov 30, 2021

Choose a reason for hiding this comment

asturur commented Dec 2, 2021

ShaMan123 commented Dec 2, 2021 via email

asturur commented Dec 2, 2021

ShaMan123 commented Dec 2, 2021

ebigtm Jan 9, 2022

Choose a reason for hiding this comment

ShaMan123 Jan 9, 2022 • edited Loading

Choose a reason for hiding this comment

ShaMan123 Jan 9, 2022

Choose a reason for hiding this comment

ebigtm Jan 10, 2022

Choose a reason for hiding this comment

asturur commented Jan 10, 2022

ShaMan123 commented Jan 10, 2022 • edited Loading

ShaMan123 commented Feb 14, 2022

asturur commented Nov 4, 2021 •

edited by ShaMan123

Loading

ShaMan123 commented Nov 6, 2021 •

edited

Loading

ShaMan123 commented Nov 13, 2021 •

edited

Loading

ShaMan123 Jan 9, 2022 •

edited

Loading

ShaMan123 commented Jan 10, 2022 •

edited

Loading