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

Ensure shadow material and mesh are not used with wireframe mode #98683

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

clayjohn
Copy link
Member

Fixes: #98532

In all three backends, the shared shadow material and shadow mesh were being used despite the wireframe render mode being on. This meant that for all backends shadows would never be wireframed and in the forward+ backend the depth prepass used the wrong shader + mesh.

Also, in the Compatibility renderer we weren't requesting wireframe mode at render time when using the wireframe render_mode.

Compatibility Mobile Forward+
Before Screenshot from 2024-10-30 17-01-33 Screenshot from 2024-10-30 17-01-21 Screenshot from 2024-10-30 17-01-45
After Screenshot from 2024-10-30 17-03-04 Screenshot from 2024-10-30 17-02-38 Screenshot from 2024-10-30 17-02-14

@clayjohn clayjohn added this to the 4.4 milestone Oct 31, 2024
@clayjohn clayjohn requested a review from a team as a code owner October 31, 2024 00:10
@clayjohn
Copy link
Member Author

CC @tetrapod00

Copy link
Member

@Calinou Calinou 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 (rebased on top of master 87318a2), it works as expected in all rendering methods.

Hoever, in Compatibility, when running the project, the mesh doesn't use wireframe:

image

It renders correctly in the editor which is also using Compatibility:

image

To test this, edit car_base.tscn in Truck Town and give the Body node material a custom shader with render_mode wireframe;.

@clayjohn
Copy link
Member Author

clayjohn commented Nov 6, 2024

@Calinou Did you remember to call RenderingServer.set_debug_generate_wireframes(true) in your project?

It is needed for the engine to generate the wireframe versions of meshes. It is true by default in the editor

@Calinou
Copy link
Member

Calinou commented Nov 6, 2024

@Calinou Did you remember to call RenderingServer.set_debug_generate_wireframes(true) in your project?

I forgot to do that. I've added a call to RenderingServer.set_debug_generate_wireframes(true) in car_select.gd's _ready() function and it still doesn't work with this PR though, even when running the project from the editor.

PS: The method's description is outdated in the class reference.

@clayjohn
Copy link
Member Author

clayjohn commented Nov 6, 2024

@Calinou Did you remember to call RenderingServer.set_debug_generate_wireframes(true) in your project?

I forgot to do that. I've added a call to RenderingServer.set_debug_generate_wireframes(true) in car_select.gd's _ready() function and it still doesn't work with this PR though, even when running the project from the editor.

PS: The method's description is outdated in the class reference.

Thanks for taking a look. I will update the docs in this PR and I'll take another look to see why the setting might be ignored at run time.

@clayjohn
Copy link
Member Author

@Calinou Did you remember to call RenderingServer.set_debug_generate_wireframes(true) in your project?

I forgot to do that. I've added a call to RenderingServer.set_debug_generate_wireframes(true) in car_select.gd's _ready() function and it still doesn't work with this PR though, even when running the project from the editor.

PS: The method's description is outdated in the class reference.

I took a look at truck town locally. The problem with truck town is that it is loading the mesh before RenderingServer.set_debug_generate_wireframes(true) is called. You need to set RenderingServer.set_debug_generate_wireframes(true) before loading the mesh otherwise wireframes will not be generated. Fixing that makes the demo work correctly.

@clayjohn clayjohn requested a review from a team as a code owner November 13, 2024 05:17
@clayjohn
Copy link
Member Author

Rebased and updated the documentation. This is ready for merging now

@akien-mga akien-mga requested a review from Calinou November 13, 2024 07:02
And in the Compatibility renderer actually use the wireframe render mode
@clayjohn
Copy link
Member Author

Updated docs based on the comments. Should be good to merge now

@Repiteo Repiteo merged commit fd4c29a into godotengine:master Nov 18, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 18, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wireframe Shader bugged in Forward+, does not work at all in Compatibility
4 participants