Update defaults for OrthographicProjection #9537
Conversation
There was a problem hiding this comment.
Maybe the methods of Camera2dBundle could be cleaned up in this PR, like suggested in the second bullet point here: #9214 (comment) ? But I'm good with it as it is.
I think the migration guide could be a bit more explicit, especially for people who use OrthographicProjection in a 3D context, just something like "default value for OrthographicProjection.near changed from 0. to -1000.".
Selene-Amanita
added
A-Rendering
|
Updated the migration guide. I could probably do a code cleanup PR separately. |
There was a problem hiding this comment.
This came from a discussion in the discord server: https://discord.com/channels/691052431525675048/692572690833473578/1143435136738803782
Note: Is it possible to update the impl Default for Camera2dBundle in crates/bevy_core_pipeline/src/core_2d/camera_2d.rs to use OrthographicProjection::default()? So that if there is any deviation in the future, it is always explicit.
| @@ -197,14 +197,16 @@ pub enum ScalingMode { | |||
| /// | |||
| /// Note that the scale of the projection and the apparent size of objects are inversely proportional. | |||
| /// As the size of the projection increases, the size of objects decreases. | |||
| /// | |||
| /// Note also that the view frustum is centered at the origin. | |||
There was a problem hiding this comment.
However a justification here would be useful, like "This helps in 2D when both the camera and sprites are likely to be placed at z=0.0"
|
Provided the discord discussion, I don't think it's controversial. |
|
(re. |
nicopap
added
the
S-Ready-For-Final-Review
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
These new defaults match what is used by `Camera2dBundle::default()`, removing a potential footgun from overriding a field in the projection component of the bundle.
|
As mentioned in the discord, this is gonna cause problems for 3d. With this objects behind the camera are going to be rendered when they shouldn’t be visible. Example: two xz planes, you put the camera between them pointed at one, and only the other will be visible. I don’t know what the proper solution is, but devil_ira pointed out this is coming in 0.12 |
|
@dmlary I brought that up in the discord yeah, but still approved this change, my reasoning/assumption is that a lot of beginners with Bevy would either use 2D, or 3D with perspective projection, and those default values are convenient to avoid footguns for them. People using orthographic projection in 3D would probably know better how projections work and be able to troubleshoot an eventual problem more easily. Maybe there should be other "static methods"/associated functions/constructors on |
This reverts commit 5012a0f.
…bevyengine#9878) # Objective - Fixes bevyengine#9876 ## Solution - Reverted commit `5012a0fd57748ab6f146776368b4cf988bba1eaa` to restore the previous default values for `OrthographicProjection`. --- ## Migration Guide - Migration guide steps from bevyengine#9537 should be removed for next release.
Adopted PR from dmlary, all credit to them! #9915 Original description: # Objective The default value for `near` in `OrthographicProjection` should be different for 2d & 3d. For 2d using `near = -1000` allows bevy users to build up scenes using background `z = 0`, and foreground elements `z > 0` similar to css. However in 3d `near = -1000` results in objects behind the camera being rendered. Using `near = 0` works for 3d, but forces 2d users to assign `z <= 0` for rendered elements, putting the background at some arbitrary negative value. There is no common value for `near` that doesn't result in a footgun or usability issue for either 2d or 3d, so they should have separate values. There was discussion about other options in the discord [0](https://discord.com/channels/691052431525675048/1154114310042292325), but splitting `default()` into `default_2d()` and `default_3d()` seemed like the lowest cost approach. Related/past work #9138, #9214, #9310, #9537 (thanks to @Selene-Amanita for the list) ## Solution This commit splits `OrthographicProjection::default` into `default_2d` and `default_3d`. ## Migration Guide - In initialization of `OrthographicProjection`, change `..default()` to `..OrthographicProjection::default_2d()` or `..OrthographicProjection::default_3d()` Example: ```diff --- a/examples/3d/orthographic.rs +++ b/examples/3d/orthographic.rs @@ -20,7 +20,7 @@ fn setup( projection: OrthographicProjection { scale: 3.0, scaling_mode: ScalingMode::FixedVertical(2.0), - ..default() + ..OrthographicProjection::default_3d() } .into(), transform: Transform::from_xyz(5.0, 5.0, 5.0).looking_at(Vec3::ZERO, Vec3::Y), ``` --------- Co-authored-by: David M. Lary <[email protected]> Co-authored-by: Jan Hohenheim <[email protected]>
Objective
These new defaults match what is used by
Camera2dBundle::default(), removing a potential footgun from overriding a field in the projection component of the bundle.Solution
Adjusted the near clipping plane of
OrthographicProjection::default()to-1000..Changelog
Changed:
OrthographicProjection::default()now matches the value used inCamera2dBundle::default()Migration Guide
Workarounds used to keep the projection consistent with the bundle defaults are no longer required. Meanwhile, uses of
OrthographicProjectionin 2D scenes may need to be adjusted; thenearclipping plane default was changed from0.0to-1000.0.