-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Implement geometry shader for range culling #16142
Conversation
If you have clip and cull (which you do), you should be fine. We're just using geometry shaders to workaround the missing cull distance support. Probably a driver thing or some specifics of Vulkan spec requirements causing it not to have geometry shaders only in Vulkan. -[Unknown] |
Very nice, thanks for taking over and finishing this up! I will test it a bit on some Mali devices before merging, in a little while. |
The end part of the geometry shader computes ProjZ without then using it, if clip distance is not supported. Can be fixed in a followup though, not important since dead code elimination should take care of it anyway.. Unfortunately, while this seems to work fine on my S20 (pending more testing though), my S8 has extreme slowdowns and triangle glitches. Will probably have to disable it on devices with a driver version older than some value... |
Yeah, not seeing any issues on S20. So I think this will need a Bugs::GEOMETRY_SHADER_BROKEN and add it to ARM devices with majorVersion < 31 to be safe. (though it's possible of course that earlier versions than 31 are fine, 16 is definitely broken). |
Maybe could temporarily make this a config setting (default off) to give people a chance to test and report which devices it works poorly for? It'd be nice to cover more devices, I don't actually have an rpi4 to know if it's horrible performance there or not. We could even only show the checkbox when cull distance is not supported but geometry is. What do you think? -[Unknown] |
Yeah, I'm fine with that. I do want it force-disabled on mali driver <= 15 though, although I can add that afterwards. |
Not yet used, but dirtied at the right times.
We're still not generating them, yet. But this tracks the objects and IDs through the pipeline.
The compat setting was really for some previously buggy cases that couldn't work without cull.
It might be supported without cull or GS. Otherwise we may need to clip the triangles manually.
They're too slow to be usable.
c6f3b3c
to
2832edc
Compare
Without clipping, these aren't used (but could be in the future with manual clipping.)
Added, only the last 3 commits are new, I just fixed a typo in a prior commit message. I decided to leave it on by default, so we get more reports. It can be turned off easily. Also, for posterity or anyone testing out this setting - here's a simple test frame dump for culling: #8251_ULUS10279_brave_black_triangle.zip Should look like this, correctly: -[Unknown] |
Thanks, and nice to have that framedump here conveniently! |
This does everything except the clip distance in the geometry shader, when supported. I've only really tested this on desktop, though.
Closes #15059. I reordered the commits so that state caching, cache loading, etc. wouldn't be broken at the start and reduced the debug changes a bit.
Examples of cases this might help:
Examples this won't help:
In the Mali case, we would still need to handle negative Z clipping, as well as depth clamp clipping. That means a triangle might need to be clipped against at most two planes. But we can add that later - this should still be an improvement compared to rendering on those devices today.
I'm also not sure DisableRangeCulling is still needed. I think that was largely a hack from before I understood the viewport Z vs projected Z culling/clipping behavior, so none of those games are likely to be broken now either way.
-[Unknown]