libcg / grvk

Vulkan-based Mantle API implementation
https://en.wikipedia.org/wiki/Mantle_(API)
zlib License
221 stars 14 forks source link

mantle: add a quirk to handle incorrect use of GR_IMAGE_STATE_TARGET_AND_SHADER_READ_ONLY #53

Closed Cherser-s closed 1 year ago

Cherser-s commented 1 year ago

Battlefield 4 uses this state incorrectly when calculating global illumination, writing into stencil attachment, which causes lighting bugs.

libcg commented 1 year ago

The spec says:

"Read-only depth-stencil view allows rendering with read-only access to the depth or stencil aspect of an image while it is also used for read access from the graphics pipeline shaders. Only one of the depth or stencil aspects can be designated as read-only, but not both at the same time. The read-only depth in a view is indicated by the GR_DEPTH_STENCIL_VIEW_CREATE_READ_ONLY_DEPTH flag and the read-only stencil is indicated by the GR_DEPTH_STENCIL_VIEW_CREATE_READ_ONLY_STENCIL flag at depth-stencil creation time.

If the depth or stencil aspect is used for simultaneous read access as a depth-stencil target and as an image view from the graphic shaders, it has to be in the GR_IMAGE_STATE_TARGET_AND_SHADER_READ_ONLY image state. The image subresources in a read-only depth-stencil view that are read from shaders should be transitioned to that state, as well as this state should be used for binding image view and appropriate aspect for the depth-stencil target."

We're not handling this properly.

libcg commented 1 year ago

Should be fixed with: https://github.com/libcg/grvk/commit/cca3f54612977038aec3ecd18f203e3f592ceee8

Cherser-s commented 1 year ago

Closing this due to the fix, actually the layout can be still VK_IMAGE_LAYOUT_READ_ONLY_OPTIMAL_KHR (on RADV at least), because only access flags do matter in this case.