Repository navigation
feat(shader): combine alpha-to-coverage with a sample-mask export - #14909
oneandonlydean merged 1 commit into
Conversation
With alpha-to-coverage and DB_SHADER_CONTROL.MASK_EXPORT_ENABLE the pixel shader now writes SampleMask as the AND of its MRT0 alpha coverage and its MRTZ mask export instead of throwing, as Vulkan multisample coverage ANDs the shader sample mask with the alpha-to-coverage mask. Adds eight- and two-sample GPU tests and a shader debt entry for the rule, which is not measured on hardware.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
✅ NVIDIA GPU test: passedPR head
Intermittent validation messages (1, seen once in the parallel run, not reproduced when the test was re-run alone 2 times; not counted against this PR)Unstable on main (3, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (5 IDs in 484 tests)Skipped (45)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
BunnyFl
left a comment
There was a problem hiding this comment.
Here's the AMD run you didn't have, and it looks right to me.
ANDing the alpha coverage with the exported SampleMask follows the Vulkan coverage rule you cite. Treating an unwritten mask as all ones keeps the alpha-only result for invocations that don't export one.
Tested on Windows 11, WinLibs GCC 15.2.0, Release, AMD Radeon RX 9070 XT (RDNA4, driver 32.0.32015.7029), Vulkan SDK 1.4.363 validation layer, with ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, head 0004d35125 merged onto main:
ctest -R 'alpha_to_coverage|sample_mask|pixel_sample_coverage': 11 of 11 pass. That includes both new*_alpha_to_coverage_sample_mask_exportcases, the dithered and state tests,agc_driver_pixel_sample_coverageand the threeagc_driver_sample_mask_*tests. No log has aVUID-orSYNC-HAZARDline.- With
SpirvEmitterState.hpp,SpirvEmitterHelpers.cppandSpirvModuleEmitter.cppput back tomain's versions, the two new cases fail and the other existing tests still pass.
So with your RTX 4060 results, the combined rule now passes on both vendors. It still isn't measured against gfx1013 or a console, which your new shader.md entry already says.
|
AMD check, since the description notes no AMD GPU was available: Radeon RX 6700 XT (RDNA 2), AMD proprietary driver 26.10.1, Windows 11 Enterprise 10.0.22621, WinLibs GCC 15.2.0 UCRT r7, Release. Exact head
This is the AMD proprietary driver on RDNA 2. It is not RADV, and not a PS5. 🤖 Generated with Claude Code |
What
With alpha-to-coverage and
DB_SHADER_CONTROL.MASK_EXPORT_ENABLE, the pixel shader now writesSampleMaskas the AND of its MRT0 alpha coverage and its MRTZ sample-mask export instead of throwing. A mask the invocation does not write counts as all ones, so only the alpha coverage applies. Alpha-to-coverage without a mask export and the other alpha-to-coverage refusals are unchanged.Semantics source:
SampleMaskand the alpha-to-coverage mask into the coverage.MASK_EXPORT_ENABLEfrom the pixel shader's sample-mask output, independently of the alpha-to-coverage state (radv_shader.cL2108-L2112, Mesaae9e17fc684b).DB_SHADER_CONTROL.COVERAGE_TO_MASK_ENABLE(bit 7) is still not decoded, as onmain, so the alpha always comes from MRT0. RADV sets that bit below GFX11 only for alpha-to-one (radv_cmd_buffer.cL13211-L13215). The combined rule is not measured on hardware, so it gets a new entry indocs/dev/debt/shader.md, anda sample-mask exportis removed from the alpha-to-coverage refusals indocs/dev/debt/libSceAgcDriver.md.Changes:
SpirvEmitterState.hpp,SpirvEmitterHelpers.cppandSpirvModuleEmitter.cpp.AlphaToCoverage.cpp, registered asagc_driver_alpha_to_coverage_sample_mask_exportandagc_driver_two_sample_alpha_to_coverage_sample_mask_exportinagc_driver_alpha_to_coverage_tests.cmake. Both are also added to the WindowsPATH=path_list_prependtest list incore/libs/prx/libSceAgcDriver/CMakeLists.txt. Without that entry the test executable can't load its DLLs and the tests hit the 30 s timeout.check_conventions.pyreports one[debt-length]warning, on thelibSceAgcDriver.md:72alpha-to-coverage entry (2436 characters). Onmainthat entry is already 2458 characters, and this PR only removesa sample-mask export,from it. Shortening the rest would rewrite gaps outside this topic.Overlap:
open_prs.jsonandgh pr list --searchforalpha-to-coverage,sample mask export,alpha coverage sample maskandgl_SampleMask. feat(agc): implement measured single-sample dithered alpha coverage #14561 (single-sample dithered alpha coverage) changes the dithered coverage tables inSpirvModuleEmitter.cppandAlphaToCoverage.cpp, not the sample-mask export.pr_overlap.pycould compare, it lists one conflicting file and no shared exports or shared new files: feat(agc): implement measured single-sample dithered alpha coverage #14561 indocs/dev/debt/libSceAgcDriver.md(L72-73 onmain), described below. It failed closed on 81 open PRs that conflict withmain. I compared each of them by hand against0004d35125, usinggit merge-tree --write-treeand a search of their diffs in this PR's files for alpha-to-coverage and sample-mask terms. None of them changes this behaviour. The only matches: fix(shader): preserve raw fragment exports for integer attachments #6235 renamesuintOutputtointegerOutputon the line that callsStoreAlphaCoverage(no new conflict), and feat(agc): blend into 11_11_10 float color targets in the pixel shader #9020 and feat(agc): draw alpha-tested packed-target merges that write depth as a depth pass and an EQUAL merge pass #9335 reword the4_4_4_4 partial writesdebt entry.docs/dev/debt/libSceAgcDriver.md(0 to 1 block): this PR removesa sample-mask export,from the alpha-to-coverage entry on line 72, and feat(agc): implement measured single-sample dithered alpha coverage #14561 rewrites the single-sample dithered alpha-to-coverage entry on line 73. feat(agc): implement measured single-sample dithered alpha coverage #14561 merges cleanly withmain. Both PRs also changeSpirvModuleEmitter.cpp,AlphaToCoverage.cppandagc_driver_alpha_to_coverage_tests.cmake, and those merge cleanly. Whichever lands second keeps this PR's line 72 and feat(agc): implement measured single-sample dithered alpha coverage #14561's line 73.core/libs/prx/libSceAgcDriver/CMakeLists.txt(1 to 2 blocks): both PRs add test names to the same WindowsPATH=path_list_prependset_tests_propertiesline near line 670. Whichever lands second keeps both sets of names.docs/dev/debt/libSceAgcDriver.md(9 to 10 blocks) anddocs/dev/debt/shader.md(7 to 8 blocks): docs(debt): separate entries with blank lines and drop derived export totals #14212 puts blank lines between entries and already conflicts withmainin 9 and 7 blocks. The edited alpha-to-coverage entry and the newshader.mdentry add one block each.Tested
Windows 10.0.26300.9711, Git Bash, NVIDIA GeForce RTX 4060 (driver 32.0.16.1088),
VK_LAYER_KHRONOS_validation1.4.363 from the LunarG Vulkan SDK 1.4.363.0. There's no AMD GPU on this machine, so nothing was measured on AMD hardware or a PS5.Build:
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DBUILD_TESTING=ON, thencmake --build build --parallel 4. It exits 0 one2f2b44722and on this PR's commit0004d35125.Focused GPU tests, run for
mainand for the change on the same machine and GPU:mainat0d5e03be8c(the branch's original base): 9/9 passed.0d5e03be8c, before the rebase: 11/11 passed.main4ac683fe02(e2f2b44722): 11/11 passed, including the newagc_driver_alpha_to_coverage_sample_mask_export(1.65 s) andagc_driver_two_sample_alpha_to_coverage_sample_mask_export(1.73 s). Every test took under 3 s with no shader cache.0004d35125, the same change rebased ontomain66e1f7f993: 11/11 passed, including the two new tests (1.83 s and 1.84 s). Every test took under 3 s with no shader cache.VUID-lines and 0was not found but was requested by env var VK_INSTANCE_LAYERSlines.1dde962ec3differs frome2f2b44722only in shorter wording of the newdocs/dev/debt/shader.mdentry.0004d35125is1dde962ec3rebased onto66e1f7f993with no conflicts and an unchanged patch.git diff --check origin/main HEADon0004d35125: clean.python tools/check_conventions.py --base origin/mainon0004d35125: exit 0, with 1 finding and 0 errors. The finding is the[debt-length]warning described under What.python tools/pr_overlap.py --branch feat/shader-alpha-to-coverage-sample-maskon0004d35125: exit 2 (overlap check failed closed). It confirms the feat(agc): implement measured single-sample dithered alpha coverage #14561 conflict described under What, and I compared the 81 PRs it couldn't check by hand against0004d35125.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)