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 shader doc comments to show up for instance uniforms #95652

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

Conversation

Chaosus
Copy link
Member

@Chaosus Chaosus commented Aug 16, 2024

@Chaosus Chaosus requested a review from a team as a code owner August 16, 2024 17:19
@Chaosus Chaosus added this to the 4.4 milestone Aug 16, 2024
@Chaosus Chaosus force-pushed the shader_fix_uniform_instance_doc branch 6 times, most recently from c5e927a to 8748023 Compare August 17, 2024 06:24
@Calinou Calinou added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Aug 17, 2024
Copy link
Contributor

@jsjtxietian jsjtxietian left a comment

Choose a reason for hiding this comment

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

Code looks good to me. But this only works when the shader is at material override, not when the shader is set to the mesh instance itself.

@jsjtxietian
Copy link
Contributor

Shader instances only show when shader set as material override.

Using the shader provided in the issue, set the shader to the mesh instance:

image

Set the shader to material override:

image

@Chaosus Chaosus force-pushed the shader_fix_uniform_instance_doc branch from 8748023 to 890b20c Compare August 21, 2024 05:24
@Chaosus
Copy link
Member Author

Chaosus commented Aug 21, 2024

Check again, I think I've fixed it.

@jsjtxietian
Copy link
Contributor

Check again, I think I've fixed it.

It did not work when the shader is draged to mehs instance's material field.

The surface_override_materials has one element but is null:

image

@Chaosus
Copy link
Member Author

Chaosus commented Aug 21, 2024

What do you mean by mesh instance's material field ? Surface material override property? If so my last commit fixed it, and it's working for myself, you need to reload a scene, maybe.

изображение

@jsjtxietian
Copy link
Contributor

I mean here, the surface material override property work fine :

image

@Chaosus Chaosus force-pushed the shader_fix_uniform_instance_doc branch 2 times, most recently from 449bc15 to cf897b2 Compare August 21, 2024 07:11
@Chaosus
Copy link
Member Author

Chaosus commented Aug 21, 2024

@jsjtxietian Ok, I understood what you mean (A material property of a PrimitiveMesh), I think I've fixed it now

Copy link
Contributor

@jsjtxietian jsjtxietian left a comment

Choose a reason for hiding this comment

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

Tested locally, works as expected.

@Chaosus Chaosus force-pushed the shader_fix_uniform_instance_doc branch 2 times, most recently from 86a2cee to ebe1c92 Compare August 21, 2024 08:03
@Chaosus Chaosus force-pushed the shader_fix_uniform_instance_doc branch from ebe1c92 to cf4d914 Compare August 21, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:editor topic:shaders
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GDShader documentation comments for per-instance uniform does not show in inspector
3 participants