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

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

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_2d/main_pass_2d_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl Node for MainPass2dNode {
let mut draw_functions = draw_functions.write();
let mut tracked_pass = TrackedRenderPass::new(render_pass);
if let Some(viewport) = camera.viewport.as_ref() {
tracked_pass.set_camera_viewport(viewport);
tracked_pass.set_camera_viewport(viewport, camera);
}
for item in &transparent_phase.items {
let draw_function = draw_functions.get_mut(item.draw_function).unwrap();
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_core_pipeline/src/core_3d/main_pass_3d_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl Node for MainPass3dNode {
let mut draw_functions = draw_functions.write();
let mut tracked_pass = TrackedRenderPass::new(render_pass);
if let Some(viewport) = camera.viewport.as_ref() {
tracked_pass.set_camera_viewport(viewport);
tracked_pass.set_camera_viewport(viewport, camera);
}
for item in &opaque_phase.items {
let draw_function = draw_functions.get_mut(item.draw_function).unwrap();
Expand Down Expand Up @@ -142,7 +142,7 @@ impl Node for MainPass3dNode {
let mut draw_functions = draw_functions.write();
let mut tracked_pass = TrackedRenderPass::new(render_pass);
if let Some(viewport) = camera.viewport.as_ref() {
tracked_pass.set_camera_viewport(viewport);
tracked_pass.set_camera_viewport(viewport, camera);
}
for item in &alpha_mask_phase.items {
let draw_function = draw_functions.get_mut(item.draw_function).unwrap();
Expand Down Expand Up @@ -186,7 +186,7 @@ impl Node for MainPass3dNode {
let mut draw_functions = draw_functions.write();
let mut tracked_pass = TrackedRenderPass::new(render_pass);
if let Some(viewport) = camera.viewport.as_ref() {
tracked_pass.set_camera_viewport(viewport);
tracked_pass.set_camera_viewport(viewport, camera);
}
for item in &transparent_phase.items {
let draw_function = draw_functions.get_mut(item.draw_function).unwrap();
Expand Down
104 changes: 91 additions & 13 deletions crates/bevy_render/src/camera/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,85 @@ use wgpu::Extent3d;
pub struct Viewport {
/// The physical position to render this viewport to within the [`RenderTarget`] of this [`Camera`].
/// (0,0) corresponds to the top-left corner
pub physical_position: UVec2,
pub physical_position: ViewportPosition,
/// The physical size of the viewport rectangle to render to within the [`RenderTarget`] of this [`Camera`].
/// The origin of the rectangle is in the top-left corner.
pub physical_size: UVec2,
pub physical_size: ViewportSize,
/// The minimum and maximum depth to render (on a scale from 0.0 to 1.0).
pub depth: Range<f32>,
}

/// An enum that represents viewport's top-left corner position either as absolute value (in pixels)
/// or as a percentage of camera's render target's size.
/// (0,0) corresponds to the top-left corner
#[derive(Reflect, Debug, Clone, Serialize, Deserialize)]
pub enum ViewportPosition {
Absolute(UVec2),
Percentage(Vec2),
}

impl Default for ViewportPosition {
fn default() -> Self {
Self::Absolute(Default::default())
}
}

impl ViewportPosition {
/// Converts this size into a absolute size in pixels.
#[inline]
pub fn as_absolute(&self, of: UVec2) -> UVec2 {
match self {
ViewportPosition::Absolute(v) => *v,
ViewportPosition::Percentage(v) => (of.as_vec2() * *v).as_uvec2(),
}
}

/// Converts this size into a absolute size in pixels if possible.
///
/// Returns `None` if self is `Percentage` and argument is `None`
#[inline]
pub fn try_as_absolute(&self, of_opt: Option<UVec2>) -> Option<UVec2> {
match self {
ViewportPosition::Absolute(v) => Some(*v),
ViewportPosition::Percentage(_) => of_opt.map(|of| self.as_absolute(of)),
}
}
}
/// An enum that represents viewport's size either as absolute value (in pixels) or as a percentage of camera's render target's size.
#[derive(Reflect, Debug, Clone, Serialize, Deserialize)]
pub enum ViewportSize {
Absolute(UVec2),
Percentage(Vec2),
}

impl Default for ViewportSize {
fn default() -> Self {
Self::Absolute(Default::default())
}
}

impl ViewportSize {
/// Converts this size into a absolute size in pixels.
#[inline]
pub fn as_absolute(&self, of: UVec2) -> UVec2 {
match self {
ViewportSize::Absolute(v) => *v,
ViewportSize::Percentage(v) => (of.as_vec2() * *v).as_uvec2(),
}
}

/// Converts this size into a absolute size in pixels if possible.
///
/// Returns `None` if self is `Percentage` and argument is `None`
#[inline]
pub fn try_as_absolute(&self, of_opt: Option<UVec2>) -> Option<UVec2> {
match self {
ViewportSize::Absolute(v) => Some(*v),
ViewportSize::Percentage(_) => of_opt.map(|of| self.as_absolute(of)),
}
}
}

impl Default for Viewport {
fn default() -> Self {
Self {
Expand Down Expand Up @@ -111,16 +182,26 @@ impl Camera {
Some((physical_size.as_dvec2() / scale).as_vec2())
}

/// Returns absolute size of this `Camera`'s viewport.
///
/// I.e. multiplies the percentage size of viewport with `physical_target_size` or uses
/// viewport's absolute size
///
/// Returns `None` if viewport size is a `Percentage` and [`Camera::physical_target_size`] returns
/// `None`
#[inline]
pub fn viewport_absolute_size(&self) -> Option<UVec2> {
self.viewport
.as_ref()
.and_then(|v| v.physical_size.try_as_absolute(self.physical_target_size()))
}

/// The rendered physical bounds (minimum, maximum) of the camera. If the `viewport` field is
/// set to [`Some`], this will be the rect of that custom viewport. Otherwise it will default to
/// the full physical rect of the current [`RenderTarget`].
#[inline]
pub fn physical_viewport_rect(&self) -> Option<(UVec2, UVec2)> {
let min = self
.viewport
.as_ref()
.map(|v| v.physical_position)
.unwrap_or(UVec2::ZERO);
let min = self.viewport_absolute_size().unwrap_or(UVec2::ZERO);
let max = min + self.physical_viewport_size()?;
Some((min, max))
}
Expand All @@ -141,9 +222,8 @@ impl Camera {
/// [`RenderTarget`], prefer [`Camera::logical_target_size`].
#[inline]
pub fn logical_viewport_size(&self) -> Option<Vec2> {
self.viewport
.as_ref()
.and_then(|v| self.to_logical(v.physical_size))
self.viewport_absolute_size()
.and_then(|s| self.to_logical(s))
.or_else(|| self.logical_target_size())
}

Expand All @@ -153,9 +233,7 @@ impl Camera {
/// For logic that requires the full physical size of the [`RenderTarget`], prefer [`Camera::physical_target_size`].
#[inline]
pub fn physical_viewport_size(&self) -> Option<UVec2> {
self.viewport
.as_ref()
.map(|v| v.physical_size)
self.viewport_absolute_size()
.or_else(|| self.physical_target_size())
}

Expand Down
24 changes: 18 additions & 6 deletions crates/bevy_render/src/render_phase/draw_state.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
camera::Viewport,
camera::{ExtractedCamera, Viewport},
prelude::Color,
render_resource::{
BindGroup, BindGroupId, Buffer, BufferId, BufferSlice, RenderPipeline, RenderPipelineId,
Expand Down Expand Up @@ -340,12 +340,24 @@ impl<'a> TrackedRenderPass<'a> {
/// Set the rendering viewport to the given [`Camera`](crate::camera::Viewport) [`Viewport`].
///
/// Subsequent draw calls will be projected into that viewport.
pub fn set_camera_viewport(&mut self, viewport: &Viewport) {
pub fn set_camera_viewport(&mut self, viewport: &Viewport, camera: &ExtractedCamera) {
Copy link
Contributor Author

@LoipesMas LoipesMas Jul 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that works too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

let physical_size = viewport
.physical_size
.try_as_absolute(camera.physical_target_size)
.expect("Couldn't get camera.physical_target_size (needed for relative viewport size)");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


let physical_position = viewport
.physical_position
.try_as_absolute(camera.physical_target_size)
.expect(
"Couldn't get camera.physical_target_size (needed for relative viewport position)",
);

self.set_viewport(
viewport.physical_position.x as f32,
viewport.physical_position.y as f32,
viewport.physical_size.x as f32,
viewport.physical_size.y as f32,
physical_position.x as f32,
physical_position.y as f32,
physical_size.x as f32,
physical_size.y as f32,
viewport.depth.start,
viewport.depth.end,
);
Expand Down
15 changes: 10 additions & 5 deletions examples/3d/split_screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use bevy::{
core_pipeline::clear_color::ClearColorConfig,
prelude::*,
render::camera::Viewport,
render::camera::{Viewport, ViewportPosition, ViewportSize},
window::{WindowId, WindowResized},
};

Expand Down Expand Up @@ -96,15 +96,20 @@ fn set_camera_viewports(
let window = windows.primary();
let mut left_camera = left_camera.single_mut();
left_camera.viewport = Some(Viewport {
physical_position: UVec2::new(0, 0),
physical_size: UVec2::new(window.physical_width() / 2, window.physical_height()),
// You can use absolute pixel values...
physical_position: ViewportPosition::Absolute(UVec2::ZERO),
physical_size: ViewportSize::Absolute(UVec2::new(
window.physical_width() / 2,
window.physical_height(),
)),
..default()
});

let mut right_camera = right_camera.single_mut();
right_camera.viewport = Some(Viewport {
physical_position: UVec2::new(window.physical_width() / 2, 0),
physical_size: UVec2::new(window.physical_width() / 2, window.physical_height()),
// ... or specify an adaptive percentage of the render target.
physical_position: ViewportPosition::Percentage(Vec2::new(0.5, 0.0)),
physical_size: ViewportSize::Percentage(Vec2::new(0.5, 1.0)),
..default()
});
}
Expand Down