Allow camera viewports to be specified as a percentage of the parent viewport

[LoipesMas]

Objective

• Closes Viewport size and position to be defined through percentages or absolute values #4960

Solution

• Use enums for viewport position and size, and convert when needed

---

Changelog

Added:

• ViewportSize and ViewportPosition enums
• Camera::viewport_absolute_size function to reduce code repetition

Changed:

• Viewport's position and size to use new enums
• TrackedRenderPass::set_camera_viewport function to take ExtractedCamera as argument (needed to calculate absolute values)

[LoipesMas]

I had to change the signature of this function, because I needed camera to calculate absolute values of viewport.
Alternatives would be:

• have caller clone the viewport and calculate those values themselves (or make it a method on Viewport) and then pass that new "derived" viewport to this function. but then someone could forget to do that
• "bake in" absolute values into viewport (i.e. only use absolutes in viewport) but tbf I'm not sure when would this have to happen (and it wouldn't be responsive)

[alice-i-cecile]

I definitely don't want the second option; that seems much harder to maintain and responsivity is important.

[superdump]

Instead of passing in the entire extracted camera, could you instead just pass in the physical target size as that is the only thing that is used?

[LoipesMas]

Yeah, that works too

[LoipesMas]

Done

[alice-i-cecile]

Suggested PR title change: "Allow camera viewports to be specified as a percentage of the parent viewport"

[LoipesMas]

Updated the PR to use two separate enums for size and position.

I'm still not fully satisfied with this solution, but it can be changed later, maybe when doing similar thing for bevy_ui

[alice-i-cecile]

PR description needs updating now :)

[superdump]

Hmm, I feel like if this method takes an Option<> then it should handle it gracefully, not panic. So I guess either we need to find a way to handle this gracefully, or this shouldn’t take an Option.

[LoipesMas]

First of all, I'm not sure when physical_target_size can be None.
I think this is the only place where it's set:

bevy/crates/bevy_render/src/camera/camera.rs

Line 424 in 9c116d5

physical_target_size: Some(target_size),

So it looks like it's always Some.
Maybe it shouldn't be an Option at all?

Also, try_as_absolute takes an Option because this value isn't required if the size/position is an absolute value. So requiring this to always be some value isn't great either. (Because absolute value without context is still valid)

[tychedelia]

It does appear that it's always Some as far as I can tell, since we wouldn't even extract the window if it's not. I think this can be refactored on ExtractedView.

[NthTensor]

This appears to be stale, but I think this feature is wanted. I propose we close this an open an issue for the feature.

[alice-i-cecile]

Agreed. Close once the issue is up.

[tychedelia]

Looks good sans the one comment about accepting a UVec2 instead of an option.

[tychedelia]

It does appear that it's always Some as far as I can tell, since we wouldn't even extract the window if it's not. I think this can be refactored on ExtractedView.

[bas-ie]

Backlog cleanup: closing due to inactivity, although looks like we got pretty close here! Can be reopened via tracking issue #4960 if anyone feels like adopting.