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

fix prepass normal_mapping #8978

Merged
merged 1 commit into from Jun 29, 2023
Merged

fix prepass normal_mapping #8978

merged 1 commit into from Jun 29, 2023

Conversation

robtfm
Copy link
Contributor

@robtfm robtfm commented Jun 27, 2023

Objective

#5703 caused the normal prepass to fail as the prepass uses pbr_functions::apply_normal_mapping, which uses mesh_view_bindings::view to determine mip bias, which conflicts with prepass_bindings::view.

Solution

pass the mip bias to the apply_normal_mapping function explicitly.

@robtfm robtfm added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Jun 27, 2023
@robtfm robtfm added this to the 0.11 milestone Jun 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.

Looks straight forward enough to me.

@superdump superdump requested a review from JMS55 June 28, 2023 21:36
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

A reasonable fix for now. Ideally we find a way to prevent these clashes across files. We (and our users) will likely continue to hit this.

@cart cart added this pull request to the merge queue Jun 29, 2023
Merged via the queue into bevyengine:main with commit 15445c9 Jun 29, 2023
23 checks passed
@robtfm
Copy link
Contributor Author

robtfm commented Jun 29, 2023

Ideally we find a way to prevent these clashes across files

I think the best fix for this is to move the binding into the bevy_render::view file alongside the struct. then you can't import the view struct/variable without also importing the binding, and all references to the binding will be the same. prepass would import it, main pass would import it as well, both could then refer to it freely. generally any functions that need the view could import it freely without clashes.

it would mean that the view struct absolutely cannot be used in a slot other than slot 0, without explicitly copy/pasting the struct definition. i think that's fine (and with automatic group 0 it would be irrelevant anyway).

it is quite invasive though so i will leave it till after the release. it should also be done for globals, mesh bindings, etc.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants