Skip to content

Conversation

danchia
Copy link
Contributor

@danchia danchia commented Mar 18, 2023

Objective

  • Texture cubemaps, such as used by point light shadows, inherently use left-handed y-up coordinate axes. Prior to this PR, I understand that we attempted to keep cubemaps mostly right-handed. However, the solution has sharp edges, such as requiring the sampling vector to still be negated, as well as hard-to-figure reasons for why the cubemap faces use a specific target/up axis.
  • Solves the same problem as bevy_pbr: Use left-handed coordinates for point light shadow cubemaps #4242, but with a slightly different approach.

Solution

  • We embrace texture cubemaps being left-handed y-up:
    • When it comes time to sample them we flip the z coordinate to transform bevy's right-handed y-up coordinates to left-handed y-up coordinates.
    • When rendering the cubemap, carefully pick the right Bevy coordinate-space target/up axis to produce the correct cubemap faces accounting for the fact that the cubemap faces are specified in the left-hand coordinate space.
    • Note that the in prior PR bevy_pbr: Use left-handed coordinates for point light shadow cubemaps #4242, we converted the entire shadow rendering pipeline for the cubemap to left-handed coordinate space. I felt that carefully mapping the cubemap faces to the right axes led to a simpler solution overall, even if the cubemap face mapping had to be done carefully.

Changelog

  • Bevy point light shadow cubemaps now use a left-handed y-up coordinate space.

Migration Guide

  • When sampling from the point light shadow cubemap, use the (expected) light to fragment direction vector but negate the z coordinate. Previously, you would have used the fragment to light direction vector.

@danchia danchia requested a review from superdump March 18, 2023 07:14
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

Simple, clean, good stuff.

@superdump superdump requested a review from robtfm March 18, 2023 07:38
Co-authored-by: Robert Swain <robert.swain@gmail.com>
@james7132 james7132 added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Mar 18, 2023
@james7132 james7132 added this to the 0.11 milestone Mar 18, 2023
Copy link
Contributor

@robtfm robtfm left a comment

Choose a reason for hiding this comment

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

lgtm apart form 1 nit

Co-authored-by: robtfm <50659922+robtfm@users.noreply.github.com>
@danchia danchia requested a review from robtfm March 18, 2023 16:17
@danchia
Copy link
Contributor Author

danchia commented Mar 18, 2023

Thanks for the reviews!

@superdump superdump added this pull request to the merge queue Mar 18, 2023
@superdump superdump merged commit 2010164 into bevyengine:main Mar 18, 2023
ProfLander pushed a commit to shfty-rust/bevy that referenced this pull request Mar 19, 2023
Co-authored-by: Robert Swain <robert.swain@gmail.com>
Co-authored-by: robtfm <50659922+robtfm@users.noreply.github.com>
ProfLander pushed a commit to shfty-rust/bevy that referenced this pull request Mar 19, 2023
Co-authored-by: Robert Swain <robert.swain@gmail.com>
Co-authored-by: robtfm <50659922+robtfm@users.noreply.github.com>
@Selene-Amanita Selene-Amanita added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jul 10, 2023
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-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants