-
Notifications
You must be signed in to change notification settings - Fork 1.9k
egui-wgpu: attach stencil buffer #7702
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
base: main
Are you sure you want to change the base?
Conversation
|
Preview available at https://egui-pr-preview.github.io/pr/7702-switchwgpu-stencil-buffer View snapshot changes at kitdiff |
Wumpf
left a comment
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 haven't checked what wgpu's validation does today, but WebGPU spec says that if there is no stencil attachment, the stencil ops musn't be set, see https://www.w3.org/TR/webgpu/#depth-stencil-attachments
so this should be set only if there is a stencil aspect
For consistency the web backend should do the same as well.
Generally I'm a bit skeptical about the fact that egui_wgpu provides a depth(/stencil) buffer integration in the first place. Instead it should just be easier to own the final render output yourself for situations where you need a full screen depth buffer etc.
This is already possible today, but quite cumbersome and requires a lot of context.
Given that we already have an optional depth buffer that is also cleared with an arbitrary value it's only consistent I figure.
Alternatively, we could make more of these settings things that one can optionally configure on the renderer 🤔
The same applies for the depth aspect, which the current implementation doesn't check.
Oh I see the code is duplicated in
I agree.
Can you clarify what you mean by that? Do you mean |
…ture's aspects before attaching
Yes, I missed that a bit. Also you have to dig through the source code to figure out how the render pass is actually being configured. But the defaults work well enough I think and one can always clear the depth-stencil buffer with whatever they want with a single draw call (which you might do anyway to set the background color). |
tbh I haven't thought it through entirely, but I'm thinking that the egui/eframe/wgpu integration should make it easier to just swap out the entire "main render pass" setup against something custom without having to redo half of eframe from scratch as it's the case now :/ |
Wumpf
left a comment
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 great, thank you!
| .options | ||
| .depth_stencil_format | ||
| .is_some_and(|depth_stencil_format| { | ||
| depth_stencil_format.has_depth_aspect() |
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.
really nice catch there, thank you!
I'm using eframe to run an app with the wgpu backend and built a custom widget that renders a 3D scene. I want to use the stencil buffer. eframe allows me to specify the depth and stencil buffer and egui-wgpu chooses the right texture format. It does attach the depth texture when rendering, but leaves the stencil attachment empty.
It makes sense to me that if a depth/stencil texture is created both are attached.
I tested that this doesn't break anything if the depth texture doesn't have a stencil part. I ran all tests and only 2 snapshot tests failed with superficial diffs. I also ran the hello_world app with the wgpu feature to make sure an app without a depth texture would still work.
./scripts/check.shdoesn't run for me (typosnot found)Is this something you want to merge into egui-wgpu at all? If so, please let me know if there is more I can do to make this easier to merge.
I can understand if this is a niche use case (in contrast to depth buffer only), but the only workaround would be to create my own render pass, render to a texture and then blit the result. For more complex setups this might be the way to go anyway.