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

[Merged by Bors] - Fix PBR regression for unlit materials #2197

Closed
wants to merge 2 commits into from

Conversation

aevyrie
Copy link
Member

@aevyrie aevyrie commented May 17, 2021

Fixes the frag shader for unlit materials by correcting the scope of the #ifndef to include the light functions. Closes #2190, introduced in #2112.

Tested by changing materials in the the 3d_scene example to be unlit. Unsure how to prevent future regressions without creating a test case scene that will catch these runtime panics.

@alice-i-cecile alice-i-cecile added C-Regression Functionality that used to work but no longer does. Add a test for this! A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior labels May 17, 2021
@mockersf mockersf 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 May 17, 2021
@mtsr
Copy link
Contributor

mtsr commented May 17, 2021

Please add an unlit material to one of the examples, to help us catch this in the future.

@aevyrie
Copy link
Member Author

aevyrie commented May 17, 2021

Added a test to the pbr example, seeing as the pbr shader file held the regression. Bumped up the spheres a row, and improved the camera and lighting while I was at it, to remove projection distortion and varying light direction.

image

@cart
Copy link
Member

cart commented May 17, 2021

bors r+

bors bot pushed a commit that referenced this pull request May 17, 2021
Fixes the frag shader for unlit materials by correcting the scope of the `#ifndef` to include the light functions. Closes #2190, introduced in #2112.

Tested by changing materials in the the `3d_scene` example to be unlit. Unsure how to prevent future regressions without creating a test case scene that will catch these runtime panics.
@bors
Copy link
Contributor

bors bot commented May 17, 2021

@bors bors bot changed the title Fix PBR regression for unlit materials [Merged by Bors] - Fix PBR regression for unlit materials May 17, 2021
@bors bors bot closed this May 17, 2021
@aevyrie aevyrie deleted the unlit_fix branch May 17, 2021 23:08
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
Fixes the frag shader for unlit materials by correcting the scope of the `#ifndef` to include the light functions. Closes bevyengine#2190, introduced in bevyengine#2112.

Tested by changing materials in the the `3d_scene` example to be unlit. Unsure how to prevent future regressions without creating a test case scene that will catch these runtime panics.
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-Regression Functionality that used to work but no longer does. Add a test for this! 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.

Unlit materials cause shader panic
5 participants