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

Implement geometry shader for range culling #16142

Merged
merged 12 commits into from
Oct 2, 2022

Conversation

unknownbrackets
Copy link
Collaborator

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:

  • ARM Mali (note: do not have shader clip either, not yet handled.)
  • Broadcom / Raspberry Pi 4 (supports clip, but not cull, in shader.)

Examples this won't help:

  • Apple devices (no geometry shaders, no cull distance, at least NANs and clip work.)
  • GLES/OpenGL devices (I think most that support geometry shaders would support Vulkan, though?)
  • Direct3D 9 or Direct3D 11 with feature level 9 cards (which wouldn't support geometry shader anyway.)

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]

@unknownbrackets unknownbrackets added this to the v1.14.0 milestone Oct 2, 2022
@ghost
Copy link

ghost commented Oct 2, 2022

My vulkan backend doesn't support geo shader.
IMG_20221002_125033
But my OpenGL support it?
IMG_20221002_124919

@unknownbrackets
Copy link
Collaborator Author

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]

@hrydgard
Copy link
Owner

hrydgard commented Oct 2, 2022

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.

@hrydgard
Copy link
Owner

hrydgard commented Oct 2, 2022

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...

@hrydgard
Copy link
Owner

hrydgard commented Oct 2, 2022

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).

@unknownbrackets
Copy link
Collaborator Author

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]

@hrydgard
Copy link
Owner

hrydgard commented Oct 2, 2022

Yeah, I'm fine with that. I do want it force-disabled on mali driver <= 15 though, although I can add that afterwards.

unknownbrackets and others added 11 commits October 2, 2022 07:42
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.
Without clipping, these aren't used (but could be in the future with
manual clipping.)
@unknownbrackets
Copy link
Collaborator Author

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:

#8251_ULUS10279_brave_black_triangle

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Oct 2, 2022

Thanks, and nice to have that framedump here conveniently!

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

Successfully merging this pull request may close these issues.

None yet

2 participants