Repository navigation
fix(agc): preserve array slice XOR in color target views - #4551
Conversation
|
I checked the overlap with my #4318. The runtime fixes are independent, but both extend I propose #4551 first, #4318 afterward. I will mark #4318 as depending on #4551 for fixture sequencing and rebase its regression to keep your R_X collapsed-slice 1/2/3 checks plus our R_X/D_X full-array transport checks. I have already rebased #4318's unchanged three commits onto |
|
Yes, that order works: #4551 first, then #4318. The runtime changes are independent; the dependency is for sequencing the shared Please keep the R_X collapsed-slice 1/2/3 checks alongside your R_X/D_X full-array transport checks when rebasing. #4551 has passed the native array-target and graphics tests on Windows/RTX 5070. Its |
|
Small conventions catch — full suite is green locally (957/957, Linux/RADV), but At byte 191725, |
4176ac7 to
ff4b7d9
Compare
|
Tested on Linux, RX 7600 (RADV NAVI33, Mesa 25.2.8) with Disney Epic Mickey: Rebrushed (PPSA15657). It's the same bug as Lost Soul Aside's, and this PR fixes it there too. Root cause, found independently in Mickey before I saw this PR:
With this PR's
My local fix took a different route: it gave such a view One problem in the diff: I haven't run |
|
Follow-up: the regression tests on Linux, RX 7600 (RADV NAVI33, Mesa 25.2.8), at head
|
|
|
ff4b7d9 to
428a7b5
Compare
|
Restored the partial rect-list, nullDescriptor placeholder image and shader float-mode entries byte-for-byte from main. The documentation diff now contains only the new collapsed color array views entry. Conventions and whitespace checks pass; runtime code is unchanged. |
|
Rebased onto main |
428a7b5 to
6157669
Compare
|
Checked the slice-XOR fold against the swizzle equations used by the detile shader ( |
6157669 to
13818ef
Compare
|
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.
|
…_SHADER APS5_SKIP_SHADER=<code address, hex> drops every draw that runs a program at that address and every dispatch of a compute program at that address, as if the packet had run. Skipping one pass at a time shows which pass first writes a wrong value into an image: Epic Mickey's (PPSA15657) green sky light was bisected this way to the copy into its sky cube (#4551). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(agc): handle the fill kernel that reads its pattern from a buffer The 16-byte fill kernel has a variant that loads its pattern with s_buffer_load_dwordx4 through the V# in s[4:7] instead of taking it from the user SGPRs. A title clears its UI canvas HTILE with it every frame; run as a shader, the depth surface never cleared and every UI draw failed its GEQUAL depth test, leaving the screen black. (cherry picked from commit 3bc3e35) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): sample non-sRGB textures through FORCE_DEGAMMA samplers as sRGB views A sampler with FORCE_DEGAMMA decodes the texels of a non-sRGB 8-bit texture as sRGB. Such a binding was refused; it now samples an sRGB view of the same image. Snapshot images whose format has an sRGB twin (R8, R8G8, RGBA8, BGRA8, ABGR8 packed, BC1/2/3/7 UNORM) are created with MUTABLE_FORMAT, and storage images are already mutable. The view is made on first use for the full, first-layer or base-level range, with sampled usage only and the descriptor's minimum LOD. A format without an sRGB twin is still refused. (cherry picked from commit ff3eae6) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): skip the draws and dispatches of one shader with APS5_SKIP_SHADER APS5_SKIP_SHADER=<code address, hex> drops every draw that runs a program at that address and every dispatch of a compute program at that address, as if the packet had run. Skipping one pass at a time shows which pass first writes a wrong value into an image: One title's green sky light was bisected this way to the copy into its sky cube (boykopovar#4551). (cherry picked from commit 0900586) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): release cached address space on device teardown (cherry picked from commit abc997b) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): give R8 and R8G8 no sRGB twin where the device decodes them in the shader SrgbTwinFormat mapped R8_UNORM and R8G8_UNORM to R8_SRGB and R8G8_SRGB without asking the device. Where 8_SRGB or 8_8_SRGB is decoded in the shader (context.srgbDecodeFormats: the device cannot sample the sRGB format, or APS5_SRGB_SHADER_DECODE=1), the forced view would have been created with a format the device cannot sample. Those two formats now get no twin there, so a FORCE_DEGAMMA sampler on them keeps the refusal. The image is no longer created MUTABLE_FORMAT either. (cherry picked from commit 12d07b8) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): drop the comment in SrgbTwinFormat (cherry picked from commit a141b8e) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): allocate CPU snapshots on demand (cherry picked from commit e2ee943) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): avoid snapshot upload PR conflict (cherry picked from commit 65eb466) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): register snapshot fixture GPU memory (cherry picked from commit a296b9c) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): use storage factory in snapshot test (cherry picked from commit 3f31403) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): avoid storage snapshot overlap Create uploads before publishing snapshot validity. Keep allocation edits separate from the per-unit validity changes in boykopovar#5024. (cherry picked from commit 0612c18) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): skip an indexed draw with no indices or instances (cherry picked from commit 0ba462f) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): skip an indexed draw with no indices or instances (cherry picked from commit af19dbf) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * perf(agc): cache a draw key the second time it is seen A draw whose key has no entry captured its stages, built a variant per stage (a copy of every captured region) and inserted a draw entry. A key that never comes back paid for the variants and the entry for nothing, and its entry pushed a live one out of the cache. Draws whose user data changes every frame are such keys. The driver now remembers the keys it has seen once, in a table of four-way buckets holding four times the cache's size in keys. A key gets its entry at its second sight; a key that already has an entry still takes new variants at once. A full bucket replaces its oldest key, so keys that share a bucket do not push each other out until more than four of them come between two sights of one. Test: agc_driver_draw_cache_reuse draws three times. The first two draws find no entry, one entry is inserted, and the third draw hits it. A sighting table of one bucket keeps two keys drawn in turn and replaces its oldest key when full. (cherry picked from commit 0adbdcf) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): decode GS_FAST_LAUNCH geometry with one point per subgroup VGT_SHADER_STAGES_EN.GS_FAST_LAUNCH = 1 (mesh-style NGG, as RADV programs it on gfx10.3) was refused. With a point list and GE_CNTL groups of one point, it launches one subgroup per input point and the first thread reads that point's index in the ES vertex id VGPR, which the mesh prologue already provides. The subgroup's primitive limit comes from GE_NGG_SUBGRP_CNTL.PRIM_AMP_FACTOR, and the GS and ES VGPR counts are not required to be full. Fast launch 2, other group sizes or inputs, and indexed fast-launch draws still throw. (cherry picked from commit 7370aaf) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): start the sample location registers at their clear-state value A draw with PA_SC_AA_CONFIG.MSAA_NUM_SAMPLES > 0 whose context never wrote PA_SC_AA_SAMPLE_LOCS_PIXEL_* (context 0x2fe-0x30d) was rejected with "missing register in context bank at DWORD 0x2fe", because the registers were absent from the initial context (a title stops on it after two frames). The MSAA_ENABLE single-sample path read them the same way. The sixteen registers now start at 0, both initially and after CLEAR_STATE. That is the value in libSceAgc's own register defaults (sceAgcGetRegisterDefaults2, every public table that holds context registers, the tables the initial context already follows for PA_SU_VTX_CNTL and PA_SC_CONSERVATIVE_RASTERIZATION_CNTL), in amdgpu's GFX10 clear-state table (clearstate_gfx10.h, gfx10_SECT_CONTEXT_def_8, loaded by gfx_v10_0_cp_gfx_start before CLEAR_STATE) and the GC 10.3 register reset value (gc_10_3_0_default.h). RADV, radeonsi and PAL always write the locations themselves and do not settle the unwritten value. 0 puts every sample at the pixel center. That is not the standard pattern for any sample count, so such a draw decodes as custom locations at the center, as a single-sample scan into a multisampled target already does. (cherry picked from commit 15955c0) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): make host imports over writable pages on macOS (cherry picked from commit 3960519) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): keep fused geometry programs out of the caller's memory sceAgcFuseShaderHalves copied both halves' code, the fused header and its tables into the caller's memory and registered the program at that address. A title fuses different halves into the same memory a few calls apart, while draws that use the earlier result are still queued. The registry keeps one program per code address, so those draws were compiled from the later program: its code read a user-data word the bound layout does not have, and the device reported "BDA access failed" for the mesh stage. The PS5 5.02 module writes only the back half's register list into that memory (sceAgcGetFusedShaderSize reports eight bytes per register, alignment 4) and leaves the code where the halves are. The fused code, header and tables now live in memory libSceAgc owns, one program per distinct pair of halves, registered once. The caller's memory receives the fused register list and the size query reports that list. A second fusion of the same halves returns the same program; halves whose contents changed get a new one. The driver looked a header up by comparing its fields with the registered header, including the register list's address. A header whose register list is a copy elsewhere with the same contents now matches too, which is what the title passes after a fusion. (cherry picked from commit d6f67d1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(libSceAgcDriver): skip CMASK decode on single-sample views with viewMip > 0 (cherry picked from commit acbc573) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(libSceAgc): ignore address argument on non-sample-dump events in sceAgcDcbEventWrite (cherry picked from commit fbf619e) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): execute INDEX_ATTRIBUTES_INDIRECT (PM4 0x91) The driver had no handler for the PFP INDEX_ATTRIBUTES_INDIRECT packet, so a submission with it threw "opcode is not known in the reference". The packet now reads the 16-byte record at attribute_base + attribute_index * 16 when it executes and loads INDEX_BASE, INDEX_BUFFER_SIZE and INDEX_TYPE from it as the direct packets do. The packet encoding follows PAL's BuildIndexAttributesIndirect and the record layout PAL's IndexAttribIndirect (gfx9Chip.h, c5e80007). indirect_mode 1, reserved bits and record index types above 3 throw. (cherry picked from commit dd3c13f) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): keep stores after occlusion dumps after them in a queued run A queued store run records each group's plain stores first and its occlusion counter dumps after them. A store started a new group only when it overlapped an earlier dump, so a label the title writes after its dumps was recorded before them. A title that polls that label then reads counters from before the dumps land. Any store after a dump now starts a new group. A title read stale or zero occlusion results every other frame and left visible geometry undrawn on alternate frames. (cherry picked from commit fce8aa8) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): run the queued write group tests in ctest (cherry picked from commit 9a1ac6a) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): describe which layers of an array surface a texture view reads (cherry picked from commit 7199916) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): sample a slice-0 array color target as a 2D texture through its image (cherry picked from commit faef399) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): flush the slice-0 draw before clearing the cached textures SampleSliceZero cleared the cached textures while its draw's results were still pending, so the storage image was torn down with them, which ClearCachedTextures only expects at device teardown. On some runs the next Run then read the first pass's slice 0 after filling it with the initial value (3 of 20 on an RTX 4070, reported on the PR). It now flushes and waits first, as Run does before its own clear. (cherry picked from commit 7262b10) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * docs(agc): record INDEX_ATTRIBUTES_INDIRECT in TechnicalDebt.md Upstream 23eb459 put its debt note in docs/dev/debt/libSceAgcDriver.md; the fork keeps the AGC driver notes in docs/dev/TechnicalDebt.md, so the note moves there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * test(agc): check the sync 9 shared decode fixes on the Metal path anyps5_metal_render_state_tests builds the State.cpp, QueueState.hpp and Pm4.cpp the Metal backend runs. Sync9SharedDecode checks three of this batch's shared fixes there: the sample location registers start at 0, so a 4x draw that never wrote them decodes as custom locations instead of throwing for a missing register; a single-sample view of mip 1 of a fast-clear target decodes no CMASK, while mip 0 of a mipmapped target is still refused; and INDEX_ATTRIBUTES_INDIRECT loads the index base, size and type from its record and refuses an index type above 3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * fix(agc): refuse indexed fast-launch mesh draws on Metal The shared State decode now accepts VGT_SHADER_STAGES_EN.GS_FAST_LAUNCH 1 with one point per subgroup (upstream 7370aaf), and upstream's Vulkan Draw refuses such a draw when it is indexed. The Metal mesh path takes the same decoded state, so it refuses indexed fast-launch draws the same way instead of running them through the indexed mesh prologue. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj * feat(agc): sample non-sRGB textures through FORCE_DEGAMMA samplers as sRGB views on Metal Upstream ff3eae6 samples a texture whose format has an sRGB twin through an sRGB view of the same image when a sampler forces sRGB decoding (S# FORCE_DEGAMMA), and refuses only formats without one. The Metal binding refused every non-sRGB format. It now does the same as the Vulkan driver: the bound view (sampled or resident) is re-viewed with the sRGB twin of its pixel format (R8, RG8, RGBA8, BGRA8, BC1, BC2, BC3, BC7), keeping its levels, slices and swizzle; a format without a twin throws "which is not sRGB and has no sRGB view". Test: anyps5_metal_driver_semantics_imagesampleforcedegamma (the upstream execution test run on the Metal driver) fails on the previous commit with "guest format 56, which is not sRGB, ... not implemented" and passes here: 8_8_8_8_UNORM through FORCE_DEGAMMA samples the decoded values of 8_8_8_8_SRGB, point and bilinear, and 32_FLOAT is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj --------- Co-authored-by: marcio-ch <marcio.zacarias@conniehealth.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Alex Collini <pipodi93@gmail.com> Co-authored-by: john.maia <johnmaia@gmail.com> Co-authored-by: MarcZX100 <marczxrr@gmail.com> Co-authored-by: Sanjaysrinivas <sanjaysrinivas47@gmail.com> Co-authored-by: mrfelek <17274927+mrfelek@users.noreply.github.com> Co-authored-by: Dean Galvin <dean.galvin@live.com> Co-authored-by: Lexroid <x.to@hotmail.com> Co-authored-by: fdfawef324 <287205748+fdfawef324@users.noreply.github.com> Co-authored-by: Zheorgyan <zheorgyan@gmail.com> Co-authored-by: fma-rc <188743521+fma-rc@users.noreply.github.com> Co-authored-by: pushkarreddyy <212133856+pushkarreddyy@users.noreply.github.com> Co-authored-by: Yossi Yadgar <yadgar360@gmail.com>
What
Collapsed color array views move the surface address to the selected slice but lose that slice's contribution to the XOR swizzle equation. A draw can look correct in its separate 2D render image while publication and a subsequent array read place its texels at the wrong addresses. Lost Soul Aside's reflection cube then reads green clear values from packed mip tails, producing flashing green lighting.
Fold the selected slice's equation bits into
pipeBankXorwhenCB_COLOR_ATTRIB3represents the view as a shifted 2D surface. Full-array and volume views retain their existing addressing. The regression extends the existing R32 R_X native test to collapsed slices 1, 2 and 3, checking the array read, untouched neighbouring slices, publication and re-upload.Addresses #4387. Main now includes #4318 (D_X color support) and #4328 (nonresident packed tails); neither implements the collapsed-view slice-XOR correction. The rebased fixture retains both R_X/D_X full-array transport cases and adds R_X collapsed slices 1/2/3. This change uses the existing swizzle equations and adds no shader instruction semantics.
Tested
Windows 11 x64, WinLibs GCC 15.2, Release, NVIDIA RTX 5070; isolated branch rebased onto main
fc1f063f5, emulator SPIRV-Tools disabled.fc1f063f5.agc_driver_array_layer_targetandagc_driver_graphicspass without skips in 5.40 s, retaining full-array and collapsed-slice 1/2/3 coverage. Conventions and whitespace checks pass. The earlier negative control failed on slice 1 without this fix; it was not repeated for this rebase.No game run was repeated for this rebase. The debt diff contains only the collapsed-color-array entry; all other entries match this main. The debt entry follows the new layout in
docs/dev/debt/libSceAgcDriver.md. No game shaders, requests, assets, capture hooks or trace files are committed.Checklist
main; no other open PR implements the same functions