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

Sprites not being rendered with translation.z < 0.0 #9138

Closed
juliohq opened this issue Jul 12, 2023 · 10 comments · Fixed by #9310
Closed

Sprites not being rendered with translation.z < 0.0 #9138

juliohq opened this issue Jul 12, 2023 · 10 comments · Fixed by #9310
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy P-Regression Functionality that used to work but no longer does. Add a test for this!
Milestone

Comments

@juliohq
Copy link
Contributor

juliohq commented Jul 12, 2023

Bevy version

0.11

[Optional] Relevant system information

`{ name: "AMD Radeon RX 560 Series", vendor: 4098, device: 26607, device_type: DiscreteGpu, driver: "AMD open-source driver", driver_info: "2023.Q2.3 (LLPC)", backend: Vulkan }`

What you did

Just changed the Z axis of a Transform translation to something < 0.0.

What went wrong

It doesn't render the sprite.

Additional information

Some examples with a tilemap (but also doesn't work for regular sprites):
Z = 0.0:
image

Z < 0.0:
image

@juliohq juliohq added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jul 12, 2023
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen P-Regression Functionality that used to work but no longer does. Add a test for this! and removed S-Needs-Triage This issue needs to be labelled labels Jul 12, 2023
@alice-i-cecile alice-i-cecile added this to the 0.11.1 milestone Jul 12, 2023
@alice-i-cecile
Copy link
Member

Note that this previously worked in Bevy 0.10.

@Selene-Amanita
Copy link
Member

Selene-Amanita commented Jul 12, 2023

This is intended behavior:

  • the default for Camera2dBundle is to have a z-coordinate of 999.9 and a far (in projection) of 1000., so all sprites with a z-coordinate < 0.1 won't be rendered
  • same for the default of OrthogonalProjection, far has value 1000.
  • this was "working" in 0.10 because frustum culling was ignored for sprites, it was introduced with Add Aabb calculation for Sprite, TextureAtlasSprite and Mesh2d #7885

It is a problem though because many people expect negative z-values to work out of the box.

Proposed changes:

  • Make the default far value equal to 2000., so z-values between -1000.1 and 999.9 work?
  • Maybe change the default z-value from 999.9 to 1000 too. My guess is that it was set to this value so stuff at a z value close to 0., but slightly negative because of rounding errors, would still show up. This "failsafe" wouldn't be necessary anymore.

Also, #7885 is a breaking change and should be mentioned in the migration guide! (I just added the Breaking Change label to it)

@Selene-Amanita Selene-Amanita added the D-Trivial Nice and easy! A great choice to get started with Bevy label Jul 12, 2023
@opstic
Copy link
Contributor

opstic commented Jul 13, 2023

Sorry about that, this was mentioned in #3944 (comment) before already, so I tested with the default Camera2dBundle before the AABB pull request, but it worked the same way (due to the default far value of 1000.. and default z of 999.9 as mentioned in @Selene-Amanita's comment), thus I didn't take notice.

@alice-i-cecile
What can I do to add an entry in the migration guide? (Since 0.11 has already been released)
Or are there other things I can do to fix this issue?

@Selene-Amanita
Copy link
Member

Selene-Amanita commented Jul 13, 2023

@opstic I think the behavior changes only for sprites (/2d meshes/texture atlases) with a z<0.1 in 0.11.0 compared to 0.10.1, for the default camera transform, and it also changes for people who don't use the default camera transform.

The migration guide can be edited on the bevy-website repo, see a PR I made to add missing information: bevyengine/bevy-website#700

@cart
Copy link
Member

cart commented Jul 13, 2023

@Selene-Amanita

Proposed changes: [ .. ]

I'm on board for both of these. People do like negative z values. And the "near zero failsafe" stuff seems like it wouldn't be necessary anymore. If we're going to support a -1000 to 1000 range, we just need to make sure it doesn't clip on the exact 1000 and -1000 values (both in shaders from the view projection and from frustum culling on the cpu) .

Migration Guide

Given that we're considering this a "bug", we probably shouldn't update the migration guide.

@Selene-Amanita
Copy link
Member

Given that we're considering this a "bug", we probably shouldn't update the migration guide.

The fact that negative values don't show up is confusing and that's what this issue is about, but from my perspective, by introducing frustum culling for 2d objects, #7885 is a breaking change. Code that "worked" before, even if it technically should never have, doesn't anymore in 0.11, so that should be mentioned in the migration guide.

@cart
Copy link
Member

cart commented Jul 13, 2023

Maybe worth calling out in the migration guide to make people aware. But idk if encouraging people to migrate their "valid but regressed" logic to something that we plan on "silently" fixing (as in, it will get picked up in a patch release) is the right call.

I guess as long as we call out our intent to "fix" in the migration guide, thats fine.

@opstic
Copy link
Contributor

opstic commented Jul 13, 2023

Made a pull request to add it in the migration guide.
@Selene-Amanita Thanks!

@Selene-Amanita
Copy link
Member

Selene-Amanita commented Jul 20, 2023

@cart @alice-i-cecile Another pitfall with 2D frustum culling that I saw several people encounter (for example here: https://old.reddit.com/r/bevy/comments/14x37ov/after_updating_to_011_from_0101_i_cannot_see/ and here: https://discord.com/channels/691052431525675048/1131259863775850557/1131259863775850557 but I encountered at least one other case, maybe two) is that they set the transform of the 2D camera to z=0 and it doesn't display anything (even stuff at z=0 because the bisecting plane is out of the half-space in Frustum's methods).

Other proposed change: make the default z-coordinate of the camera 0., far = 1000. and near = -1000..

I tested it and it works, it's a bit unconventional to have a negative near, and could be a bit confusing, but most people don't think much about z-coordinates of the camera in 2d and people who do would be able to understand I think. It would let us keep the default transform of Vec3::ZERO and have the expected behavior for people who just change the transform and setting the z-coordinate to 0. without thinking about what it means for a 2d camera to have a z-coordinate.

@cart
Copy link
Member

cart commented Jul 25, 2023

Yeah that seems reasonable to me! Having a non-zero position always felt a bit weird anyway.

github-merge-queue bot pushed a commit that referenced this issue Jul 31, 2023
# Objective

- Fixes #9138

## Solution

- Calling `Camera2dBundle::default()` will now result in a
`Camera2dBundle` with `Vec3::ZERO` transform, `far` value of `1000.` and
`near` value of `-1000.`.
- This will enable the rendering of 2d entities in negative z space by
default.
- I did not modify `new_with_far` as moving the camera to `Vec3::ZERO`
in that function will cause entities in the positive z space to become
hidden without further changes. And the further changes cannot be
applied without it being a breaking change.
cart pushed a commit that referenced this issue Aug 10, 2023
# Objective

- Fixes #9138

## Solution

- Calling `Camera2dBundle::default()` will now result in a
`Camera2dBundle` with `Vec3::ZERO` transform, `far` value of `1000.` and
`near` value of `-1000.`.
- This will enable the rendering of 2d entities in negative z space by
default.
- I did not modify `new_with_far` as moving the camera to `Vec3::ZERO`
in that function will cause entities in the positive z space to become
hidden without further changes. And the further changes cannot be
applied without it being a breaking change.
lukas-kirschner pushed a commit to lukas-kirschner/Exodus that referenced this issue Aug 19, 2023
PraxTube added a commit to PraxTube/magus-parvus that referenced this issue Dec 1, 2023
Note that the z range is limited from `-1000` to `1000`, see
bevyengine/bevy#9138
github-merge-queue bot pushed a commit that referenced this issue Sep 9, 2024
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 <dmlary@gmail.com>
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy P-Regression Functionality that used to work but no longer does. Add a test for this!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants