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

fix(fabric.Object) bounding box display when outside group #7611

Merged
merged 2 commits into from
Jan 15, 2022

Conversation

asturur
Copy link
Member

@asturur asturur commented Jan 15, 2022

closes #7429

The generic option angle can't be used when we draw the bounding box of an object with skewY, the reason is not super clear to me right now, but has probably to do with the fact that the calculation of the boudning box size do consider skewY while the decomposition removes skewY.

So the code is drawing a boudning box with this angle

image

the angle is wrong.
but this is because the angle is calculated for an object that has lost skewY after a reduction.

So the angle is correct for the same object, but with this different size

image

before code changes

image

after code changes

image

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2022

Code Coverage Summary

> fabric@4.6.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.38 |    77.11 |   86.16 |    83.1 |                                               
 fabric.js |   83.38 |    77.11 |   86.16 |    83.1 | ...,30173,30298,30378-30443,30566,30665,30882 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur
Copy link
Member Author

asturur commented Jan 15, 2022

image

First time i manage to use stylesOverride for something useful

@asturur
Copy link
Member Author

asturur commented Jan 15, 2022

@ShaMan123 this add a test for the bug you are seeing in your PR.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2022

Code Coverage Summary

> fabric@4.6.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |    83.4 |    77.15 |   86.16 |   83.12 |                                               
 fabric.js |    83.4 |    77.15 |   86.16 |   83.12 | ...,30173,30298,30378-30443,30566,30665,30882 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur
Copy link
Member Author

asturur commented Jan 15, 2022

(the controls or the controlbox with skewY are not rendered correctly inside a group)

@asturur asturur merged commit ff53792 into master Jan 15, 2022
@asturur
Copy link
Member Author

asturur commented Jan 15, 2022

Ideally to fix it properly, we stop to use strokeRect and we move between the 4 main coordinates with lines. period.

@asturur asturur mentioned this pull request Jan 26, 2022
@asturur asturur deleted the skewY-controls branch February 4, 2022 17:53
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

Successfully merging this pull request may close these issues.

The controls for the skewed objects are in the wrong place.
1 participant