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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Fix post_processing and shader_prepass examples #7419

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Fix post_processing and shader_prepass examples #7419

wants to merge 2 commits into from

Conversation

superdump
Copy link
Contributor

@superdump superdump commented Jan 30, 2023 •
edited
Loading

Objective

Solution

  • The prepass was broken due to a missing MAX_CASCADES_PER_LIGHT shader def. Add it.
  • The shader used in the post_processing example is applied to a 2D mesh, so use the correct mesh2d_view_bindings shader import.

This was referenced Jan 30, 2023
Closed
Closed
@superdump superdump added this to the 0.10 milestone Jan 30, 2023
@superdump superdump added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples labels Jan 30, 2023
@superdump superdump requested a review from robtfm January 30, 2023 04:46
@superdump
Copy link
Contributor Author

I feel we should make a separate view uniform shader import and use it everywhere as they're all constructed in the same way from the same struct. I also feel like I made a PR doing this a long time ago... maybe.

@IceSentry
Copy link
Contributor

I feel we should make a separate view uniform shader import

Yeah, that does seem like a better fix. I originally tried reusing the MeshPipeline::specialize for the prepass, but it didn't work. I don't remember exactly why. It does seem possible to extract the common part to a separate function that could be used in both places.

robtfm
robtfm approved these changes Jan 30, 2023
View reviewed changes
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.

I feel we should make a separate view uniform shader import and use it everywhere as they're all constructed in the same way from the same struct. I also feel like I made a PR doing this a long time ago... maybe.

i've done that at least once too, as part of #6738. also noting this fix would be covered by #5703

@rparrett
Copy link
Contributor

rparrett commented Jan 30, 2023 •
edited
Loading

This also closes #7375

@superdump
Copy link
Contributor Author

bors r+

