Repository navigation
fix(agc): alpha-to-coverage from a 32_AR MRT0 export without a color target - #5521
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
The mode == 3u branch remaps only source components 0 and 1 to raw R and A, but the revised exported mask now permits exp.en bits 2 and 3 too. For a valid 32_AR export with EXP_EN=0xF (or 0xC), those source components are never copied into raw; after the export mapping the corresponding channels become zero/default, and in particular an alpha supplied in source component 3 is lost because A is taken from component 1. The new depth-only fixture appears to use only EXP_EN=0x3, so it would not catch this. Could you add a focused mask 0xF regression and either preserve the enabled channels consistently with the 32_AR semantics or reject unsupported enable masks?
6495158 to
269d806
Compare
|
Thanks, you're right, and the overlap bot points at the same line: #5110 already fixes the I added the |
|
Thanks, that addresses my specific concern. The new |
|
Windows test of #5521: Windows 10 (19045), MinGW-w64 GCC 15.2, Release, AI-assisted (Claude Code). |
A pixel export to an MRT whose SPI_SHADER_COL_FORMAT is 32_AR read its alpha from export channel 3 (EN mask 0x9), the layout before GFX10. On GFX10, which the PS5's gfx1013 is, the alpha is channel 1: Mesa's emit_ps_color_export (ac_nir_lower_ps_late.c, index gfx_level >= GFX10 ? 1 : 3), ACO and radeonsi in Mesa 22.0 (enabled_channels 0x3 with the alpha in channel 1) and LLPC's FragmentColorExport.cpp (EXP_FORMAT_32_AR, exports[1]) all emit it there. A console shader's `exp mrt0 v0, v1, off, off` therefore lost its alpha: it stayed at the 1.0 default, so an alpha-only COLOR_32 ALT_REV target stored 1.0 and an R32 target blended with SRC_ALPHA blended as if the alpha were 1. With channel 3 enabled, channel 3 was taken instead. ExportVector now reads channels 0 and 1 for 32_AR and puts channel 1 in the alpha component; channels 2 and 3 are ignored. The layout comes from the compilers, not from a hardware measurement, and is recorded in docs/dev/debt/shader.md. agc_driver_pixel32_ar_export draws 32_AR exports with EN 0x3, 0xb and 0xf, as wave32 and wave64, into a COLOR_32 FLOAT ALT_REV target and a COLOR_32 FLOAT target blended with SRC_ALPHA and ONE_MINUS_SRC_ALPHA, and compares every stored value bit for bit. Without the fix every row fails: the stored value uses an alpha of 1.0 or of channel 3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… color target radeonsi exports MRT0 as 32_AR when alpha-to-coverage is on and no color buffer is bound, so a depth-only draw can depend on that alpha. The decoder now accepts 32_AR and an absent MRT0 target, and reads the MRT0 export format from SPI_SHADER_COL_FORMAT, which it masked to 0 for draws without a color target. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
269d806 to
c8662b6
Compare
|
Rebased on current |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
|
What
Undithered alpha-to-coverage (
DB_ALPHA_TO_MASK = 0xaa01) was refused on draws without a color target on MRT0, and on32_ARMRT0 exports. Those are the draws radeonsi produces for alpha-to-coverage without a color buffer: the color format of MRT0 becomesSPI_SHADER_32_ARso that the alpha still reaches the DB. PPSA12361 (Transport Fever 2: Console Edition) issues them when the camera zooms in on a map and stopped withalpha-to-coverage without a written MRT0 color target is unsupported.DecodeStateaccepts alpha-to-coverage when no color target is written on MRT0, and accepts the32_ARexport format.SPI_SHADER_COL_FORMAT. The decoder masked the export formats to 0 for draws without a color target, so such a draw was refused asformat 0even with a32_ARexport.Which channel of a
32_ARexport carries the alpha is #5110's change, which this PR depends on: its first commit here is #5110 unchanged. Before #5110 the recompiler took the alpha from channel 3 and the coverage of these draws ignored it. Dithered alpha-to-coverage (#4451) is unrelated: this PR does not change whichDB_ALPHA_TO_MASKvalues are accepted.Tested
Windows 11, MinGW-w64 GCC 15.2.0, NVIDIA GeForce MX450 (driver 581.83).
agc_driver_alpha_to_coverage_tests: newDepthOnlyCoveragedraws a depth-only triangle with alpha-to-coverage and a32_ARexport over a cleared depth surface, then counts the pixels a depth test still passes. With export mask0x3, alpha 0.75 writes the depth everywhere and alpha 0.25 writes none. With export mask0xF, alpha 0.25 in channel 1 and 0.75 in channel 3, it writes none: the coverage follows channel 1 and channel 3 is ignored. All alpha-to-coverage tests pass.agc_driver_graphics_tests:32_ARis accepted with a color target; withCB_TARGET_MASK0 and a32_ARexport the state decodes with alpha-to-coverage and no target and passes the precheck; formats 1 and 2, and no MRT0 export without a target, are refused. All graphics tests pass.agc_driver_pixel32_ar_export_testsfrom fix(shader): take the 32_AR export alpha from channel 1 as GFX10 does #5110 passes on this branch.mainat30f961266with fix(shader): take the 32_AR export alpha from channel 1 as GFX10 does #5110.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)🤖 Generated with Claude Code