Skip to content
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

[Bug]: Adding objects to group works differently (incorrectly?), when added initially or after the group is created. #8944

Closed
7 tasks done
bladerunner2020 opened this issue May 21, 2023 · 12 comments

Comments

@bladerunner2020
Copy link

CheckList

  • I agree to follow this project's Code of Conduct
  • I have read and followed the Contributing Guide
  • I have read and followed the Issue Tracker Guide
  • I have searched and referenced existing issues and discussions
  • I am filing a BUG report.
  • I have managed to reproduce the bug after upgrading to the latest version
  • I have created an accurate and minimal reproduction

Version

6.0.0-beta6

In What environments are you experiencing the problem?

No response

Node Version (if applicable)

None

Link To Reproduction

https://codesandbox.io/s/fabric-group-zij6s0?file=/src/index.ts

Steps To Reproduce

  1. Create an empty group with top and left coordinates then add to canvas
  2. Add objects to the group => initial coordinates of the group are ignored
  3. Create a group with objects => group keeps its coordinates

Expected Behavior

The group should keep the initial coordinates.
The behaviour of adding objects before creating group or after should be the same.

The 2 groups in example should have the same coordinates.

Actual Behavior

The coordinates of groups are different.

Error Message & Stack Trace

No response

@ShaMan123
Copy link
Contributor

No it should not.
You should read the layout strategy part.
I thought this was exposed but I guess it is in a pending PR

_onRelativeObjectAdded(object: FabricObject) {
this.enterGroup(object, false);
this.fire('object:added', { target: object });
object.fire('added', { target: this });
}

@ShaMan123
Copy link
Contributor

layout === 'fit-content' by default, change it or subclass or PR as you see fit

@ShaMan123
Copy link
Contributor

@ShaMan123
Copy link
Contributor

class MyGroup extends Group {
  _onObjectAdded(...args) {
    return super._onRelativeObjectAdded(...args);
  }
}

Parcel is so bad! It doesn't let me run this code snippet

@bladerunner2020
Copy link
Author

bladerunner2020 commented May 22, 2023

Thank you very much for your reply. After looking into the code and your comments, I got a feeling that I started to understand the logic behind this behaviour...

layout === 'fit-content' by default, change it or subclass or PR as you see fit

What is the difference between fit-content and fit-content-lazy and clip-path?

Parcel is so bad! It doesn't let me run this code snippet

If I understood you correctly you failed to run your code due to the wrong target in the default tsconfig.json. I created another sample with this calss
https://codesandbox.io/s/fabric-group-zij6s0?file=/src/index.ts

@ShaMan123
Copy link
Contributor

Soon when the website is up I will write an extensive guide about group and group layout
But for now that doesn't help you so:

  • fit-content: the group resizes around the objects. If an object is moved/modified the group resizes.
  • fit-content-lazy: same as fit-content for specific situations where perf can be improved. No need to use it unless you are dealing with mega groups and hitting a problem
  • clip-path: group resizes around the clip path

The layout mechanism is built for fine grained control. It isn't good enough yet though. But has no prominent bugs.

@bladerunner2020
Copy link
Author

Thank you again. I think I understand now (more or less) how grouping works.
In fact I want to be able to have object coordinates relative from group coordinates.
I understand how to do it with redefining _onObjectAdded, but I didn't understand what _onRelativeObjectAdded does. From name it should have doing what I need, but its actual behaviour is quite strange and far from what I want )))

@bladerunner2020
Copy link
Author

Looked at the code and found out 'objectsRelativeToGroup' option...
That seems what I need...

@bladerunner2020
Copy link
Author

@ShaMan123 I've got more questions...

  • when I add an object to a group do I need to call canvas.renderAll()? As I found out if I add an object to a group not immediately but in some period of time the Canvas is not updated, but should be?
  • after an object added to a group its left and top coordintates becomes negative. why?

@ShaMan123
Copy link
Contributor

  • when I add an object to a group do I need to call canvas.renderAll()? As I found out if I add an object to a group not immediately but in some period of time the Canvas is not updated, but should be?

You should call renderAll, requestRenderAll better in most cases.
renderOnAddRemove is what makes canvas render when you add an object to it.
That might go away in the future

  • after an object added to a group its left and top coordintates becomes negative. why?

Please take take time to read discussions labelled with group
I remember explaining this thoroughly somewhere.
#7670 is a must to read.
The short answer is relative coordinate systems. All objects inside a group relate to its center

@bladerunner2020
Copy link
Author

Thanks. As far as I understand requestRenderAll() should help to avoid unnecessary renders and we should use it instaed of renderAll in the most cases?

@ShaMan123
Copy link
Contributor

Correct
And it is timed with the tick of the frames

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants