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

Incorporate viewport position into clamping in camera_system #18242

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mweatherley
Copy link
Contributor

Objective

In its existing form, the clamping that's done in camera_system doesn't work well when the physical_position of the associated viewport is nonzero. In such cases, it may produce invalid viewport rectangles (i.e. not lying inside the render target), which may result in crashes during the render pass.

The goal of this PR is to eliminate this possibility by making the clamping behavior always result in a valid viewport rectangle when possible.

Solution

Incorporate the physical_position information into the clamping behavior. In particular, always cut off enough so that it's contained in the render target rather than clamping it to the same dimensions as the target itself. In weirder situations, still try to produce a valid viewport rectangle to avoid crashes.

Testing

Tested these changes on my work branch where I encountered the crash.

@mweatherley mweatherley added D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Bug An unexpected or incorrect behavior labels Mar 10, 2025
@mweatherley mweatherley requested review from JMS55 and tychedelia March 11, 2025 15:11
@tychedelia tychedelia requested a review from IceSentry March 11, 2025 19:26
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 C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

1 participant