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

Add getRotatedRectBounds helpers #2298

Merged
merged 23 commits into from
Jan 19, 2022
Merged

Conversation

Geokureli
Copy link
Member

@Geokureli Geokureli commented Mar 8, 2021

Add helpers to FlxRect that calculate the globally-aligned bounding box of a rotated rectangle, and similarly the GABB of a FlxObject and FlxSprite. Useful for determining the actual world space of a rotated rect for things like calculating redraw regions or broad phase collision (like getting a rect quadtree bounds).

I made this when I was looking into issues with pixelPerfectCheck, I thought this might optimize it, preventing it from iterating pixel data when the sprites are far away from each other. that's still in progress, and might not even be fruitful.

I wanted to see if this utility was up to standards, or even worthwhile on its own, before I invested my time into that application of it.

Here's a demo: https://www.newgrounds.com/projects/games/1603054/preview

@Gama11
Copy link
Member

Gama11 commented Mar 8, 2021

Tbh, my first thought is that FlxRect shouldn't know about FlxSprite or FlxObject. :/

@Geokureli
Copy link
Member Author

Geokureli commented Mar 8, 2021

originally this was in flxCollision. Austin mentioned that people may not look there. i can put them in FlxObject/FlxSprite? or maybe FlxSpriteUtil?

@Geokureli Geokureli marked this pull request as draft March 8, 2021 16:29
@Geokureli
Copy link
Member Author

Geokureli commented Mar 8, 2021

converting back to a draft for some drastic changes, namely

  • renaming getRotatedRectBounds to calcRotatedBounds since "get" implies simple data retrieval.
  • convert the static FlxRect method to an instance method
  • finding new places for FlxSprite and FlxObject versions of this.
  • unit tests

@Geokureli Geokureli marked this pull request as ready for review March 9, 2021 03:00
@Geokureli
Copy link
Member Author

@Gama11 any more thoughts on this?

@Gama11
Copy link
Member

Gama11 commented Jul 27, 2021

Not sure how I feel about a method called calcCollisionBounds() on FlxSprite and FlxObject that respects rotation, considering collisions in Flixel don't respect rotation... So these actually aren't the collision bounds in flixel terms, which could lead to confusion.

In general, I'd be happier with this if they were actually needed for something within flixel, i.e. that optimization you mentioned.

@Geokureli
Copy link
Member Author

I think I already have the optimization in another branch, I can add it here with some performance info.

got anything shorter than calcRotatedCollisionBounds?

@Geokureli
Copy link
Member Author

Geokureli commented Sep 7, 2021

I've added the optimization to FlxCollision.pixelPerfectCheck

Here is a simple video of the performance difference: https://imgur.com/ISFuHcj
And you can play it yourself here (follow directions in authors comments): https://www.newgrounds.com/projects/games/1574925/preview

The old method checked a bounds much bigger than what was needed. the benefits of this change are bigger as the dimensions of the image increase, especially a long skinny image like the spikes on flappybalt. with the previous method it's bounds were like half the screen even though it's only 9 px wide

I've also renamed calcCollisionBounds to calcRotatedBounds and calcGraphicBounds to calcRotatedGraphicBounds

@Geokureli Geokureli marked this pull request as draft September 10, 2021 19:10
@Geokureli
Copy link
Member Author

Geokureli commented Sep 10, 2021

Did some further testing, I didn't realize that pixelPerfectCheck ignores the sprite's scale, when I apply random scaling on the PPC demo's sprites I actually get worse performance because it's checking a larger bounds than it did previously.

it would be neat to honor scale in pixel perfect overlap (it already works with flipX/Y), but for now it may be better to just ignore scale in the bounds check. I may add an arg to calcRotatedGraphicBounds to ignore/allow scale but that seems kinda ugly.

input appreciated

@Geokureli
Copy link
Member Author

Geokureli commented Sep 15, 2021

Apparently it's pretty easy to get pixelPerfectCheck to work with scale, so I added it. Check it out! Yes, I'm planning many updates to the demo, alongside this.

I renamed calcRotatedGraphicBounds to getScreenBounds to match getScreenPosition. I renamed all the calcRotatedBounds methods back to getRotatedBounds, as well.

@Geokureli Geokureli marked this pull request as ready for review September 15, 2021 05:12
@Geokureli
Copy link
Member Author

Geokureli commented Sep 15, 2021

That's weird. I forgot to update the tests but all tests passed.
Edit: it's probably because I pushed before marking the PR ready

@Geokureli Geokureli marked this pull request as draft September 20, 2021 15:54
@Geokureli
Copy link
Member Author

Geokureli commented Sep 20, 2021

Found some other uses and optimizations for this. I also wanted to look into pixelPerfectPosition, pixelPerfectRender and camera zoom to see if those should be considered for these tools

The scope of this change seems to be much bigger than initially thought, if you think it's getting out of hand, let me know

@Geokureli Geokureli marked this pull request as ready for review September 20, 2021 22:04
@Geokureli Geokureli closed this Sep 20, 2021
@Geokureli Geokureli reopened this Sep 20, 2021
@Geokureli
Copy link
Member Author

Geokureli commented Sep 20, 2021

