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

Swap depth range to use minz/maxz #8461

Merged
merged 5 commits into from
Jan 20, 2016
Merged

Conversation

unknownbrackets
Copy link
Collaborator

This does a few things:

  1. Properly supports minz/maxz by using them as the depth range.
  2. Flushes when clipEnable changes.
  3. Sacrifices some bits of precision to allow for "fake clamping" of the depth range when clip is enabled. Currently, I've sacrificed 2 bits allowing depth between [-2.5, 2.5].

This actually fixes MotoGP for me (would be nice to confirm), since it is using clipEnable, and therefore clamping the depth.

Some flaws (not sure if important):

  • Still does not handle rounding per my tests; e.g. without clip enabled, cuts off at precisely [0, 65535]. Not worse than before.
  • Per-vertex depth rounding comes out with slightly offset values. I'm not sure if this is just a precision issue, or if I've made an error. But they are close and Phantasy Star Portable 1 works.
  • Per-pixel depth rounding is potentially wrong now; but I couldn't get it to come out right (tried applying offset and scale.) Phantasy Star Portable 1 still works as-is if per-pixel is forced.

Could use help on the per-pixel rounding. This is what I tried:

    if (gstate_c.Supports(GPU_ROUND_FRAGMENT_DEPTH_TO_16BIT)) {
        const float offset = 0.5f * (DepthSliceFactor() - 1.0f) * (1.0f / DepthSliceFactor());
        const double scale = DepthSliceFactor() * 65535.0;
        WRITE(p, "  highp float z = gl_FragCoord.z;\n");
        WRITE(p, "  z = floor((z - %f) * %f) * %f + %f;\n", offset, scale, 1.0 / scale, offset);
        WRITE(p, "  gl_FragDepth = z;\n");
    }

Anyway, this seems like an improvement otherwise.

Fixes #8456 and fixes #8424.

-[Unknown]

This allows values that fall outside the viewport, but still within the
depth range, to be drawn.
We'll allow extra space in both directions.  It's not exactly right, but
it's a fast approximation.
@unknownbrackets unknownbrackets added this to the v1.2.0 milestone Jan 19, 2016
@hrydgard
Copy link
Owner

Cool. Getting this tested with Heroes Phantasia might be a good idea if pixel depth rounding has changed..

@hrydgard
Copy link
Owner

Seems to work well in all the games I tried.

@daniel229
Copy link
Collaborator

Heroes Phantasia missing menu.
01

disable pixel depth rounding,menu appearing, but it will flicker badly.

@daniel229
Copy link
Collaborator

Where is DepthSliceFactor()

@unknownbrackets
Copy link
Collaborator Author

Sorry, add this to GPUStateUtils.cpp:

float DepthSliceFactor() {
    return depthSliceFactor;
}

And then GPUStateUtils.h:

float DepthSliceFactor();

-[Unknown]

@daniel229
Copy link
Collaborator

Pixel depth rounding works with that code,and even flicker less than before.

@daniel229
Copy link
Collaborator

That modified per-pixel rounding code make Phantasy Star Portable 2 JP missing text.

@unknownbrackets
Copy link
Collaborator Author

Turns out the issue was precision (sprintf("%f").) I've changed it to something simpler.

Also, how does Heroes Phantasia fair with static const float depthSliceFactor = 256.0f; instead of 4.0f?

-[Unknown]

@daniel229
Copy link
Collaborator

Phantasy Star Portable 2 JP now flicker even with PixelDepthRounding.
For Heroes Phantasia 256.0f seems not better or worse than 4.0f

@unknownbrackets
Copy link
Collaborator Author

If you change depthSliceFactor to 1.0f does Phantasy Star Portable 2 JP stop flickering? Or does it still flicker? Does 256 help it, either?

-[Unknown]

@daniel229
Copy link
Collaborator

Never mind,just forgot add game id to the list,it works fine.

@unknownbrackets
Copy link
Collaborator Author

Ah, great. So then sounds like pixel rounding is working fine now, and this fixes a few bugs otherwise. Yay.

-[Unknown]

@hrydgard
Copy link
Owner

@daniel229 please send pull requests with any missing game ids :)

hrydgard added a commit that referenced this pull request Jan 20, 2016
Swap depth range to use minz/maxz
@hrydgard hrydgard merged commit 4c8384a into hrydgard:master Jan 20, 2016
@unknownbrackets unknownbrackets deleted the gpu-depth branch January 20, 2016 14:58
@daniel229
Copy link
Collaborator

Fixed a minor issue in Danball Senki W.
01
02

@unknownbrackets
Copy link
Collaborator Author

That's probably the minz/maxz clipping. Cool.

-[Unknown]

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.

Crash Mind over Mutant does not show anything except the videos. MotoGP graphical artifact
3 participants