fabricjs / fabric.js

Javascript Canvas Library, SVG-to-Canvas (& canvas-to-SVG) Parser
http://fabricjs.com
Other
28.74k stars 3.49k forks source link

`V6`: πŸŽ‰ Group Rewrite #7670

Closed ShaMan123 closed 4 months ago

ShaMan123 commented 2 years ago

V6

Hi everyone, fabric is moving forward! We have 2 main goals for v6:

  1. js/ts migration which is covered here #7596
  2. Group rewrite πŸŽ‰

You can see #8316

Group Rewrite

This is a tracking issue for the group rewrite, read below for more detail including the current state of things and gist. We aim to fix and support long desired features including:

  1. Nested selection - DONE
  2. Resizing and customized layout - DONE
  3. Transform and coordinate system (which is relative to the parent group) - DONE

Motivating Issues/Discussions

7473 https://github.com/fabricjs/fabric.js/pull/7316 https://github.com/fabricjs/fabric.js/issues/6776 https://github.com/fabricjs/fabric.js/discussions/7136 https://github.com/fabricjs/fabric.js/discussions/7299 https://github.com/fabricjs/fabric.js/issues/7130 https://github.com/fabricjs/fabric.js/issues/7142 #7449 and anything with a group tag

How do you fit in?

  1. You can become an alpha tester (we aim to publish an alpha tag to npm regularly). IMPORTANT NOTICE: all changes made to the v6 branch are experimental and can be removed or changed without notice and without proper support (including supported features from v5). fabric might change drastically between commits. Meaning if you want in - it's on you.

You can join the effort by contributing

This description is updated continuously with relevant information You don't need to scroll and read endless comments (unless you are a maintainer :))


Current State

Fixed logic across fabric for nested objects to be selectable. Group has been completely rewritten and Layer has been introduced. As part of the rewrite group is now a layout engine, you can use it to customize layout of objects. In general the rewrite allows any object to be selectable and interactive if it is referenced in the _objects array of it's parent and it references it's parent via the group property. If you build custom objects with nesting this is what you are looking for.

Usage

