Skip to content

Conversation

IceSentry
Copy link
Contributor

@IceSentry IceSentry commented Mar 27, 2023

Objective

  • We support enabling a normal prepass, but the main pass never actually uses it and recomputes the normals in the main pass. This isn't ideal since it's doing redundant work.

Solution

  • Use the normal texture from the prepass in the main pass

Notes

I used NORMAL_PREPASS_ENABLED as a shader_def because NORMAL_PREPASS is currently used to signify that it is running in the prepass while this shader_def need to indicate the prepass is done and the normal prepass was ran before. I'm not sure if there's a better way to name this.

@IceSentry IceSentry added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible labels Mar 27, 2023
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.

I suppose maybe LOAD_PREPASS_NORMALS could be clearer?

@IceSentry
Copy link
Contributor Author

Oh yeah, good call, I'll rename it to that

Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

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

Yay GBuffer :)

@james7132 james7132 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 Mar 28, 2023
@james7132 james7132 added this pull request to the merge queue Mar 29, 2023
Merged via the queue into bevyengine:main with commit 0859f67 Mar 29, 2023
@IceSentry IceSentry deleted the prepass-normal-main-pass branch March 31, 2023 17:25
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 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.

4 participants