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

GL: fw_dice gcm sample broken texture #1887

Closed
danilaml opened this issue Jul 10, 2016 · 7 comments · Fixed by #1892
Closed

GL: fw_dice gcm sample broken texture #1887

danilaml opened this issue Jul 10, 2016 · 7 comments · Fixed by #1892

Comments

@danilaml
Copy link
Contributor

danilaml commented Jul 10, 2016

Works fine with D3D12 and Vulkan.

Happened after #1865 by @vlj

@raven02 raven02 changed the title fw_dice test broken in oGL GL: fw_dice gcm sample broken texture Jul 11, 2016
@raven02
Copy link
Contributor

raven02 commented Jul 11, 2016

@vlj . In that refactoring commit , i observed

GL

-   __glcheck glDepthMask(rsx::method_registers[NV4097_SET_DEPTH_MASK]);
+   __glcheck glDepthMask(rsx::method_registers.depth_write_enabled());

DX12

-   prop.DepthStencil.DepthWriteMask = !!(rsx::method_registers[NV4097_SET_DEPTH_MASK]) ? D3D12_DEPTH_WRITE_MASK_ALL : D3D12_DEPTH_WRITE_MASK_ZERO;
+   prop.DepthStencil.DepthWriteMask = rsx::method_registers.depth_write_enabled() ? D3D12_DEPTH_WRITE_MASK_ALL : D3D12_DEPTH_WRITE_MASK_ZERO;

Vulkan

-   properties.ds.depthWriteEnable = (!!rsx::method_registers[NV4097_SET_DEPTH_MASK]) ? VK_TRUE : VK_FALSE;
+   properties.ds.depthWriteEnable = rsx::method_registers.depth_write_enabled() ? VK_TRUE : VK_FALSE;

where

    bool depth_write_enabled() const
    {
        return !!registers[NV4097_SET_DEPTH_MASK];
    }

It is true for DX12 and Vulkan but not GL i think (i may be wrong)

@kd-11
Copy link
Contributor

kd-11 commented Jul 11, 2016

Just noticed this as well. The depthmask seems correct by the way since GL just sets it to 0 or 1

@raven02
Copy link
Contributor

raven02 commented Jul 11, 2016

I see @kd-11

@kd-11
Copy link
Contributor

kd-11 commented Jul 11, 2016

The problem seems to be the separable back and front diffuse lighting, but that makes no sense (looking at the shaders, they should have the same value, but they dont)

EDIT: Seems the issue is more complicated than that. I'll take a deeper look.

@kd-11
Copy link
Contributor

kd-11 commented Jul 11, 2016

I'm not sure what's happening here but I've noticed the following:

The sample passes diffuse color using the vertex_registers. The register data size is 16 bytes and attribute the size is given as 4. Unless the size parameter is the size of the whole vertex attribute,
this implies that only one vertex is defined since float4 is 16 bytes wide. The buffer is then accessed using vertexID which is out of bounds hence the black bits. For some reason texel reads on vulkan seem to be more lenient. Can confirm that hacking the vertex shader to read vertex 0 always returns correct results. Why it worked before at all I have no idea.

EDIT: It's the emitted vertex shader that is wrong. Comparing with vulkan and dx12 shows it was supposed to sample vertex 0 always.

@kd-11
Copy link
Contributor

kd-11 commented Jul 11, 2016

Found the issue here https://github.com/RPCS3/rpcs3/pull/1865/files#diff-b7984c633ee70a99b6c5ade52375b28dL251

Other backends ignore this flag and mark all register access as non_array, while the current rsx decompiler still relies on this behavior. I'll submit a fix for this soon by adding a is_register field to the raw program state

@raven02
Copy link
Contributor

raven02 commented Jul 11, 2016

Nice catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants