Repository navigation
fix(agc): serve sampled DCC array views from their viewed slices - #9961
Conversation
|
The current conventions check is failing because the PR template still has |
|
Tested this change on Windows 11 with an NVIDIA GeForce RTX 5090 (driver 617.42) on AMD Ryzen 9 9950X3D. Test Results
Code review of the diff looks great. Narrowing sampled array windows to slice-swizzle boundaries and dropping DCC metadata when keys evaluate to Uncompressed avoids retaining multi-gigabyte uncut surfaces without compromising address calculation accuracy. AI-assisted: yes. Written with AI assistance and reviewed by me. |
|
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
The new test and reported hardware coverage are relevant: the description records a Windows main-vs-change negative control from another contributor, while your Linux RX 6700 XT run reports the focused graphics test passing under the Khronos layer. Local current-main conventions, test registration and diff checks pass. I could not run the GPU test in this environment.
Please refresh from current main before this can be considered ready: check_main_sync.py reports 1 ahead /259 behind, with ShaderResources.cpp and docs/dev/debt/libSceAgcDriver.md changed on both sides since merge base (clean merge). After refreshing, run the same focused agc_driver_graphics_tests on current main and the change on the same RX 6700 XT, with ctest -V, VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ANYPS5_NO_SHADER_CACHE=1, and ANYPS5_REQUIRE_VULKAN=1; compare for new VUIDs. The body currently reports the change-side validation run but does not give a matching current-main run or VUID delta. Keep the title-unblock status as unverified until an in-game A/B reaches the reported point. This is for the repository's current-main and AGC validation requirements, not queue size.
|
Windows run on NVIDIA and Intel, to go with the RADV one. Windows 11 Pro 10.0.26300, WinLibs GCC 15.2.0 r7, Release. NVIDIA GeForce RTX 3060 Ti (driver 617.42) and Intel Arc Graphics (Meteor Lake, driver 101.9150), selected with Tests: the 1230 of 2029 that
AI-assisted: yes (Claude Code) |
8b41f3b to
6d453d1
Compare
6b0dacf to
f7c5a43
Compare
|
Tested on AMD under Windows: Radeon RX 6600 XT (Navi 23, 0x73ff), AMD driver 32.0.21045.16010, Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, Vulkan SDK 1.4.363.
The new AI-assisted: yes |
f7c5a43 to
b166232
Compare
|
@MarcZX10 refreshed as requested: rebased onto current Same RX 6700 XT, One correction to the VUID evidence, including mine: The title-unblock claim stays unverified until an in-game A/B. Could you re-review, and could a maintainer allow the build/test jobs to run? On the previous head they were skipped. |
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
|
@MarcZX10 each point of your review is addressed on the current head |
What
Targets Demon’s Souls PPSA01341 (#1283); title-unblock status remains unverified pending an in-game A/B. On current
main,ViewedArrayWindowreturns the entire array whendccAddress != 0(TextureTiling.cpp:471). For the title’s sampled 1024x1024 format-22 view of slice 636 in a 637-slice Z_64KB_X array, the unreadable DCC keys mean texels are read as stored, but the snapshot still spans all 637 slices: 2,671,771,648 bytes.This fix starts an XOR-swizzled window at the last slice-swizzle boundary before the view, and cuts a DCC-bearing view only when whole-surface keys resolve to
Uncompressed. It drops DCC metadata on that window. The regression test decodes the title’s exact descriptor words and checks that the window covers slices 624..636 (54,525,952 bytes), preserves the Z_X texel addresses, and preserves the key decision. The descriptor itself is not modified.Tested
ctest -V -R '^agc_driver_graphics(_maps_fallback)?$'passed 2/2 on currentmainb4d8e8bb3b14a9ce97caeca6e4466b02836ee813and on PR headb166232c722f77e3616dbed5e24a579e8be7d3bf(rebased onto thatmain), same machine, withANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1, andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(Vulkan-ValidationLayers 1.4.363 on the layer path).agc_driver_graphics_testsdoes not link or load the Vulkan loader (nolibvulkaninldd; no loader output underVK_LOADER_DEBUG=layer), so the validation layer never attaches to these tests on either arm and the VUID comparison is 0 against 0 by construction, not evidence.main, a view at base slice 1530 keeps the full array; this fix windows from slice 1520. For the title’s DCC descriptor, the currentmainguard likewise keeps all 637 slices.Checklist
main; no other open PR implements this exact change