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

Shadow of Destiny ULUS10459 black sky #9545

Open
benderscruffy opened this issue Apr 4, 2017 · 18 comments
Open

Shadow of Destiny ULUS10459 black sky #9545

benderscruffy opened this issue Apr 4, 2017 · 18 comments
Labels
D3D9 Direct3D 9 Depth / Z Issue involves depth drawing parameters. Vulkan
Milestone

Comments

@benderscruffy
Copy link

benderscruffy commented Apr 4, 2017

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

using ogl backend the sky is correct
ulus10459_00002

@benderscruffy benderscruffy changed the title Shadow of Destiny ULUS10459 black sky DX11 backend Shadow of Destiny ULUS10459 black sky D3dX11 backend Apr 4, 2017
@benderscruffy benderscruffy changed the title Shadow of Destiny ULUS10459 black sky D3dX11 backend Shadow of Destiny ULUS10459 black sky D3d11 backend Apr 4, 2017
@unknownbrackets unknownbrackets added the D3D11 Direct3D 11 label Apr 4, 2017
@unknownbrackets
Copy link
Collaborator

Does this still happen? Does it happen with Vulkan?

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Nov 15, 2017

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 LunaMoo added D3D9 Direct3D 9 Vulkan Depth / Z Issue involves depth drawing parameters. labels Nov 15, 2017
@LunaMoo
Copy link
Collaborator

LunaMoo commented Nov 20, 2017

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
Copy link
Collaborator

unknownbrackets commented Nov 21, 2017

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
Copy link
Owner

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
Copy link
Collaborator

LunaMoo commented Nov 21, 2017

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
Copy link
Owner

Oh, right. Sigh...

@hrydgard
Copy link
Owner

Wait, that happens in D3D11 too?

@LunaMoo
Copy link
Collaborator

LunaMoo commented Nov 21, 2017

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

@hrydgard
Copy link
Owner

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
Copy link
Collaborator

LunaMoo commented Nov 21, 2017

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

@unknownbrackets
Copy link
Collaborator

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

-[Unknown]

unknownbrackets added a commit to unknownbrackets/ppsspp that referenced this issue Sep 19, 2018
Uses the same logic as Vulkan, improving hrydgard#9545 on most D3D11 devices.
@unknownbrackets
Copy link
Collaborator

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

-[Unknown]

@benderscruffy benderscruffy reopened this Oct 3, 2018
@hrydgard hrydgard added Vulkan and removed D3D11 Direct3D 11 Vulkan labels Oct 3, 2018
@benderscruffy benderscruffy changed the title Shadow of Destiny ULUS10459 black sky D3d11 backend Shadow of Destiny ULUS10459 black sky Oct 5, 2018
@hrydgard hrydgard modified the milestones: v1.8.0, v1.9.0 Feb 6, 2019
@unknownbrackets
Copy link
Collaborator

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
Copy link

ghost commented Sep 21, 2022

How about now? 3ff400e

@unknownbrackets
Copy link
Collaborator

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
Copy link
Owner

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
Copy link
Collaborator

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]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D3D9 Direct3D 9 Depth / Z Issue involves depth drawing parameters. Vulkan
Projects
None yet
Development

No branches or pull requests

4 participants