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 frustum culling w/ sprite transform scaling #2753

Closed
wants to merge 5 commits into from

Conversation

fuzzbuck
Copy link

@fuzzbuck fuzzbuck commented Aug 30, 2021

Objective

Solution

  • Take sprite's Transform.scale into account when measuring whether camera_rect & sprite_rect intersect each other

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Aug 30, 2021
@rparrett
Copy link
Contributor

Just want to point out that my test case and the one from the author of that issue seem to be showcasing two separate issues.

In my test case, sprite scaling is unaccounted for.

In that issue, it seems that camera scale is unaccounted for.

@fuzzbuck
Copy link
Author

fuzzbuck commented Aug 30, 2021

Just want to point out that my test case and the one from the author of that issue seem to be showcasing two separate issues.

In my test case, sprite scaling is unaccounted for.

In that issue, it seems that camera scale is unaccounted for.

That issue happens because sprites outside of camera bounds are not drawn, regardless of window size
I'm not sure whether this is expected behavior or not

Perhaps each camera should have its own frustum_enabled setting?
With additional setting that dictates whether it takes into account camera transform scale, or just the window size

@fuzzbuck
Copy link
Author

fuzzbuck commented Aug 30, 2021

It does not seem like #1760 happens because of camera Transform scaling ?

When scaling the camera below 1.0, the sprite just disappears completely (even with frustum feature disabled), so I don't think this is related to frustum

uTWqJWAkPr.mp4

@fuzzbuck
Copy link
Author

Ok, in summary:

  • This PR fixes frustum culling on sprites scaled by Transform
  • It adds an example that showcases frustum culling

There are plethora of other issues but I think that can be resolved in other PRs (or v0.6)

@rparrett
Copy link
Contributor

Yeah it is possible that #1760 is actually describing some separate issue related to sprite sheets or something.

Just want to make sure #1760 doesn't get closed if this is merged, because I'm pretty sure my repro is not the bug originally described.

@alice-i-cecile alice-i-cecile added A-Transform Translations, rotations and scales C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels Sep 4, 2021
@alice-i-cecile
Copy link
Member

#2861 looks like it will obsolete this; is that correct?

@superdump
Copy link
Contributor

#2861 looks like it will obsolete this; is that correct?

I suppose that practically anything against the 'old' renderer is obsoleted by the new renderer. That said, I didn't actually implement culling for 2D, just visibility propagation.

@cart and I were on the fence about whether culling would bring benefits over just culling on the GPU and opted to leave it for later as batched sprite rendering already has great performance on pipelined-rendering anyway and it may not be a clear win. That said, if someone wanted to add sprite rect vs view rect intersection testing for 2D on top of #2861 and check whether it improves performance, I'm sure no one would complain. :)

@alice-i-cecile
Copy link
Member

Closing as no longer relevant <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-Transform Translations, rotations and scales C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants