Fix for DisplayObject/DisplayObjectContainer - getting dimensions or bounds do NOT retrieve proper values #2639
Conversation
…bounds do NOT retrieve proper values * Documentation * TypeScript Defs * Nothing, it's a bug fix Describe the changes below: I noticed that getting dimensions or bounds changed in Phaser 2.6.1 and it was bugged - returned dimensions were doubled in the direction of the scaling for example, but the nice thing was that this time the dimensions included the rotation of the object - which I think should be the default behavior. It also bugged getLocalBounds which started returning global getBounds(); When I checked the previous version of phaser 2.5.0 - total different story. getLocalBounds returns the object without any transformations to it. Getting width and height works ok, but rotation wasn't calculated to the dimensions at all. In all cases only getBounds returned proper dimensions, which is obviously not enough and this is also a very important issue to be resolved as soon as possible since it's a major core component feature used all the time. That's why I decided to spare the time and hopefully find a good fix for all of this mess and I think I finally got there and tried to make the changes as much as possible with your Code of Conduct. This is actually the first time I am requesting a pull so I hope it will do some good. I did compare my results with how Flash handles dimensions - they match, so I do hope I didn't miss something and believe you guys would make sure everything works ok. To help you with reviewing and possibly (and hopefully) accepting this pull request I've made available online 3 demos of the issues and with demo of the fix of course: 1. http://www.nedyalkov.net/filip/demos/phaser/IncorrectDisplayObjectDimensionsDemo_v2.5.0/ 2. http://www.nedyalkov.net/filip/demos/phaser/IncorrectDisplayObjectDimensionsDemo_v2.6.1/ 3. http://www.nedyalkov.net/filip/demos/phaser/IncorrectDisplayObjectDimensionsDemoFixed_v2.6.1/ Here is a zip file of the demo projects (build with VS2015): http://www.nedyalkov.net/filip/demos/phaser/IncorrectDisplayObjectDimensionsDemos.zip I really hope to see this FIX in the next version - would help me and a lot of people I guess to get the job done properly! Thank you!
|
If this fix is accepted then I guess the only thing left to change is in the docs the DisplayObjectContainer.getBounds() method that I've changed to getBounds(targetCoordinateSpace) to make it more flexible and to allow checking bounds to any target not just local and global. |
|
This is likely related to #2627 |
|
@st0nerhat yes, sorry, I didn't know how to properly mention those two commits are for the same issue. I do hope I found the proper fix for it! |
|
I mentioned that DisplayObjectContainer.getBounds(targetCoordinateSpace) allows checking bounds to any target not just local and global, but actually right now it doesn't. What it can do is:
The idea was that it will return bounds related to any targetCoordinateSpace, parent or no parent. Well it still fixes getting 1) 2) and 3). Still good enough, I was hoping it will do more than that but it needs to be reworked a bit so it could do the trick. If I have the time I will add that too or you guys could finish it if you'd like. But in any ways even if you add this fix as is, it will still be a lot better than what it is or what it was before, because it will fix the current issues we have with dimensions. |
|
I am almost done with a completely fixed version for this so I will probably close this commit and create a new one later today. |
|
Go for it. I'm away today so can't merge anything until tomorrow anyway, On Tuesday, 19 July 2016, Filip Nedyalkov notifications@github.com wrote:
Photon Storm Ltd. Skype: richard.davey Registered in England and Wales. |
|
I will create the fixed pull request very soon :) Sounds great you like it :) |
|
I am done with the fix. I think it works great. I did a lot of testing and thinking of how is the best way to make it work that makes sense. I hope you'll like it. I need a few more hours - I still need to redo a little bit the demos and write a description of the changes that makes sense. So until later ;) |
|
Don't worry, there's no rush. Spend time to get it right. 2.7 won't be released for at least a week, so you've plenty of time. |
… dimensions and bounds With the previous fix what the getBounds did was: 1) if targetCoordinateSpace is the same instance as the caller of getBounds(), then it will return the bounds of the caller without any transformations; 2) if targetCoordinateSpace is null/undefined it will return the global bounds of the caller. 3) if targetCoordinateSpace is any valid DisplayObject it will return the local bounds of the caller. What this fix does is fixing 3) along with other obsolete code that wasn't necessary so I reverted it. So now if the targetCoordinateSpace is a valid DisplayObject: - if it's a parent of the caller at some level it will return the bounds relative to it - if it's not parenting the caller at all it will get global bounds of it and the caller and will calculate the x and y bounds of the caller relative to the targetCoordinateSpace DisplayObject Also I have fixed how empty groups are treated when they have no other children except groups, so now calculations are correct. They obviously have 0 width and height but are still being positioned and other things could possibly relate to that bounds and it didn't make sense to me to ignore them. Also added a DisplayObjectContainer.contains(child) method which determines whether the specified display object is a child of the DisplayObjectContainer instance or the instance itself. This method is used in the new getBounds function. Corrected DisplayObject's default _bounds rect from (0, 0, 1, 1), to (0, 0, 0, 0) - it doesn't seem to break anything and also in the getBounds before the fix, when there were no children it assigned a (0, 0, 0, 0) rectangle to it so I am pretty sure it's safe to correct it.
|
Hi I've committed the changes to the branch on my forked rep so I see they automatically translated here - so I won't close this pull request, because It does seem it will do the job. I have updated all links with the demos so you could check them out. I really hope I didn't miss something this time. I hope I helped out and will be happy to see this in the next version :) You can read the full description of what changed and how from the initial commit in the second commit's description. Regards |
|
Thanks for opening this issue, and for submitting a PR to fix it. We have merged your PR into the |
|
Awesome! :) That's great guys. Thank you! |
The PR changes include:
I noticed that getting dimensions or bounds changed in Phaser 2.6.1 and it was bugged - returned dimensions were doubled in the direction of the scaling for example, but the nice thing was that this time the dimensions included the rotation of the object - which I think should be the default behavior. It also bugged getLocalBounds which started returning global getBounds();
When I checked the previous version of phaser 2.5.0 - total different story. getLocalBounds returns the object without any transformations to it. Getting width and height works ok, but rotation wasn't calculated to the dimensions at all.
In all cases only getBounds returned proper dimensions, which is obviously not enough and this is also a very important issue to be resolved as soon as possible since it's a major core component feature used all the time.
That's why I decided to spare the time and hopefully find a good fix for all of this mess and I think I finally got there and tried to make the changes as much as possible with your Code of Conduct. This is actually the first time I am requesting a pull so I hope it will do some good. I did compare my results with how Flash handles dimensions - they match, so I do hope I didn't miss something and believe you guys would make sure everything works ok.
To help you with reviewing and possibly (and hopefully) accepting this pull request I've made available online 3 demos of the issues and with demo of the fix of course - check the console log:
Here is a zip file of the demo projects (build with VS2015):
http://www.nedyalkov.net/filip/demos/phaser/IncorrectDisplayObjectDimensionsDemos.zip
I really hope to see this FIX in the next version - would help me and a lot of people I guess to get the job done properly!
Thank you!