Repository navigation
Conversation
95fc84a to
9e349dd
Compare
9e349dd to
183f2d5
Compare
|
Thanks for the detailed review @hckrman101! 1. Rebase & HKSTATE2 (
|
Joey-Magnus
left a comment
There was a problem hiding this comment.
This conflicts with main in gpu/vk/vk_texture.cpp, and the code it changes has been reworked. #2327 (device loss rendering to 3D BC slices) took 3D BC images off the raw-attachment path: they now render through a separate uncompressed mip, and the usage block at vk_texture.cpp:2043 is x.type != VK_IMAGE_TYPE_3D only. So the "3D: taken" half of this diff no longer applies.
Two points from the earlier review are still open after the rebase. First, || x.type == VK_IMAGE_TYPE_2D gives every 2D BC image the attachment usage on every vendor, whatever vkGetPhysicalDeviceImageFormatProperties says. That's outside the spec, and it changes NVIDIA and Intel too, not only amdvlk. Please scope it to the AMD driver where you saw -11, or show a validation-clean run. Second, the view_format_of change at line 159 attaches a BC-format view as R32G32B32A32_UINT, which the fragment shader writes float outputs into. The screenshot shows the crystals change, but not that the write is defined. A RenderDoc capture of the Dodongo's Cavern pass would settle it. The issue is now tracked in #1006, and it's listed as a regression since v2.0.12, so it's worth checking which commit caused it.
…s on AMD On AMD drivers (amdvlk), vkGetPhysicalDeviceImageFormatProperties returns VK_ERROR_FORMAT_NOT_SUPPORTED (-11) when querying compressed formats with VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT, because the driver checks the compressed format directly rather than raw block-texel views. But with EXTENDED_USAGE, the driver creates the image and its raw views without error. Scope the usage override to AMD (dev.props.vendorID == 0x1002) on 2D BC images when the raw format supports it, relying on the existing vkCreateImage refusal fallback. (3D BC attachments are handled via separate uncompressed mips per NotRexed#2327). In addition, map BC formats under ViewUse::Attachment in view_format_of() to their raw block-texel format (storage_view_format). Previously this returned VK_FORMAT_UNDEFINED, causing texture_view() to return VK_NULL_HANDLE. When drawing into BC targets, targets() skipped the attachment, leaving the render pass unexecuted and the texture holding uninitialized VRAM memory (rainbow diamond artifacts on Dodongo's Cavern crystals, NotRexed#1801). With HKSTATE2, attachment_format() also uses view_format_of(), ensuring prebuilt pipeline formats match what targets() binds.
183f2d5 to
9b05cb6
Compare
|
Rebased onto the latest \upstream/main\ (\ce53453) and addressed both points from @Joey-Magnus:
Passes \ est_bc_raw_targets.py\ cleanly. Ready for another look @hckrman101! |
Joey-Magnus
left a comment
There was a problem hiding this comment.
Thanks for scoping the usage override. The rebase onto #2327 is clean: the 3D path is untouched. I merged this into current main and ran the texture, BC, view and pipeline-state checks. All pass except nx2/test_bc_storage_views, which fails the same way on main on Linux.
Two points still stand:
gpu/vk/vk_texture.cpp:159:view_format_ofnow turns every BC-format attachment view intoR32G32B32A32_UINT/R32G32_UINTon every vendor, not just AMD.attachment_format()uses the same function, so the pipeline state list changes for everyone too. The fragment shader still writes float outputs into a UINT attachment, which the spec leaves undefined. The screenshot shows the crystals change, but it doesn't show that the write is defined. Please scope this to the case you saw, or show a RenderDoc capture of the pass.vk_texture.cpp:2064:vendorID == 0x1002covers RADV, so the Steam Deck too, as well as AMD's Windows driver, not only amdvlk as the comment says. Has the -11 been seen on RADV? If not, please gate ondriverIDand run once with validation on.
#1006 lists this as a regression since v2.0.12. Finding the commit that caused it may give a smaller fix.

Resolves the issue where 2D block-compressed textures (such as BC6H/BC5 crystal emissive lights in Dodongo's Cavern) fail to receive colour attachment usage or valid views on AMD Windows drivers, resulting in skipped render passes and unwritten multicoloured/rainbow block artifacts. Closes #1801.
Why this is needed
== VK_SUCCESS\ for 2D images, \x.usage |= VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT\ was not set.
Solution
Passes \ ests/test_bc_raw_targets.py\ cleanly. Tested and confirmed on AMD Radeon RX 6600 XT.