Skip to content

Make SubCameraView into a new primitive shape #19302

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

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

Conversation

ecoskey
Copy link
Contributor

@ecoskey ecoskey commented May 20, 2025

Step 2 of the camera restructure!

Motivation: In the proposal, I've stolen the SubView name to represent a component that requests a viewport proportional to the size of the render target managed by a compositor. Funnily enough, that means keeping the exact type definition of CameraSubView but using it for a different purpose, which I feel warrants splitting out the idea of a proportional "sub-rectangle" into its own type.

  • Renamed SubCameraView to SubRect and move to bevy_render::primitives (bikeshedding badly needed for the name)
  • added several utility methods. These mostly consume Self, with the idea being that most of the "frontend" components in the proposal are immutable.
  • renamed Camera::sub_view to Camera::crop and edited docs. I think this name better reflects its purpose of limiting the extents the camera actually renders to, and avoids conflicting with future alternate usage of "SubView"

Testing

  • Tested examples. They're currently broken lol

TODO:

  • fix examples
  • fix SubRect::inset_by (omitted from this pr, it's non-trivial I think)
  • write migration guide

@ecoskey ecoskey added the A-Rendering Drawing game state to the screen label May 20, 2025
@ecoskey ecoskey added S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Needs-Release-Note Work that should be called out in the blog due to impact D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes and removed C-Feature A new feature, making something new possible labels May 20, 2025
Copy link
Contributor

It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note.

Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes.

Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19302

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

1 similar comment
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19302

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19302

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

1 similar comment
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19302

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19302

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@ecoskey ecoskey marked this pull request as ready for review May 21, 2025 22:05
@ecoskey ecoskey removed the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label May 22, 2025
@ecoskey ecoskey removed the M-Needs-Release-Note Work that should be called out in the blog due to impact label May 22, 2025
@ecoskey ecoskey added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label May 22, 2025
ecoskey added 3 commits May 21, 2025 18:57
this will be relevant in later prs, but handled by a different struct
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-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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