Skip to content

Shader: Fix the default behavior when mat uniforms are null #106592

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

beicause
Copy link
Contributor

Fixes #106561.

@beicause beicause changed the title ShaderMaterial: Fix default parameter behavior in editor ShaderMaterial: Fix mat default value behavior in editor after setting shader May 19, 2025
@beicause beicause force-pushed the shader-fix-editor-mat-default branch 2 times, most recently from d9c943b to 9aa0166 Compare May 19, 2025 13:23
@AThousandShips AThousandShips requested a review from a team May 19, 2025 16:21
@AThousandShips AThousandShips added this to the 4.5 milestone May 19, 2025
@AThousandShips AThousandShips added the cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release label May 19, 2025
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Editor code should not be in the scene/ folder. There are a few places where it is unavoidable, but I don't think this is one of those places since this only impacts mat types and not the other types. I think you should take a look at how other types are handled and then see if mat is just missed somewhere

@beicause
Copy link
Contributor Author

beicause commented May 21, 2025

I think the reason is that when the ShaderMaterial and Shader are newly created, the material parameters are not set, but get_property_list has already generated default values as properties. The default values of mat (and color alpha I guess, not tested yet) are not zero.

Perhaps this behavior should also be fixed for the game, not just for the editor

@beicause beicause force-pushed the shader-fix-editor-mat-default branch from 9aa0166 to 996680b Compare May 21, 2025 04:55
@beicause beicause changed the title ShaderMaterial: Fix mat default value behavior in editor after setting shader Shader: Fix the default behavior when mat uniforms are null May 21, 2025
@beicause
Copy link
Contributor Author

beicause commented May 21, 2025

Pushed another solution.

The alpha of color is already handled before _fill_std140_ubo_empty(). So it makes sense to deal with mat in the same way.

@beicause beicause force-pushed the shader-fix-editor-mat-default branch from 996680b to d97948d Compare May 21, 2025 05:04
@beicause beicause force-pushed the shader-fix-editor-mat-default branch from d97948d to 4621d3e Compare May 21, 2025 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release topic:rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The matrix uniform parameter of newly created ShaderMaterial is zero matrix instead of identity matrix
3 participants