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

getBoundingRect doesn't account for canvas zooming #1745

Closed
ryan-digbil opened this issue Oct 13, 2014 · 12 comments
Closed

getBoundingRect doesn't account for canvas zooming #1745

ryan-digbil opened this issue Oct 13, 2014 · 12 comments
Milestone

Comments

@ryan-digbil
Copy link

While the object properties account for canvas zooming, getBoundingRect doesn't return an object that accounts for the current zoom level and pan of the canvas.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@ggmendez
Copy link

ggmendez commented Nov 4, 2014

Hi,
I have come across this problem today. I have tried to solve it using the getZoom method from the canvas object but, so far, I have not been able to figure out the numeric relations between the zoom level and the bounding rect of an object.

@ryan-digbil
Copy link
Author

I solved this on my project. When I get into work in 10 hours I will share
my solution with you.
On Nov 5, 2014 12:43 AM, "ggmendez" [email protected] wrote:

Hi,
I have come across this problem today. I have tried to solve it using the
getZoom method from the canvas object but, so far, I have not been able to
figure out the numeric relations between the zoom level and the bounding
rect of an object.


Reply to this email directly or view it on GitHub
#1745 (comment).

@ggmendez
Copy link

ggmendez commented Nov 4, 2014

That would be great! Will be expecting for news from you on this issue. Many thanks in advance!

@ryan-digbil
Copy link
Author

https://gist.github.com/ryan-digbil/21b6b0fc9974b3ab29be

There you go! You might need to alter it a bit to get it to work with your project.

If you have any issues just let me know.

@ggmendez
Copy link

ggmendez commented Nov 5, 2014

Your solution works perfectly in my application! Thank you very much for sharing the knowledge! :)

@lastobelus
Copy link

@ggmendez, @ryan-digbil I'm curious to know when you would want a viewportTransform transformed bounding rect?

I think it is definitely not a bug that the current implementation doesn't transform the bounding rect; normal use of the bounding rect is to calculate things relative to other pre-transformed coordinates, but it could be useful to have another method as you implemented.

@asturur
Copy link
Member

asturur commented Jan 22, 2015

We have a mixed situation i think.

Canvas zoom came later from a external PR that has been merged.
While canvas zooming and panning mostly work with everything is not perfect yet, some aspect have not been covered.

Adding functionalities often changes the meaning of some other functions but methods do not get renamed for this.

Probably have both methods would be usefull AFTER a review of current situation ( what getBoundingBox does and if some other internal method uses it and why)

@asturur
Copy link
Member

asturur commented Jun 1, 2015

The bounding rect is based on canvasZooming as well.
The problem is that calling the function does not force recalculation of setCoords();
if you call setCoords() on the object before asking for bouding box you will have correct values.

Maybe we should do it automatically?
we always avoided calling setCoords automatically.

@dhakan
Copy link

dhakan commented Jan 8, 2016

I'm having what I assume is a similar issue to this one, where the bounding box is actually in the wrong place when applying zooming( zoomToPoint) and/or panning(relativePan) to the canvas.

My usecase is a set group of fabric.Rect(created programmatically), where I'm trying to select individual objects within this group by clicking on them. Whenever I click on the group, I use discardActiveGroup, and then directly setActiveObject for the object whose coordinates are containsPoint with the mouse coordinates.

I'm able to verify that the compensation fix above from @ryan-digbil is outputting the correct coordinates, but I do not know how to use this to update the bounding box(meaning, visually to use the updated coordinates) to account for the zooming or panning of the canvas, nor why this is happening in the first place.

I was digging through some of the source code also, without success. Neither did I get setCoords to work.

Any thoughts?

tl;dr: I've been looking for some kind of equivalent to pointer-events: none in CSS, meaning making the group containing the objects not grab the focus upon selecting.

EDIT:

I'm suspecting the discardActiveGroup + setActiveObject combination could be the problem here, since the first call might be related to grouped selections only made with the mouse. I'd rather want to just transfer the focus from the group to the object within the group.

@asturur
Copy link
Member

asturur commented Jan 8, 2016

Did you try my comment above your post?

@dhakan
Copy link

dhakan commented Jan 8, 2016

I did try to call setCoords before calling getBoundingRect without success, sorry for not clarifying. But I don't have a usecase where I want to use the values of the getBoundingRect, or is the call actually supposed to put the bounding box back to it's place?

@asturur asturur added this to the 2.0.0 milestone Sep 11, 2016
@asturur
Copy link
Member

asturur commented Nov 14, 2016

closed with #3417
Now getBoundingRect returns always updated coordinates and with a boolean flag you can choose to have it at canvas level or at zoomed canvas level.

@asturur asturur closed this as completed Nov 14, 2016
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

6 participants