bors bot pushed a commit that referenced this pull request Jan 30, 2023
@superdump
Fix post_processing and shader_prepass examples (#7419)
9989d79 
# Objective

- Fix `post_processing` and `shader_prepass` examples as they fail when compiling shaders due to missing shader defs
- Fixes #6799
- Fixes #6996
- Fixes #7375 
- Supercedes #6997
- Supercedes #7380 

## Solution

- The prepass was broken due to a missing `MAX_CASCADES_PER_LIGHT` shader def. Add it.
- The shader used in the `post_processing` example is applied to a 2D mesh, so use the correct mesh2d_view_bindings shader import.
@bors
Copy link
Contributor

bors bot commented Jan 30, 2023

Build failed:

@superdump
Copy link
Contributor Author

bors r+

bors bot pushed a commit that referenced this pull request Jan 30, 2023
@superdump
Fix post_processing and shader_prepass examples (#7419)
72be3af 
# Objective

- Fix `post_processing` and `shader_prepass` examples as they fail when compiling shaders due to missing shader defs
- Fixes #6799
- Fixes #6996
- Fixes #7375 
- Supercedes #6997
- Supercedes #7380 

## Solution

- The prepass was broken due to a missing `MAX_CASCADES_PER_LIGHT` shader def. Add it.
- The shader used in the `post_processing` example is applied to a 2D mesh, so use the correct mesh2d_view_bindings shader import.
@bors
Copy link
Contributor

bors bot commented Jan 30, 2023

Build failed:

@superdump
Copy link
Contributor Author

bors r+

bors bot pushed a commit that referenced this pull request Jan 30, 2023
@superdump
Fix post_processing and shader_prepass examples (#7419)
cdc780c 
# Objective

- Fix `post_processing` and `shader_prepass` examples as they fail when compiling shaders due to missing shader defs
- Fixes #6799
- Fixes #6996
- Fixes #7375 
- Supercedes #6997
- Supercedes #7380 

## Solution

- The prepass was broken due to a missing `MAX_CASCADES_PER_LIGHT` shader def. Add it.
- The shader used in the `post_processing` example is applied to a 2D mesh, so use the correct mesh2d_view_bindings shader import.
@bors
Copy link
Contributor

bors bot commented Jan 30, 2023

Build failed:

@superdump
Copy link
Contributor Author

bors r+

bors bot pushed a commit that referenced this pull request Jan 30, 2023
@superdump
Fix post_processing and shader_prepass examples (#7419)
9e939db 
# Objective

- Fix `post_processing` and `shader_prepass` examples as they fail when compiling shaders due to missing shader defs
- Fixes #6799
- Fixes #6996
- Fixes #7375 
- Supercedes #6997
- Supercedes #7380 

## Solution

- The prepass was broken due to a missing `MAX_CASCADES_PER_LIGHT` shader def. Add it.
- The shader used in the `post_processing` example is applied to a 2D mesh, so use the correct mesh2d_view_bindings shader import.
@rparrett
Copy link
Contributor

rparrett commented Jan 30, 2023 •
edited
Loading

The failure is real

edit: weird, I was sure that I saw a real error in github action after the first try earlier today.

But anyhow, I'm getting this locally (macos and wasm) in post_processing

ERROR bevy_render::render_resource::pipeline_cache: failed to process shader: Unresolved import: Custom("bevy_pbr::mesh2d_view_bindings").

shader_prepass seems fine

@bors
Copy link
Contributor

bors bot commented Jan 30, 2023

Build failed:

@superdump
Copy link
Contributor Author

Oh, I saw the markdown lint failure that seemed to just be an intermittent docker issue. I'll fix that.

superdump
superdump commented Jan 30, 2023
View reviewed changes
Copy link
Contributor Author

@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.

bors r+

bors bot pushed a commit that referenced this pull request Jan 30, 2023
@superdump
Fix post_processing and shader_prepass examples (#7419)
08eaa7a 
# Objective

- Fix `post_processing` and `shader_prepass` examples as they fail when compiling shaders due to missing shader defs
- Fixes #6799
- Fixes #6996
- Fixes #7375 
- Supercedes #6997
- Supercedes #7380 

## Solution

- The prepass was broken due to a missing `MAX_CASCADES_PER_LIGHT` shader def. Add it.
- The shader used in the `post_processing` example is applied to a 2D mesh, so use the correct mesh2d_view_bindings shader import.
@bors
Copy link
Contributor

bors bot commented Jan 30, 2023

Build failed:

@superdump
Copy link
Contributor Author

bors r+

bors bot pushed a commit that referenced this pull request Jan 30, 2023
@superdump
Fix post_processing and shader_prepass examples (#7419)
8b7ebe1 
# Objective

- Fix `post_processing` and `shader_prepass` examples as they fail when compiling shaders due to missing shader defs
- Fixes #6799
- Fixes #6996
- Fixes #7375 
- Supercedes #6997
- Supercedes #7380 

## Solution

- The prepass was broken due to a missing `MAX_CASCADES_PER_LIGHT` shader def. Add it.
- The shader used in the `post_processing` example is applied to a 2D mesh, so use the correct mesh2d_view_bindings shader import.
@bors
Copy link
Contributor

bors bot commented Jan 30, 2023

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Fix post_processing and shader_prepass examples [Merged by Bors] - Fix post_processing and shader_prepass examples Jan 30, 2023
@bors bors bot closed this Jan 30, 2023
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@superdump @ItsDoot
Fix post_processing and shader_prepass examples (bevyengine#7419)
ebc0312 
# Objective

- Fix `post_processing` and `shader_prepass` examples as they fail when compiling shaders due to missing shader defs
- Fixes bevyengine#6799
- Fixes bevyengine#6996
- Fixes bevyengine#7375 
- Supercedes bevyengine#6997
- Supercedes bevyengine#7380 

## Solution

- The prepass was broken due to a missing `MAX_CASCADES_PER_LIGHT` shader def. Add it.
- The shader used in the `post_processing` example is applied to a 2D mesh, so use the correct mesh2d_view_bindings shader import.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IceSentry IceSentry IceSentry approved these changes

@robtfm robtfm robtfm approved these changes

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Examples An addition or correction to our examples
Projects
None yet
Milestone
0.10
4 participants
@superdump @IceSentry @rparrett @robtfm