-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
OrthographicProjection scaling mode + camera bundle refactoring #400
Changes from all commits
5755737
2558b35
9af00aa
857b61f
b0ea4fa
8603eb8
88805e0
373392e
8ec9d89
3f2e859
b24bb8d
6b6513d
524e092
8ec5b75
5912c4b
a20734d
88dc59f
da2b1ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,20 @@ pub enum WindowOrigin { | |
BottomLeft, | ||
} | ||
|
||
#[derive(Debug, Clone, Reflect, Serialize, Deserialize)] | ||
#[reflect_value(Serialize, Deserialize)] | ||
pub enum ScalingMode { | ||
/// Manually specify left/right/top/bottom values. | ||
/// Ignore window resizing; the image will stretch. | ||
None, | ||
/// Match the window size. 1 world unit = 1 pixel. | ||
WindowSize, | ||
/// Keep vertical axis constant; resize horizontal with aspect ratio. | ||
FixedVertical, | ||
/// Keep horizontal axis constant; resize vertical with aspect ratio. | ||
FixedHorizontal, | ||
} | ||
|
||
#[derive(Debug, Clone, Reflect)] | ||
#[reflect(Component)] | ||
pub struct OrthographicProjection { | ||
|
@@ -61,36 +75,67 @@ pub struct OrthographicProjection { | |
pub near: f32, | ||
pub far: f32, | ||
pub window_origin: WindowOrigin, | ||
pub scaling_mode: ScalingMode, | ||
pub scale: f32, | ||
} | ||
|
||
impl CameraProjection for OrthographicProjection { | ||
fn get_projection_matrix(&self) -> Mat4 { | ||
Mat4::orthographic_rh( | ||
self.left, | ||
self.right, | ||
self.bottom, | ||
self.top, | ||
self.left * self.scale, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 I really like this simplification |
||
self.right * self.scale, | ||
self.bottom * self.scale, | ||
self.top * self.scale, | ||
self.near, | ||
self.far, | ||
) | ||
} | ||
|
||
fn update(&mut self, width: f32, height: f32) { | ||
match self.window_origin { | ||
WindowOrigin::Center => { | ||
match (&self.scaling_mode, &self.window_origin) { | ||
(ScalingMode::WindowSize, WindowOrigin::Center) => { | ||
let half_width = width / 2.0; | ||
let half_height = height / 2.0; | ||
self.left = -half_width; | ||
self.right = half_width; | ||
self.top = half_height; | ||
self.bottom = -half_height; | ||
} | ||
WindowOrigin::BottomLeft => { | ||
(ScalingMode::WindowSize, WindowOrigin::BottomLeft) => { | ||
self.left = 0.0; | ||
self.right = width; | ||
self.top = height; | ||
self.bottom = 0.0; | ||
} | ||
(ScalingMode::FixedVertical, WindowOrigin::Center) => { | ||
let aspect_ratio = width / height; | ||
self.left = -aspect_ratio; | ||
self.right = aspect_ratio; | ||
self.top = 1.0; | ||
self.bottom = -1.0; | ||
} | ||
(ScalingMode::FixedVertical, WindowOrigin::BottomLeft) => { | ||
let aspect_ratio = width / height; | ||
self.left = 0.0; | ||
self.right = aspect_ratio; | ||
self.top = 1.0; | ||
self.bottom = 0.0; | ||
} | ||
(ScalingMode::FixedHorizontal, WindowOrigin::Center) => { | ||
let aspect_ratio = height / width; | ||
self.left = -1.0; | ||
self.right = 1.0; | ||
self.top = aspect_ratio; | ||
self.bottom = -aspect_ratio; | ||
} | ||
(ScalingMode::FixedHorizontal, WindowOrigin::BottomLeft) => { | ||
let aspect_ratio = height / width; | ||
self.left = 0.0; | ||
self.right = 1.0; | ||
self.top = aspect_ratio; | ||
self.bottom = 0.0; | ||
} | ||
(ScalingMode::None, _) => {} | ||
} | ||
} | ||
|
||
|
@@ -102,13 +147,15 @@ impl CameraProjection for OrthographicProjection { | |
impl Default for OrthographicProjection { | ||
fn default() -> Self { | ||
OrthographicProjection { | ||
left: 0.0, | ||
right: 0.0, | ||
bottom: 0.0, | ||
top: 0.0, | ||
left: -1.0, | ||
right: 1.0, | ||
bottom: -1.0, | ||
top: 1.0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing the defaults here, to allow for this: let projection = ОrthographicProjection {
scaling_mode: ScalingMode::None,
..Default::default()
}; |
||
near: 0.0, | ||
far: 1000.0, | ||
window_origin: WindowOrigin::Center, | ||
scaling_mode: ScalingMode::WindowSize, | ||
scale: 1.0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this scale field shouldn't be here, and instead should be handled by the scale on the Obviously that would also scale There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I strongly disagree. They serve different purposes. I think it is important to have this field, to be able to set the "span" of the projection. The camera's transform is typically used to perform "zoom in/out" effects in the game. The "scale" of the projection is if you need to normalize the sizes of things (say, depending on what units you work in, particularly for a 2D game). Say, for example, you are making a 2D game and you want it to scale based on screen resolution, to always fit the same content on screen. You might want the projection to go -600.0 to +600.0 (1200 total height), because you designed your assets for a nominal 1200-vertical-pixels screen size. You shouldn't have to complicate your camera math to account for that. The camera scale should stay at 1.0 unless you are doing some "zoom in/out" effects. This is the same argument I've been giving before w.r.t your suggestion of "just use the window-pixels 2d projection bevy already provides for 2D games and fiddle with the camera scale to make your content fit the screen" ... it's an ugly workaround and not ergonomic. It really doesn't cost anything to have this field, and I think it opens up some likely-useful extra flexibility for users. Why potentially cripple/limit the functionality, just to avoid that extra field? IMO it makes sense to have it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My point is that if we did have scale only as the field on transform, then that would be the only thing defining the size (of the fixed axis) of the in-world screen/size of the in-world viewing pane Whereas at the moment you have to multiply it by this scale field to know that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, the projection scale is at 1.0 by default, so what you said applies, unless the user explicitly goes to mess with the projection. I think it is valuable for the users to have the option of doing that. Which means we shouldn't remove the scale field. It's not going to surprise or confuse anyone; it's only relevant to those users who know they want to use it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that also makes sense. |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,19 +22,40 @@ pub struct MeshBundle { | |
pub global_transform: GlobalTransform, | ||
} | ||
|
||
/// A component bundle for "3d camera" entities | ||
/// Component bundle for camera entities with perspective projection | ||
/// | ||
/// Use this for 3D rendering. | ||
#[derive(Bundle)] | ||
pub struct Camera3dBundle { | ||
pub struct PerspectiveCameraBundle { | ||
pub camera: Camera, | ||
pub perspective_projection: PerspectiveProjection, | ||
pub visible_entities: VisibleEntities, | ||
pub transform: Transform, | ||
pub global_transform: GlobalTransform, | ||
} | ||
|
||
impl Default for Camera3dBundle { | ||
impl PerspectiveCameraBundle { | ||
pub fn new_3d() -> Self { | ||
Default::default() | ||
} | ||
|
||
pub fn with_name(name: &str) -> Self { | ||
PerspectiveCameraBundle { | ||
camera: Camera { | ||
name: Some(name.to_string()), | ||
..Default::default() | ||
}, | ||
perspective_projection: Default::default(), | ||
visible_entities: Default::default(), | ||
transform: Default::default(), | ||
global_transform: Default::default(), | ||
} | ||
} | ||
} | ||
|
||
impl Default for PerspectiveCameraBundle { | ||
fn default() -> Self { | ||
Camera3dBundle { | ||
PerspectiveCameraBundle { | ||
camera: Camera { | ||
name: Some(base::camera::CAMERA_3D.to_string()), | ||
..Default::default() | ||
|
@@ -47,22 +68,24 @@ impl Default for Camera3dBundle { | |
} | ||
} | ||
|
||
/// A component bundle for "2d camera" entities | ||
/// Component bundle for camera entities with orthographic projection | ||
/// | ||
/// Use this for 2D games, isometric games, CAD-like 3D views. | ||
#[derive(Bundle)] | ||
pub struct Camera2dBundle { | ||
pub struct OrthographicCameraBundle { | ||
pub camera: Camera, | ||
pub orthographic_projection: OrthographicProjection, | ||
pub visible_entities: VisibleEntities, | ||
pub transform: Transform, | ||
pub global_transform: GlobalTransform, | ||
} | ||
|
||
impl Default for Camera2dBundle { | ||
fn default() -> Self { | ||
impl OrthographicCameraBundle { | ||
pub fn new_2d() -> Self { | ||
// we want 0 to be "closest" and +far to be "farthest" in 2d, so we offset | ||
// the camera's translation by far and use a right handed coordinate system | ||
let far = 1000.0; | ||
Camera2dBundle { | ||
OrthographicCameraBundle { | ||
camera: Camera { | ||
name: Some(base::camera::CAMERA_2D.to_string()), | ||
..Default::default() | ||
|
@@ -76,4 +99,30 @@ impl Default for Camera2dBundle { | |
global_transform: Default::default(), | ||
} | ||
} | ||
|
||
pub fn new_3d() -> Self { | ||
OrthographicCameraBundle { | ||
camera: Camera { | ||
name: Some(base::camera::CAMERA_3D.to_string()), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the new camera use the |
||
..Default::default() | ||
}, | ||
orthographic_projection: Default::default(), | ||
visible_entities: Default::default(), | ||
transform: Default::default(), | ||
global_transform: Default::default(), | ||
} | ||
} | ||
|
||
pub fn with_name(name: &str) -> Self { | ||
OrthographicCameraBundle { | ||
camera: Camera { | ||
name: Some(name.to_string()), | ||
..Default::default() | ||
}, | ||
orthographic_projection: Default::default(), | ||
visible_entities: Default::default(), | ||
transform: Default::default(), | ||
global_transform: Default::default(), | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May as well have
ScalingMode::None
too, if we're here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you mean this: that nothing should happen on window resize (let the image stretch), just letting the user set the
left
/right
/top
/bottom
fields manually.OK, added this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, exactly that. No point limiting the functionality.