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

Depth scale fixes #16947

Merged
merged 11 commits into from
Feb 11, 2023
Merged

Depth scale fixes #16947

merged 11 commits into from
Feb 11, 2023

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Feb 9, 2023

Changing depth scale during runtime had some side effects. Fixes #16941.

We can't bake scale factors into shaders anymore, so remove an instance of that. Also, when initializing depth buffer using DrawPixels we didn't obey the depth scale. This was fine for the testing done by Burnout Dominator's effect, but wrong.

Hm, noticed that Ratchet & Clank's depth trickery is a bit broken again on master too, and seems to have been for a while. This makes it even worse, so should probably look at that before merging.. Yeah we're not handling depth scale properly in either GenerateDraw2D565ToDepthFs or GenerateDraw2D565ToDepthDeswizzleFs.

Will fix Ratchet tomorrow.

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Feb 9, 2023
@hrydgard hrydgard added this to the v1.15.0 milestone Feb 9, 2023
@hrydgard
Copy link
Owner Author

hrydgard commented Feb 10, 2023

Actually, I'm gonna peel off the depth clearing and commit that first. Then I plan to clean up our depth conversion functions (#16948), and then rebase this on top of that.

@hrydgard hrydgard marked this pull request as draft February 10, 2023 15:55
@hrydgard hrydgard mentioned this pull request Feb 10, 2023
@hrydgard
Copy link
Owner Author

hrydgard commented Feb 11, 2023

This now contains the commits from #16949 , and on top of that, fixes various issues with depth buffer conversion. For example, we can't use the same shader both for copying depth and converting color to depth.

This fixes the depth regression in Ratchet & Clank.

@hrydgard hrydgard marked this pull request as ready for review February 11, 2023 18:01
Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

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

Would be a good idea to double check the Phantasy Star text dialog issue in D3D9 and etc. with this, as I think that was a good example of using mixed modes (transform and through) with equal depth that needs to round correctly.

-[Unknown]

@unknownbrackets
Copy link
Collaborator

Also just to note, originally when I wrote the depth download shader I simply had a pspautotest write specific depth values and then tested that a dmac copy + CPU read read back the exact values both on desktop and mobile. That motivated the two paths, for example.

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Feb 11, 2023

Right, I'll do a little extra testing. I'm hoping that we can match it with a single path now.

Is that pspautotest that you used around, and if so can we enable it to run by default?

@hrydgard
Copy link
Owner Author

After some brief testing, I'm not seeing any issues whatsoever.

But yeah, ideally we should do another round of bit exact comparisons.. and as I said above, turn on such a test by default.

@hrydgard hrydgard merged commit c02fbd4 into master Feb 11, 2023
@hrydgard hrydgard deleted the depth-scale-fixes branch February 11, 2023 22:09
@unknownbrackets
Copy link
Collaborator

I'll look, but it's of course difficult to do mobile GPU tests, our CI doesn't even run with any GPU.

-[Unknown]

@hrydgard
Copy link
Owner Author

Right, i keep forgetting that little issue...

@hrydgard
Copy link
Owner Author

Though on Linux, we could theoretically leverage llvmpipe - while not a real gpu test, could catch issues like mismatched depth.

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.

Regression in Outrun's environment mapping, assert hit
2 participants