It is advised to use set on group/objects from now on (until #7596 is done, then it might change due to setters) Changing subTargetCheck or interactive enables/disables crucial functionality so for these props it is not an advice but a MUST.

new fabric.Group(objects, {
  subTargetCheck: true,
  interactive: true   //  enables selecting subtargets
});

Progress

Experimental

Triggering Layout

9148 extracts layout management to a standalone class

triggerLayout forces a group to recalculate its bounding box and reposition its objects. You can define a layout strategy and assign it to the layout manager. Read the code, it has comments explaining stuff and types that make it clear what you need to return from the method. Look at the modified trigger, it is possible adding more triggers. However I will promote an overall solution. #7882 is a POC, follow the progress trigger

BREAKING

Pitfalls

All object origin methods (e.g. translateToOriginPoint) are relative methods. You should use them ONLY in the correct plane and with points that belong to the relative plane. IMO it makes them worthless.

8767 changes that.

BUGS

If you encounter bugs related to group or if you have a feature request please submit a relevant ticket. Notice that bug reports of versions below v6 will be completely disregarded and closed. This was hard work, put effort into your repro so it is in v6.

Working Examples

Using v6! stale branch - CodeSandbox or JSFiddle - Layer or in JSFiddle

To Do

Look

Video is running in original speed tiger

Cutting Edge

The blue circles are rendered by a textbox and because of the rewrite with a few lines of code I got this working Pay no attention to caching of course

This is EXTREMELY exciting because it means that anyone could create custom objects subclassing whatever object they need and have nested selection work out of the box!

ezgif com-gif-maker (13)

ShaMan123 commented 2 years ago

First milestone #7858 🀟

ShaMan123 commented 2 years ago

First milestone is merged!! πŸ₯³πŸ₯³ Meaning that fabric.Group is now the new group.

ShaMan123 commented 2 years ago

updated description

ShaMan123 commented 2 years ago

updated v6-alpha branch now contains:

ShaMan123 commented 2 years ago

Second milestone merged! Most of the functionalities are now available in master! Usage:


new fabric.Group(objects, {
  subTargetCheck: true,
  interactive: true   //  enables selecting subtargets
})
ShaMan123 commented 2 years ago

4th milestone merged!

ShaMan123 commented 2 years ago

@asturur I think we can drop coords in favor of getCenterPoint + _getTransformedDimensions What do you think?

ShaMan123 commented 2 years ago

Group layout now respects origin, skewing and angle. Stay tuned for more!

ShaMan123 commented 2 years ago

Group is now in beta!

ShaMan123 commented 2 years ago

Next Up

dubem-design commented 2 years ago

How are things going?

ShaMan123 commented 2 years ago

We are prioritizing TS so the rest of the open prs will find their way in after that. You can work on the alpha branch if you need to

ShaMan123 commented 2 years ago

I am pretty sure some of the prs will be rewritten to accomodate better caching logic

rybakovanton-metta commented 2 years ago

@ShaMan123 really a lot of branches, too hard to follow latest changes. Good description in first posts but where latest code is not clear

ShaMan123 commented 2 years ago

Under the Alpha testing section it is very clear IMO

rybakovanton-metta commented 2 years ago

Under the Alpha testing section it is very clear IMO

yes, but at 16 Jun you said that Group is now in beta!

ShaMan123 commented 2 years ago

It has nothing to do with the naming of the branch

dubem-design commented 1 year ago

Just checking up ✌️

ShaMan123 commented 1 year ago

8316

ShaMan123 commented 1 year ago

Update for Groupies, Group has been migrated to TS along with ActiveSelection #8455. This means I can port the remaining PRs to v6 so that they can enter the merging queue (which isn't short). We plan to delegate the layout mechanism to a standalone class as part of the migration and for ease of customization and subclassing. I am considering adding a small feature that anchors objects to a given origin during layout. This should make logic much more simple and accessible (It takes me a lot of time and frustration to write more layout logic and I might be the only one that does it currently which says bad things about design). I am now convinced that we should introduce a mechanism that will support interactive group with the handling of caching instead of the ugly workarounds I did to make it work (I now consider them as mere POCs) so expect it to change, especially that #8298 is a valid POC and if merged will have a real impact on fabric.

enriqueerrando commented 1 year ago

Hi,

I was wondering if V6 will change how toDatalessJSON structures data.

The thing is I'm building a designer tool based on FabricJS and the user will be able to save to the db the canvas data via toDatalessJSON to retreive it later with loadFromJSON. Will V6 be able to load that data after being exported with V5?

Thanks in advance!

ShaMan123 commented 1 year ago

@enriqueerrando toDatalessJSON | toDatalessObject are methods that are used after parsing from svg. It has been changed in group. It used to have this strange option that the objects property was a link to an svg file. That was removed. The only object that actually implements this method is Path (IMO we should remove that as well and deprecate the entire concept). If you aren't using svg parsing and Path in particular, using toObject will have the same result.

So answering your question, you might need to re-export objects from v5 using toObject. I think it is a good idea we expose a link that does that... you load a json, it downloads a new one. Sure, it will be done.

timoostrich commented 1 year ago

@enriqueerrando toDatalessJSON | toDatalessObject are methods that are used after parsing from svg. It has been changed in group. It used to have this strange option that the objects property was a link to an svg file. That was removed. The only object that actually implements this method is Path (IMO we should remove that as well and deprecate the entire concept). If you aren't using svg parsing and Path in particular, using toObject will have the same result.

So answering your question, you might need to re-export objects from v5 using toObject. I think it is a good idea we expose a link that does that... you load a json, it downloads a new one. Sure, it will be done.

Glad to hear that as I also created a design tool where users saved thousand of files till now... :D

ShaMan123 commented 1 year ago

I don't store fabric raw output in db. I act on it, too much junk data.

timoostrich commented 1 year ago

Me neither, I'm saving it as a gzipped file on the server. What do you do with the data?

ShaMan123 commented 1 year ago

Depends on business logic. I am not for gzipped files since then you have to fetch all of it instead of bits of interest. But that might be suitable for your app.

ShaMan123 commented 1 year ago

I gave it another thought. Since we are planning to release a registry concept in v6 (POC #8282) I think (depending on the final design) it would be possible to add a custom/legacy resolver that patches differences in serialization between versions without any elaborate workarounds.

enriqueerrando commented 1 year ago

Hi @ShaMan123 and @timoostrich, thanks for answering ;)

No, I do not save data to the db neither, I do save it to a file too (not gzipped though), but for the example obove I thought was simpler to say so because thought that was what the majority was doing, but I can see now it isn't ;)

I use loadSVGFromURL then apply groupSVGElements to them.

If I use toObject woudn't that save all SVG code insted of referencing the URL? I was using toDatalessJSON to reduce the size of the exported data.

I gave it another thought. Since we are planning to release a registry concept in v6 (POC https://github.com/fabricjs/fabric.js/pull/8282) I think (depending on the final design) it would be possible to add a custom/legacy resolver that patches differences in serialization between versions without any elaborate workarounds.

That would be great, thanks! But since the V6 release is so close, I think I'll just wait for it to make sure all of my code works fine with it.

ShaMan123 commented 1 year ago

Fabric is almost fully migrated, Group is doing GREAT! #8461 just merged.

Remaining work:

enriqueerrando commented 1 year ago

Those are great news!! Thanks for the hard work!

timoostrich commented 1 year ago

Really looking forward to it, thank you so much for your effort!

dubem-design commented 1 year ago

Wow Great work. I can't wait

enriqueerrando commented 1 year ago

Hi @ShaMan123,

How is V6 going? I've seen there are some tasks left to finish it, but don't know how that translates in time.

Thanks a lot for your work!

ShaMan123 commented 1 year ago

Yesterday or something @asturur managed to stabalize the build. So unofficially we are in beta. A few days, a week, until we roll out the official version.

enriqueerrando commented 1 year ago

Woooow! I didn't expect that answer, hahaha!! Great news then!! Thanks for everything u're doing ;)

enriqueerrando commented 1 year ago

Hi @ShaMan123 !

How is the beta testing going?

Thanks in advance!

ShaMan123 commented 1 year ago

8622

enriqueerrando commented 1 year ago

It's looking good ;)

ShaMan123 commented 1 year ago

8665 has been merged meaning that nested mutliselection is now supported.

Still a few issues with group caching that break mutliselection. Workaround by disabling group caching in such a case.

enriqueerrando commented 1 year ago

Great! Thanks for keepping us updated!

I'm just waiting for the V.6 release to try it all together, because the web app I'm building needs several things that may change from 5 to 6, so I cannot continue 'till the update ;)

Keep up the good work!! U're almost done!

abdussami-tayyab commented 1 year ago

Is there something in v6 that can help with this usecase:

cc: @ShaMan123

ShaMan123 commented 1 year ago

Group was redesigned to be simple (apart from one mistake I hope to fix soon) So you simply call group.removeAll and then to regroup you can call group.add toadd the objects back to the original group or create a new one. That is it. However, in v6 you can transform nested objects, meaning that you do NOT need to ugroup at all. So depends on UX you could flag group.interactive instead of ungrouping.

abdussami-tayyab commented 1 year ago

There is an issue with the way Group is working for me. The interactivity (that the project I'm working on massively will rely on) is great, subtargets also work well. But there's quite a glitchy line when I move the group around. Here is the video.

And here is the CodeSandbox.

This is more or less the same way I did it with v5 (which is working), but since v6 fits exactly my use-case I want to go with it. But this is causing quite an issue, especially when the background is white (which we'll go with eventually). Another thing is that the line when moved around via it's control circle, part of it vanishes which is outside the bounding box's bounds.

Screenshot 2023-05-18 at 11 28 17 PM
ShaMan123 commented 1 year ago

This is a caching issue. Perhaps it is not well documented. Caching is still a working progress. Disable group caching. Or mark it is dirty...

abdussami-tayyab commented 1 year ago

Thanks @ShaMan123 , it is working for me with objectCaching: false on the Group object.

Sorry to pour here with questions, I realise you guys must be super-busy, but I also feel bringing up problems right now is best for all of us. A question I have is, let's say there's a Group of Rect and a Circle, and these 2 conditions are true:

  1. Rect object is bigger than the Circle
  2. Circle is inside the Rect

Is it possible to select the Group? I am unable to do that, because the Rect covers the whole Group and when I select Rect, it makes the Rect interactive (and movable).

Are there a couple of event listeners that might help selecting the Group if Rect.size === Group.size?

ShaMan123 commented 1 year ago

Rect.selectable = false?

abdussami-tayyab commented 1 year ago

It will make the rect unselectable/non-interactive which is also something I don't want. I just tried this use-case in Canva, and in Canva if I create a rectangle of 50x50 and place a circle of 30x30 inside it, then group both of them. Then when I select the group to move, the whole group moves and I am not able to move the rectangle or circle although I can change their attributes.

I wonder what event listeners we can use, for example if a user moves a group without Shift key pressed, we deal with the group but if Shift key is pressed, we can select the individual object inside the group. If this is an advanced topic for us, we can have a look at it later, but important to at least think about it because we are sure this is not an unusual case.

ShaMan123 commented 1 year ago

Did you try toggling the selectable prop in correlation to the shift key? You can also try to fiddle with onSelect, onDeselect and the related events

abdussami-tayyab commented 1 year ago

Sorry but can you please point me to that in the code from where I see how it works? Thanks.

On Sat, 20 May 2023, 02:40 Shachar, @.***> wrote:

Did you try toggling the selectable prop in correlation to the shift key? You can also try to fiddle with onSelect, onDeselect and the related events

β€” Reply to this email directly, view it on GitHub https://github.com/fabricjs/fabric.js/issues/7670#issuecomment-1555284700, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABW3JIVLM2CUAQTTGVW7QJ3XG7SGXANCNFSM5OAWEAHQ . You are receiving this because you commented.Message ID: <fabricjs/fabric. @.***>

abdussami-tayyab commented 1 year ago

@ShaMan123 I tried setting the selectable attribute of objects inside a group, the problem with that now is that moving the group now does nothing because the underlying object which has selectable: false.

Is there any way we can set interactive only on IText objects and not on any other shape?