Skip to content

Fix CameraProjection panic and improve CameraProjectionPlugin - #11808

Merged
mockersf merged 4 commits into
bevyengine:mainfrom
doonv:fix-cameraprojection-crash
Apr 27, 2024
Merged

Fix CameraProjection panic and improve CameraProjectionPlugin#11808
mockersf merged 4 commits into
bevyengine:mainfrom
doonv:fix-cameraprojection-crash

Conversation

@doonv

@doonv doonv commented Feb 10, 2024

Copy link
Copy Markdown
Contributor

Objective

Fix #11799 and improve CameraProjectionPlugin

Solution

CameraProjectionPlugin is now an all-in-one plugin for adding a custom CameraProjection. I also added PbrProjectionPlugin which is like CameraProjectionPlugin but for PBR.

P.S. I'd like to get this merged after #11766.


Changelog

  • Changed CameraProjectionPlugin to be an all-in-one plugin for adding a CameraProjection
  • Removed VisibilitySystems::{UpdateOrthographicFrusta, UpdatePerspectiveFrusta, UpdateProjectionFrusta}, now replaced with VisibilitySystems::UpdateFrusta
  • Added PbrProjectionPlugin for projection-specific PBR functionality.

Migration Guide

VisibilitySystems's UpdateOrthographicFrusta, UpdatePerspectiveFrusta, and UpdateProjectionFrusta variants were removed, they were replaced with VisibilitySystems::UpdateFrusta

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen S-Blocked This cannot move forward until something else changes labels Feb 10, 2024
@doonv

doonv commented Feb 26, 2024

Copy link
Copy Markdown
Contributor Author

This is now unblocked since #11766 has been merged.

@NthTensor NthTensor removed the S-Blocked This cannot move forward until something else changes label Apr 23, 2024

@NthTensor NthTensor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code looks like a big improvement. Will approve later after I get the chance to evaluate this on examples.

@Zoomulator

Zoomulator commented Apr 23, 2024

Copy link
Copy Markdown
Contributor

@doonv I did a rebase and fixed conflicts: doonv#3

Notable conflicts:

  • CameraProjectionPlugin<T> got new trait bounds T: CameraProjection + Component + GetTypeRegistration, which conflicted with a doc comment above it.
  • CameraUpdateSystem struct definition was moved to the line below CameraProjectionPlugin
  • check_visibility is now generic check_visibility::<WithMesh> (view/visibility/mod.rs)

@Zoomulator Zoomulator left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Been running this in my own bevy fork for my game and it's been working well with my custom projection + pbr shadows. No complaints :)

@BD103 BD103 added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Apr 23, 2024
@doonv
doonv requested a review from Zoomulator April 25, 2024 21:25

@Zoomulator Zoomulator left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Well spotted to not need the check_visibility.after given the recently added .before(CheckVisibility) above.

@NthTensor NthTensor added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Apr 26, 2024
@mockersf
mockersf added this pull request to the merge queue Apr 26, 2024
Merged via the queue into bevyengine:main with commit de9dc9c Apr 27, 2024
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 C-Feature A new feature, making something new possible M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Panic upon using a CameraProjection with DirectionalLight shadows

6 participants