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: Simplify depth clamped clip planes #16069

Merged
merged 1 commit into from
Sep 21, 2022

Conversation

unknownbrackets
Copy link
Collaborator

@unknownbrackets unknownbrackets commented Sep 20, 2022

Sorry, after #16068 I realized this could be simpler with less constants, probably more efficient and more likely to avoid math errors. This makes more sense when thinking about w.

There's no need to think about the scaled Z if we're using w anyway, just use the existing Z clipping range to avoid clamp.

-[Unknown]

There's no need to think about the scaled Z if we're using w anyway, just
use the existing Z clipping.
@unknownbrackets
Copy link
Collaborator Author

Actually, when I made this change I was thinking I must've handled it somewhere but I thought about it again - the clipping was overly lenient before, and this will fix bugs (not just efficiency.)

The integerZ way is only clipping against 0 / 65535. This was the most important (i.e. if you set clamp but then maxz = 65520, you don't want 999999 clamped to 65535), but meant it's not actually clipping against minz/maxz.

However, this change should properly clip against minz and maxz.

-[Unknown]

@hrydgard hrydgard merged commit 3ff400e into hrydgard:master Sep 21, 2022
@Panderner
Copy link
Contributor

Unfortunately this PR breaks Test Drive Unlimited:
ULUS10249_00000

@unknownbrackets unknownbrackets deleted the depth-clamp branch September 21, 2022 13:48
@unknownbrackets
Copy link
Collaborator Author

Can you provide a GE frame dump? The line looks better, at least.

-[Unknown]

@Panderner
Copy link
Contributor

Can you provide a GE frame dump? The line looks better, at least.

-[Unknown]

Here:
ULUS10249.zip

@unknownbrackets
Copy link
Collaborator Author

Hm, I get black bars with that dump unless I filter to 1-8717. I think there's a post-processing effect that's not playing nice.

Post-processed image

On the PSP, it has no black bars but is just as dark and a bit blurry. Anyway, I added filtering to my PSP playback script and got this frame dump (pre-postprocessing):
#16069_ULUS10249_test_drive_clamp

Compared to PPSSPP with the same filter:
Vulkan rendering

It seems like there's an issue with the grass, although I don't see the water like in your screenshot. Software rendering looks good (although slow.)

Seems like it's draw 1408/8813 that is somehow going wrong. It draws in what looks like the right place, but the colors are totally off. Z is 0x0010 - 0x4007, so this shouldn't even be using depth clamp. Maybe that's the issue.

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

unknownbrackets commented Sep 22, 2022

I tried the frame dump on an earlier version and it was still broken. Can you try the version that worked and create a frame dump from that version so it's easier to compare? It was just hardware tess, I forgot I had it on from something else.

I wonder if this is a misdetected framebuffer-as-CLUT and not related to these changes. It seems to be a bug in hardware tessellation, I think because of gen: tex matrix, proj: pos texturing (interesting that software gets it right.)

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

Okay, so this is only happening in GLES, and if I step and flush in the GE debugger, it goes away. So most likely a state flushing issue.

-[Unknown]

unknownbrackets added a commit to unknownbrackets/ppsspp that referenced this pull request Sep 22, 2022
Just recreate when it needs to be larger.  Fixes Test Drive Unlimited
issues noted in hrydgard#16069.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants