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

[Unknown]'s range culling implementation for software transform #15037

Closed
wants to merge 2 commits into from

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Oct 20, 2021

See #13462

Credit to unknownbrackets, of course.

Doesn't fully solve the range culling issues but does solve a lot of it.

I think it makes sense to have this as-is for now for testing, we can optimize it in followup PRs.

EDIT: This might be used as a reference for a better software transform implementation.

For Mali though, which currently needs this for correct culling, I plan a geometry shader solution (yeah I know it's recommended against, but our vertex counts aren't high by modern standards).

@hrydgard hrydgard added Guardband / Range Culling Involves vertices outside fustrum. GE emulation Backend-independent GPU issues labels Oct 20, 2021
@hrydgard hrydgard added this to the v1.13.0 milestone Oct 20, 2021
@vit9696
Copy link
Contributor

vit9696 commented Oct 20, 2021

For Mali though, which currently needs this for correct culling, I plan a geometry shader solution (yeah I know it's recommended against, but our vertex counts aren't high by modern standards).

Why would not it work on macOS? Or do you simply not know?

@hrydgard
Copy link
Owner Author

hrydgard commented Oct 20, 2021

On MacOS I recommend using MoltenVK so you can get Metal instead of OpenGL, where I think cull distance is supported?

But yeah once I've done a geo shader solution for OpenGL in addition to Vulkan for Mali, it might work on Mac.

@vit9696
Copy link
Contributor

vit9696 commented Oct 20, 2021

On MacOS I recommend using MoltenVK so you can get Metal instead of OpenGL, where I think cull distance is supported?

As I mentioned earlier (#13462 (comment)), it is not supported by MoltenVK and will unlikely be, as it is a fairly unused feature. So ideally a Vulkan solution is also added.

@hrydgard
Copy link
Owner Author

Ah, right.

Yeah a Vulkan geometry shader solution is needed anyway on Mali.

@hrydgard
Copy link
Owner Author

hrydgard commented Oct 20, 2021

Although, it seems Metal doesn't support geometry shaders anyway. So we might fall back to software (or the old per-game-specified hacks) on Mac... Or it would also be possible to process triangles with a compute shader as a fallback, but not ideal.

@unknownbrackets
Copy link
Collaborator

Well, I'd written this just to show that in theory it can work, but I don't really like this way. If we're going this path, I'd rather move the transform earlier and just not do it in the shader at all. That'd be easier for lines/points as polygons anyway.

-[Unknown]

@hrydgard
Copy link
Owner Author

Hm, I do agree that we should indeed move the whole transform out, in the software transform case. I'll make this a draft PR for now, just to keep this code around for convenience.

Still won't simplify lines/points-as-polygons in the hardware transform case, but that was never gonna be super easy :)

@hrydgard hrydgard marked this pull request as draft October 20, 2021 13:58
@hrydgard hrydgard removed this from the v1.13.0 milestone Oct 21, 2021
@unknownbrackets
Copy link
Collaborator

unknownbrackets commented Oct 23, 2021

I implemented this but something is wrong, seems like Z testing is broken. Things look generally right, just a few draws that shouldn't happen are, but not from normal Z culling.

master...unknownbrackets:guardband

It's definitely caused by "GPU: Process proj matrix in sw transform", and on triangles, so it's not related to the guardband issue really, just to moving the projection matrix...

-[Unknown]

@hrydgard
Copy link
Owner Author

I haven't looked closely yet, but doing the division by w anywhere else than letting the GPU do it after the vertex shader is going to lead to problems. Since we need the post-divide coordinates to do some checks, we still need to do the divide I guess but we should pass "undivided" xyzw coordinates to the ideally nearly no-op vertex shader.

@unknownbrackets
Copy link
Collaborator

Hm, I guess I'll just move fog to a separate field then and live with slightly expanding it... I did think of that, though, and in the software transform path, we don't really do anything with w outside the guardband culling (which was the main reason I was thinking of moving fog to a dedicated float already, the clipping is more often supported than cull.)

Interpolation should in theory be the same pre-divided or post-divided right?

-[Unknown]

@hrydgard
Copy link
Owner Author

No, since the division by w is not linear, and w is interpolated across the triangle, I don't think interpolation matches, unless w is constant across the triangle.

@unknownbrackets
Copy link
Collaborator

Ah right, that makes sense. I fixed that (updated branch above), which fixed the weirdness indeed.

But still not working in #13950 - the draw at 2063/2174 draws mostly white (it's a non-through rectangle, so always sw transform), and in the #11578 dump, models are offset only in sw transform mode. So I must be applying the project matrix wrong somehow, I guess?

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Oct 24, 2021

Hm, at an initial glance, code looks good to me :/ This is tricky stuff though, easy to miss little things..

@unknownbrackets
Copy link
Collaborator

unknownbrackets commented Oct 31, 2021

Figured out the mistake and opened #15069 without any culling yet. Still seeing some problems with the culling, actually, but haven't messed with it a lot. Fixed.

-[Unknown]

@unknownbrackets unknownbrackets deleted the unknowns-software-transform-culling branch October 31, 2021 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GE emulation Backend-independent GPU issues Guardband / Range Culling Involves vertices outside fustrum.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants