-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add material with specializations for debugging meshes #7390
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
Add material with specializations for debugging meshes #7390
Conversation
// This is a more useful default than to saturate/clamp. | ||
return vec4<f32>(uv.x % 1.0, uv.y % 1.0, 0.0, 1.0); | ||
#else | ||
return vec4<f32>(0.0, 0.0, 0.0, 1.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a sane default when a user wants to debug UVs (or other attributes) but there are none?
I opted for just showing black.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect something garish and visibly different: I would go with hot pink as that's pretty common for "something has gone wrong".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using @superdump's shader now, which does indeed use pink.
@@ -13,7 +13,11 @@ repository = "https://github.com/bevyengine/bevy" | |||
rust-version = "1.67.0" | |||
|
|||
[workspace] | |||
exclude = ["benches", "crates/bevy_ecs_compile_fail_tests", "crates/bevy_reflect_compile_fail_tests"] | |||
exclude = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a format-on-save whitespace change.
Cargo.toml
Outdated
name = "Debug mesh" | ||
description = "Debug meshes by displaying UVs, world normals, etc." | ||
category = "3D Rendering" | ||
wasm = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasm = false
is this an opt-out of having the example on Bevy's homepage?
I haven't tried the example on a wasm build, let me know what to put here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct. I'd set this to true
, it's visually impressive and good to verify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set to true now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome! Can we enable this as a mode in our scene_viewer tool?
I did a PR a long time ago that focused more on screen-wide PBR debug stuff like this. #5398 I'm not sure which I feel is better, or if both are desirable. I feel like a whole-screen solution is useful, just as a per-mesh entity solution is useful. Perhaps they can share some parts or something. Also things have moved on a lot since my PR. It needs updating and there are many more things to debug now. :) Would you perhaps be up for putting together a combined solution? |
@alice-i-cecile thanks for the review comments, I'll address them after figuring out where to take this. @superdump Locally now I've merged your PR into this one (so git history should be preserved). Adapted it a bit since there have been a few changes, was not too much work. So now the scene viewer works when cycling all of the enum variants from your PR. Since yours was a lot more thorough it makes sense to use what you made.
Another thing: If we have the scene viewer, do we still want the example I added or should I remove it? |
@alice-i-cecile Merged in @superdump's PR so the scene viewer is included now. @superdump From your PR the main changes I've made is to update the PBR functions shader slightly since the To attempt getting the best of both worlds I added So I'll debug that. However the scene viewer works: Also even though I did a UVs, depth, interpolated normals, interpolated tangets work.
Not sure why, and I don't get how it works for scene viewer. Perhaps the cache invalidation is relevant? |
I think having an example is good too. The |
This is super cool! The ability to have multiple instances of a mesh side-by-side with different debug views at the same time is surely incredibly useful for hacking away on shaders! |
Could you add:
into the PR description? |
b1beb4e
to
49ebb8d
Compare
@superdump updated the description. I still need to figure out why only some debug modes work when I'm using materials. |
Merging #7431 so you can build on top of that now to simplify the ifdeffery in this one. :) |
Thanks! I'll revisit this in the next couple of days and apply that PR 👍 |
Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
Co-authored-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no> Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
…ommits merged. Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
…. Material only shows pink, have to debug. Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
49ebb8d
to
0ee578d
Compare
Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
Signed-off-by: Torstein Grindvik <torstein.grindvik@nordicsemi.no>
Got rid of a lot of However I'm a bit stuck on understanding why the material approach does not work (it works only for the few shown in the images). I'll compare my understanding of the global/resources vs. the material approach: The global/resource based version checks if the By this logic I created So intuitively I thought both approaches should work since the goal is simply to get the right shader defs in place. |
At least at first glance, your description sounds reasonable. I'll take a look at the PR locally. |
The problem is that the To be more specific and concrete, Hmm, I think to avoid duplication, the material approach will need some more thought. I'm not yet sure how to solve it without just building it into |
Thanks for looking into that. Since we currently don't have a clear image of how to solve that, should I perhaps split this PR up? |
I don't think this is close enough to shippable to make the 0.10 release, but feel free to prove me wrong ;) |
I see the PBR shaders have changed a lot, seems the PBR pipeline has gotten more structured and extensible in a year which is nice. |
Objective
Allows users to debug meshes by looking at UVs, and world {position, normals, tangents}.
Solution
Add a material which allows per-mesh viewing of the above attributes.
Changelog
Added
DebugMeshMaterial
with pipeline specializations for attributes: UVs, world position, world normals, world tangents.debug_mesh
example to show usage ofDebugMeshMaterial
s on a simple cube and on the flight helmet.Discussion
Correctness
UVs (2nd debug helmet front-to-back)
debug_mesh.mp4
Compare the above Bevy video to the glTF Sample Viewer (see here):
This generally looks correct to me although the web viewer has darker colors.
Features
Perhaps local position, normals, tangents would be good to have.
Let me know. I haven't looked into if I have all the functions/data I need in the frag shader for that to be a simple calculation or if a different vertex shader would be needed then.