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

GPU: Avoid clears for non-simple depth values #16361

Merged
merged 3 commits into from
Nov 9, 2022

Conversation

unknownbrackets
Copy link
Collaborator

See #12626 and #14223. This makes some related changes.

First, it avoids using a "true" clear when depth is not 0.0 or 1.0 if the game has ever used an equal comparison for depth. This avoids rounding inaccuracies between the clear and draw paths in drivers, i.e. such as in #12626. I'm not sure this fixes that bug, but I'm hopeful based on what's seen.

Note that this is conceptually similar to the "rebuilding jit with rounding mode checks", where we use the more optimal path if possible.

Next, because the above makes doing this safer, this also prefers a scaled depth range to in-shader depth rounding when 24-bit depth is used (practically always.) This should be significantly faster for games that need per-pixel rounding (i.e. #14223), although it probably makes the #16015 workaround less usable.

Because a scaled range will never use 0/1, this does also mean that rounded depth with equality will never use clears, which probably has a penalty. But it's probably made up for by not rounding in the shader.

As an attempt to improve the impact to #16015... I noticed multiple games in that list were using ineffective ALWAYS depth tests and tried forcing it off in that case. I'm not sure it'll help but it doesn't seem like a bad thing.

-[Unknown]

@unknownbrackets unknownbrackets added the GE emulation Backend-independent GPU issues label Nov 8, 2022
@unknownbrackets unknownbrackets added this to the v1.14.0 milestone Nov 8, 2022
@hrydgard
Copy link
Owner

hrydgard commented Nov 8, 2022

Hm, I thought LEQUAL/GEQUAL were super common, is it really worth having a separate mode?

@unknownbrackets
Copy link
Collaborator Author

Well, they're relatively common (as is rounding mode in CPU), but there are definitely plenty of games that just use > or < throughout. I didn't want to penalize them.

-[Unknown]

Comment on lines 127 to 131
} else if (PSP_CoreParameter().compat.flags().PixelDepthRounding) {
// Use fragment rounding on desktop and GLES3, most accurate.
features |= GPU_ROUND_FRAGMENT_DEPTH_TO_16BIT;
features |= GPU_SCALE_DEPTH_FROM_24BIT_TO_16BIT;
} else if (PSP_CoreParameter().compat.flags().VertexDepthRounding) {
features |= GPU_ROUND_DEPTH_TO_16BIT;
features |= GPU_SCALE_DEPTH_FROM_24BIT_TO_16BIT;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So Pixel/Vertex depth rounding are now equivalent? Deserves a comment, and can collapse these two else if a bit. Same in some more places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I'll just keep them different for now. I was worrying that we've made vertex shaders overly complex from that other driver bug (that turned out to be the switch on unsigned), and figured that the range scaling is probably both faster and less likely to cause equal mismatches.

But we can leave it just to pixel for now, and I'll leave vertex alone.

-[Unknown]

Some drivers don't round depth the same way for a clear vs for drawing,
which can cause mismatches.  We only do this if we see equals-style depth
comparison funcs used in drawing.
In most cases, this will be a lot faster than pixel depth rounding, and
may avoid more issues in vertex rounding.
See hrydgard#16015.  Attempting to avoid a driver bug.
@hrydgard hrydgard merged commit a853757 into hrydgard:master Nov 9, 2022
@unknownbrackets unknownbrackets deleted the depth-equal branch November 9, 2022 14:37
@ghost
Copy link

ghost commented Nov 10, 2022

This broke Nayuta graphics using vulkan backend.

Screenrecording_20221110_115829_org.ppsspp.ppsspp.mp4

@unknownbrackets
Copy link
Collaborator Author

Can you upload a frame dump of where it has a trail like that?

-[Unknown]

@ghost
Copy link

ghost commented Nov 10, 2022

here we go
NPJH50625.ppdmp.zip

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants