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

Object geometry methods for grouped objects #7130

Closed
ShaMan123 opened this issue Jun 18, 2021 · 3 comments · Fixed by #7858
Closed

Object geometry methods for grouped objects #7130

ShaMan123 opened this issue Jun 18, 2021 · 3 comments · Fixed by #7858
Labels

Comments

@ShaMan123
Copy link
Contributor

ShaMan123 commented Jun 18, 2021

Version

latest

Test Case

http://jsfiddle.net/mkdve1x9/15/

Information about environment

Browser.

Steps to reproduce

  1. Create a Group/ActiveSelection
  2. Try using a method from object_geomerty on an object that belongs to the group

Expected Behavior

Should work for nested objects without additional code

Actual Behavior

It returns wrong values because the object's transform matrix is used and is relative to the group

@ShaMan123 ShaMan123 changed the title Object geometry methods for nested objects Object geometry methods for grouped objects Jun 18, 2021
@asturur
Copy link
Member

asturur commented Jun 20, 2021

Yes.
The more i look at it and the more i think all the whole aCoord,oCoord is, excuse me the term, fucked up.
Fix it is a mess, it requires a full rewrite of so many internals and a ton of regression test/unit tests.

Ideally we want to get rid of oCoords entirely and all that absolute: true/false on those kind of geometry functions.
The only way to fix this is to add inside a

var points = this.getCoords(true, calculate);
// right after 
if (this.group) {
// take group transform matrix
// multiply the points for group transform matrix
}
// proceed with normal checks
if (points.some(function(point) {

If you want to open a small pr for this, go for it.

@asturur asturur added the bug label Jun 20, 2021
@ShaMan123
Copy link
Contributor Author

Man I absolutely agree.
You rock.

@ShaMan123
Copy link
Contributor Author

#7599

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