in summary this adds:

  • FlxRect.getRotatedBounds
  • FlxObject.getRotatedBounds
  • FlxSprite.getRotatedBounds - override
  • FlxSprite.getScreenBounds - similar to FlxSprite.getScreenPosition
  • FlxSpriteUtil.cameraWrap - similar to FlxSpriteUtil.screenWrap but uses FlxDirectionFlags instead of 4 booleans
  • FlxSpriteUtil.cameraBounds - similar to FlxSpriteUtil.cameraWrap
  • FlxCamera.getViewRect - helper for FlxSpriteUtil.cameraWrap and cameraBound
  • FlxCamera.containsRect- helper for FlxSpriteUtil.cameraWrap and cameraBound

Made changes to:

  • FlxCollision.pixelPerfectCheck - uses getScreenBounds to reduce the number of pixel checks, honors FlxSprite's scale
  • FlxSprite.isOnScreen - uses getScreenBounds for more accuracy
  • FlxCamera.containsPoint - calls putWeak

@Geokureli Geokureli merged commit f311df4 into HaxeFlixel:dev Jan 19, 2022
@Geokureli Geokureli deleted the rotated_rect branch January 24, 2022 20:33
Geokureli added a commit that referenced this pull request Jan 26, 2022
* Enable workflow_dispatch

* Fixing little error for android build (#2407)

* Explain FlxG singletons in input util docs (#2411)

* Mention color in docs for FlxBitmapFont.fromMonospace (#2412)

This makes it a little more clear that if you're loading a FlxBitmapFont from a monospace font file that you wish to change the color of, ensure the pixels are white. This allows the textColor property to work as expected. If the pixels happen to be black, the textColor changes won't be visible.

* redraw sprite before `updateHitbox()`. fixes #2413 (#2417)

* Remove references to FlxObject direction flag constants (#2424)

* fix: image showcase link (#2438)

* Fixed spelling in comment in FlxBasePreloader (#2447)

* Update documentation on scale modes. (#2455)

* Update license link (#2464)

* remove redundant modifier (#2467)

* Fix CI

* Add FlxAnimationController.getNamesList() to get the current list of animation names (#2473)

* Add the list thing

* Oops, beta description

* Kinda a fix?

* Experimental stuff with setting new names lol

* Adds a more safe option in case you don't complete the entire thing

* remove the variable and add the functions

* change some minor stuff

* remove the last _list things

* Update FlxAnimationController.hx

* Idk if it works but maybe the checks are failing cos of that?

* Add a nicer code and description stuff

* add tests, and small corrections

* Add the rename test on FlxAnimationControllertest

Co-authored-by: George FunBook <[email protected]>

* fix typo in unit tests

* Add getRotatedRectBounds helpers (#2298)

* `FlxRect.getRotatedBounds`
* `FlxObject.getRotatedBounds`
* `FlxSprite.getRotatedBounds` - override
* `FlxSprite.getScreenBounds` - similar to `FlxSprite.getScreenPosition`
* `FlxSpriteUtil.cameraWrap` - similar to `FlxSpriteUtil.screenWrap` but uses FlxDirectionFlags instead of 4 booleans
* `FlxSpriteUtil.cameraBounds` - similar to `FlxSpriteUtil.cameraWrap`
* `FlxCamera.getViewRect` - helper for `FlxSpriteUtil.cameraWrap` and `cameraBound`
* `FlxCamera.containsRect`- helper for `FlxSpriteUtil.cameraWrap` and `cameraBound`
* `FlxCollision.pixelPerfectCheck` - uses getScreenBounds to reduce the number of pixel checks, honors FlxSprite's scale
* `FlxSprite.isOnScreen` - uses getScreenBounds for more accuracy
* `FlxCamera.containsPoint` - calls putWeak

* ignore failing test for now

* sceenCenter Readibility (#2441)

* add screenCenter test

* improve readability of screenCenter

* add check with no parameters

* inline screenCenter

* update FlxState comment on constructor

* Prevent hashlink segfaults on linux (#2487)

* change integers used for ANY and NONE FlxKey <REVERTED>

* add FlxKey tests for ANY/NONE

* remove switch causing seg faults

Co-authored-by: jf <[email protected]>
Co-authored-by: George FunBook <[email protected]>

* call members' kill/revive when called on sprite groups (#2423)

* call members' kill/revive when called on sprite groups

* make sure kill/revive is NOT called when not needed

* fix spriteGroup.reset()

* minimize setters

* add resetTest

* remove haxe4 compile conditions (#2489)

* Release 4.11.0 (#2490)

* add release notes

* seg fault bugfix

* FlxSpriteGroup kill/revive

* change haxelib

* wording

* change date

Co-authored-by: Jens Fischer <[email protected]>
Co-authored-by: Gliar <[email protected]>
Co-authored-by: Brett Chalupa <[email protected]>
Co-authored-by: Julian Holfeld <[email protected]>
Co-authored-by: RafPlayz69YT <[email protected]>
Co-authored-by: Eric Myllyoja <[email protected]>
Co-authored-by: MrClogsworthYT <[email protected]>
Co-authored-by: MrCheemsAndFriends <[email protected]>
Co-authored-by: jobf <[email protected]>
Co-authored-by: jf <[email protected]>
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.

2 participants