feat(Layer): Group Rewrite

[ShaMan123]

Motivation

#7316 #6776 #7136 #7299 #7130 #7142 #2073
Group is obsolete. We need something much better for fabric.
I think this is a successful predecessor that can replace Group entirely.

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

• In order to propagate transformation from Layer to it's objects I've extended calcOwnMatrix. The method adds the transform diff from the previous call to objects.
• In order to layout the Layer we need to disable transform propagation.

To Do

• object caching (copy from group?)
• performance - in case of a very large amount of objects (SprayBrush) + stress tests
• additional layout strategies + overflow (exposed public method getLayoutStrategyResult, need to support rotation)
• test svg import/export
• make interactive!! selectable nested objects - Ohh yes it's coming.
• tests - cover all group cases, etc.

[ShaMan123]

Check out the POC!!
https://codesandbox.io/s/lpek0
Try transforming the group, selecting objects and transforming them and vice versa
It doesn't work on codesandbox. some error raises. BUT it works magnificently

[ebigtm]

Looks great! exactly what we were asking if can be done a while ago, thank you @ShaMan123 !

Do you have an ETA for all the to dos? any way we can help?

P.s. My name is Elad and would really love to chat with you when you have the time [email protected]

[andrewzolotukhin]

Looks like a very good feature.

[ShaMan123]

@ebigtm I messaged you.

Regarding ETA. This PR is almost done.
Putting tests and edge cases aside the major thing remaining is to decide how object selection should occur and implement it. This shouldn't take more than a couple of hours once it's decided. ​
Should the layer be selected and then objects can be selected?
Should objects be selected even if layer isn't selected yet?
Should this be handled with some option/callback/modifier key?
Or perhaps leave this be and let the dev implement it, say by overriding Layer's onSelect method?
How would this works with very deep objects nested in layers?

Another issue that needs to be decided is if Layer will in fact deprecate Group.
If so, we will need to trace all usages, replace and test.
This is the hard part because it opens up a chance for regressions.
@asturur what do think?

Another thought

The matrix diff takes place in calcOwnMatrix. Logically that's where it should happen.
I have thoughts though of edge cases like SprayBrush.
With so many objects to iterate over, things slow down.
Group suffered from this as well.
Layer's matrix diff approach is solid but it requires to iterate over all objects.
In the SprayBrush case we don't really need/want to select the dot of the spray brush so we don't care if the dot's matrix is up to date with canvas, we simply want it to render properly, as it was in Group.
This makes me think that if the matrix diff happens in render it will leverage object caching and solve the SprayBrush case without additional logic.
However, it opens up optional trouble in case someone will want to use object caching on layer and select objects. Maybe this can be handled by the onSelect method. OR better of, maybe matrix diffing should be handled in onSelect (performance-wise it sounds great, downside would be that geometry methods won't be correct until the diff is applied).
I need @asturur to understand implications.

Contributing

Let's! Choose a to do from the to do list and go for it. I can help you getting started. I think creating/migrating tests is the right place to start. I would simply copy all relevant tests from group and add additional tests for Layer specifics (the matrix diff approach) and tests covering all issues that this PR aims to solve (see PR description). And of course docs and a demo.
Another good place to start with is the layout strategy (layout option). I made it auto but I think it can have a few options. And I think it needs more handling anyways.
I was thinking also about clipping objects in Layer. Should Layer clip them? Should this be handled by another option? I think it should, something like css overflow. Another good option to help with.

[ShaMan123]

updated comments

[ebigtm]

In my opinion, regarding the way objects should be chosen, as far as I know there are two major ways that should both be possible:

1. Select the objects inside using a double click on the parent.
2. Select the objects inside using a modifier key (usually alt).

The regular behaviour should be group selection (with bounding boz of the whole group).

however I would leave it to be decided later by whoever uses it since each case can be different, so I would not hard-code this behaviour

Regarding the contribution, sounds great!

P.s. I tried to put an absolute positioned clippath and it produces some weird glitches in the left and top edges when moving individual objects.
https://wj37y.csb.app/

When trying to add relative position clippath it didn't even render the changes for some reason.

[ShaMan123]

I am not sure how caching behaves. Clip path is rendered using the cache canvas. Compare group to layer to better understand. I'll have time later to look at it. בתאריך יום ב׳, 25 באוק׳ 2021, 10:28, מאת ebigtm ***@***.***>:

…

In my opinion, regarding the way objects should be chosen, as far as I know there are two major ways that should both be possible: 1. Select the objects inside using a double click on the parent. 2. Select the objects inside using a modifier key (usually alt). The regular behaviour should be group selection (with bounding boz of the whole group). *however I would leave it to be decided later by whoever uses it since each case can be different, so I would not hard-code this behaviour* Regarding the contribution, sounds great! P.s. I tried to put an absolute positioned clippath and it produces some weird glitches in the left and top edges when moving individual objects. https://wj37y.csb.app/ When trying to add relative position clippath it didn't even render the changes for some reason. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7449 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIGAW4OT75DSC4RBO5WQ75TUIUBKHANCNFSM5GSQP6RQ> . 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]

Just seen this, i ll try to read it all tomorrow.
I need to understand the scope of the Layer first.
i will look for

• what it solves
• what is the feature set

If you know where in the 5 linked issues those informations are, please direct me.

[asturur]

I will also start with some specific questions:
do layers support any kind of properties for their own rendering?

• stroke
• fill
• backgroundColor
• others.....
  (my idea is no, but better ask)

Can layers be part of groups?
(my idea is no....)

Can layers nest insider other layers?
(again in my opinion no...)

Please try to define how a Layer is supposed to behave so that i can read code in that context

[asturur]

Groups are not going away.
Groups are well defined concept in general, and while they may not support extra features, this does not make them deprecated.

[ShaMan123]

The latest commits fixed object stacking when selecting. Of course this needs to be tested with n>2 levels of nesting.
The major thing I can't get working is getObjectsBoundingBox when instance is rotated. These calculation are very unintuitive.
After that's solved I think this is mature enough to be merged.

[ShaMan123]

active selection tests need adjusting

[ShaMan123]

Anyone interested in trying this out:
Modify onSelect method of object and of layer (which will be renamed soon) to achieve different UX

[ShaMan123]

closed by #7473