-
-
Couldn't load subscription status.
- Fork 4.2k
Open
Labels
A-RenderingDrawing game state to the screenDrawing game state to the screenC-BugAn unexpected or incorrect behaviorAn unexpected or incorrect behaviorC-UsabilityA targeted quality-of-life change that makes Bevy easier to useA targeted quality-of-life change that makes Bevy easier to useD-ComplexQuite challenging from either a design or technical perspective. Ask for help!Quite challenging from either a design or technical perspective. Ask for help!S-Ready-For-ImplementationThis issue is ready for an implementation PR. Go for it!This issue is ready for an implementation PR. Go for it!
Description
Supercedes #20251
Context
#15537 introduced the SubCameraView API, by copying the equivalent API from three.js. This resulted in a subpar implementation, however, with confusing documentation and undocumented panic conditions, amongst other things.
The only public API for this feature is the sub_camera_view field on Camera, which is an Option<SubCameraView>. The SubCameraView type is defined as:
pub struct SubCameraView { pub full_size: UVec2, pub offset: Vec2, pub size: UVec2, }If set, this is used to calculate a new, smaller frustum for the camera, which is used for rendering instead of the camera's full frustum.
The Problems
This smaller frustum must be entirely contained within the base, unmodified frustum. If it isn't, the current impl throws a WGPU validation error, which in turn throws a panic. As the API is just a fewThis is caused by the camera's viewport going outside of the window, not directly related topubfields, with no helper methods, there are no safeguards to prevent this from happening.SubCameraView.- The size of the new frustum is defined as a fraction of the size of the base frustum. Instead of specifying this fraction directly, it is defined implicitly by the component-wise ratio of the values of the
sizeandfull_sizefields. This is unnecessarily overcomplicated and confusing. - The documentation is lackluster and fails both to explain how to use this feature, and to justify when to use it. The above panic conditions are also not mentioned anywhere.
- There is an example that showcases
SubCameraView, but it is unhelpful and hard to understand.
The Refactor
- The
sizeandfull_sizefields should be folded into a single field that directly describes the scaling factor. - They should be
Vec2instead ofUVec2, because they are already converted directly tof32everywhere they are used. Alternatively, instead of scaling being done component-wise with a singleActually, there should only be a singleVec2, have twof32fields, one for each axis.f32scaling value. Using a different scaling factor for each axis can change the aspect ratio of the subview to be different than that of the full frustum/viewport (which have their aspect ratios automatically kept in sync), which just causes the image to become distorted. I don't see a reason to want to do this intentionally.None of the fields should bepub, instead there should be methods that enforce the invariants needed to not panic.SubCameraViewdoesn't actually cause panics, although helper methods may still be useful.- The documentation should explain how it actually works, including the units used, and examples of motivating use cases.
- The example should be simplified and reworked to clearly demonstrate the effects that different
SubCameraViewvalues have, compared to a camera that doesn't use the feature.
Metadata
Metadata
Assignees
Labels
A-RenderingDrawing game state to the screenDrawing game state to the screenC-BugAn unexpected or incorrect behaviorAn unexpected or incorrect behaviorC-UsabilityA targeted quality-of-life change that makes Bevy easier to useA targeted quality-of-life change that makes Bevy easier to useD-ComplexQuite challenging from either a design or technical perspective. Ask for help!Quite challenging from either a design or technical perspective. Ask for help!S-Ready-For-ImplementationThis issue is ready for an implementation PR. Go for it!This issue is ready for an implementation PR. Go for it!