hrydgard / ppsspp

A PSP emulator for Android, Windows, Mac and Linux, written in C++. Want to contribute? Join us on Discord at https://discord.gg/5NJB6dD or just send pull requests / issues. For discussion use the forums at forums.ppsspp.org.
https://www.ppsspp.org
Other
10.87k stars 2.13k forks source link

Shadow of Destiny ULUS10459 black sky #9545

Open benderscruffy opened 7 years ago

benderscruffy commented 7 years ago

ppsspp-v1.4-20-g8244b82 using d3d11 backend black sky ulus10459_00000

using ogl backend the sky is correct ulus10459_00002

unknownbrackets commented 6 years ago

Does this still happen? Does it happen with Vulkan?

-[Unknown]

LunaMoo commented 6 years ago

This might be same depth clamping issue as on some stages in Valkyria Chronicles discussed in here, except here by some accident it works in OGL. Affects all the other backends. Frame dump to reproduce: ULUS10459_0001.zip

LunaMoo commented 6 years ago

Hmm not sure if that's a depth issue anymore, as can be seen in the included dump above, depth is weird in broken backends showing 0.375/0.0 where OGL shows 0.0/0.0 and 1.0/65535.0 after the skybox shows, but it doesn't show after draw, but after "Clear mode: 000401 (on, depth)" that follows the draw. Should that clear mode set depth or something?

Edit: Couldn't figure it out, but I could brake it same way in OGL by forcing:

Clear mode: 000701 (on, color, alpha/stencil, depth)

or

Clear mode: 000501 (on, color, depth)

in place of the (On, depth) clear, so maybe that leads somewhere:].

unknownbrackets commented 6 years ago

Hmm so.... this is because of an insane viewport offset for Z. It's a problem with the accurate depth path.

Viewport Scale 739.000000, 447.000000, -32767.000000 Viewport Offset 2048.000000, 2048.000000, 145281024.000000

  1. Clear color/alpha/depth: everything to 0.
  2. Clear again, still zero.
  3. Draw some sky with crazy high depth values.
  4. OpenGL has sky now, Vulkan doesn't.
  5. Clear depth only: to 0.
  6. Draw more things that do draw (sane viewport depth.)

Software handles this, I believe, correctly. Clip is enabled, so it clamps, and all is well.

OpenGL clamps the specified viewport to [1, 1]. This effectively ignores the crazy offset, and effectively clamps to 65535. It would fail for sufficiently negative vertex Z values, I think, but those don't happen here.

However, accurate depth sets the depth range based on minz/maxz (since this is the actual "clip range" of depth values), and then tries to clamp values outside this range by hoping they are within [-1.5, 2.5].

This value of course is 2216.88 or so, well outside that range.

There's a hack that fixes this. I'm not sure how bad or not so bad it is.

        if (gstate.isClippingEnabled() && gstate_c.Supports(GPU_SUPPORTS_ACCURATE_DEPTH)) {
            vpZCenter = std::max(-65535.0f, vpZCenter);
            vpZCenter = std::min(65535.0f * 2.0f, vpZCenter);
        }

In ConvertViewportAndScissor(). It makes the sky appear, which I'm pretty sure confirms my above theories. The problem with it is that a sufficiently crazy negative vertex Z could cause problems. Ugh.

The right solution is probably real depth clamping, I guess...

-[Unknown]

hrydgard commented 6 years ago

For achieving the maybe-less-than-noble goal of getting 1.5 out, I'm going to add a compat flag to blacklist the accurate depth path from games that it breaks for various reasons like this. Was just going to put Midnight Club on it, but I'll throw in this one as well. As this doesn't solve the issue for real, I won't close.

LunaMoo commented 6 years ago

This compat might need to be disabled for AMD ~ restores #10082 which apparently wasn't Vulkan specific:|. It made all non-OGL backends looks like: ulus10459_00000

hrydgard commented 6 years ago

Oh, right. Sigh...

hrydgard commented 6 years ago

Wait, that happens in D3D11 too?

LunaMoo commented 6 years ago

Yeah d3d11, as well as d3d9 and vulkan they all break without accurate depth;].

hrydgard commented 6 years ago

I don't have reliable vendor checks on those APIs though. Can you let me know what's next to "Vendor" in system info on D3D9/11, is it simply "AMD"?

LunaMoo commented 6 years ago

Unfortunately there's only name of my GPU model there so it will differ for everyone:|, you can see screenshots from all backends https://github.com/hrydgard/ppsspp/issues/10109#issuecomment-343794675 and under it in https://github.com/hrydgard/ppsspp/issues/10109#issuecomment-343801716 an output from https://github.com/unknownbrackets/ppsspp/tree/driver-ver which has more info.

unknownbrackets commented 5 years ago

Note to self: DepthClipEnable false might help this (and cause #11399 to be a problem) on Direct3D 11.

-[Unknown]

unknownbrackets commented 5 years ago

Well, it's still an issue in Direct3D 9 and Vulkan when depth clamp is not supported.

-[Unknown]

unknownbrackets commented 5 years ago

Just a note so we don't think the "accurate depth" path is always wrong: Mimana Iyar Chronicle (ULUS10492_#9545_mimana_depth_clamp.zip) also has a black sky issue in the "Tree of Elves" area, but it's in any backend without depth clamp.

-[Unknown]

ghost commented 1 year ago

How about now? https://github.com/hrydgard/ppsspp/commit/3ff400e40e122dcba771d32b8a064a3b42e7e371

unknownbrackets commented 1 year ago

This hasn't really changed since 2018, except that OpenGL / GLES now sometimes depth clamp. We would need to use another varying and adjust Z, writing Z always in the fragment shader on affected devices. It'd be a lot slower.

-[Unknown]

hrydgard commented 1 year ago

I thourgh of an idea to use the vertex cache and store computed bounding boxes for draw calls, and only activate fragment shader clipping when the bounding box for a draw is near the clip plane... Could avoid it for a lot of draws. Though the near-plane triangles often cover a lot of screen space...

unknownbrackets commented 1 year ago

Would need to do that on the projected Z, although could probably compute a vector to dot product every input position against to determine if it crosses the negative Z plane. The same could be used to determine if depth clamp clipping or depth clamp at all would be required too, I think. That might make it much cheaper to write the actual depth value in the fragment shader as well (when no depth clamp.)

-[Unknown]