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

D3D11: Use appropriate shader for PEEK_Z. #236

Merged
merged 1 commit into from Apr 27, 2014

Conversation

magumagu
Copy link
Contributor

Trying to use GetDepthMatrixProgram outside of
TCacheEntry::FromRenderTarget is a bad idea, so don't. Instead, use a
shader which only copies the input.

Fixes lens flare depth test in Twilight Princess. See
http://code.google.com/p/dolphin-emu/issues/detail?id=5999 .

Trying to use GetDepthMatrixProgram outside of
TCacheEntry::FromRenderTarget is a bad idea, so don't.  Instead, use a
shader which only copies the input.

Fixes lens flare depth test in Twilight Princess. See
http://code.google.com/p/dolphin-emu/issues/detail?id=5999 .
@Sonicadvance1
Copy link
Contributor

Maybe there should be a comment as to why it uses the Color program instead of the depth program for PEEK_Z.

@neobrain
Copy link
Member

Why is it a bad idea to use GetDepthMatrixProgram outside of TCacheEntry::FromRenderTarget?

As far as I can see it should be possible to use the depth matrix program in other places, too. That said, there's no denying that the current depth matrix program is simply incorrect, in particular due to precision loss by multiplying with 16777215.0f/16777216.0f (also, the d3d code is setting the shader constants such that blue and green components get calculated per pixel even though the render target is R32F). It should be possible to change it to use integers though.

@magumagu
Copy link
Contributor Author

Well, there's a couple issues here. One, there isn't any reason to use a pixel shader to swizzle the output for a single pixel. Two, (which I think is the cause of the issue) Renderer::AccessEFB uses a DXGI_FORMAT_R32_FLOAT texture, where GetDepthMatrixProgram expects something more like DXGI_FORMAT_R8G8B8A8_UNORM.

I mean, I'm sure the rounding is broken too, but I'm pretty sure that isn't the issue here.

@neobrain
Copy link
Member

I was going to object against the fact that GetDepthMatrixProgram supposedly expects an R8G8B8A8_UNORM format, but you made a point when you mentioned there's no need to "use a pixel shader".

@neobrain
Copy link
Member

I don't think the usage of GetColorCopyProgram needs to be commented any further.

LGTM. I'll let someone else have the last vote on GetColorCopyProgram commenting.

@neobrain
Copy link
Member

Apparently no one else is interested in commenting. Merging.

neobrain added a commit that referenced this pull request Apr 27, 2014
D3D11: Use appropriate shader for PEEK_Z.
@neobrain neobrain merged commit a40ea4e into dolphin-emu:master Apr 27, 2014
@magumagu magumagu deleted the d3d11-fix-peekz branch May 4, 2014 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants