Repository navigation
fix(agc): serve views of arrays past maxImageArrayLayers from their viewed slices - #6772
Conversation
80d33f9 to
dfeb3d8
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.
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
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the storage-array windowing path. The code limits the requested slice window to maxImageArrayLayers, derives its guest base from the original array geometry, and rejects thick/non-array and slice-dependent XOR cases before creating the view. The layout regression checks the rebased address, slice count and mip offset, and the validation report identifies the original oversized Vulkan image. I found no blocking issue.
|
@XairRrR thanks for this. One regression: storage views of 3D volumes deeper than 2048 texels now throw. They worked before this PR. Cause: Minimal fix: let volumes pass through unchanged. core/libs/prx/libSceAgcDriver/Graphics/src/TextureTiling.cpp:439 if (viewed.depthOrLastArray < maxArrayLayers || viewed.dimension == TextureDimension::k3D) return viewed;core/libs/prx/libSceAgcDriver/tests/TextureTiling.cpp:408 Require(StorageArrayWindow(volume, 2048).baseAddress == volume.baseAddress && StorageArrayWindow(volume, 2048).depthOrLastArray == volume.depthOrLastArray, "a volume is not an array");Repro: I copied tests/execution/ImageStoreArrayThroughVolume.cpp and changed
Everything else checked out:
I did not check whether any shipped title binds a 3D storage volume this deep. The validation layer isn't installed here, so I make no VUID claims, including for the 2D-array view of the 2049-deep image on the merge-base files. Setup: Linux, lavapipe Mesa 26.2.4 (LLVM 23.1.1), AI-assisted: yes |
|
@Dextroid17 thanks, you're right: a volume's
AI-assisted: yes |
|
Intel and AMD Windows data, from GPUs on both sides of the limit. Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built, Reported
There is no without-run: this PR's Full CTest ( AI-assisted: yes (Claude Code) |
XairRrR
changed the title
|
Pushed a95b44a: sampled views now use the same layer window as storage views ( While testing the earlier head on Uncharted: Legacy of Thieves Collection (PPSA05684), the main menu lost about a third of its frame rate. It samples a 16x16 array of up to ~8000 layers through image-table views of 2689 to 8065 slices. The sampled lookup read and snapshotted the whole surface (~8 MB) before the AI-assisted: yes |
github-actions
Bot
added
unblocks title
a95b44a to
5a04013
Compare
|
Rebased onto current AI-assisted: yes |
5a04013 to
04bc0ee
Compare
… their viewed slices A PS5 array may hold up to 8192 layers; Vulkan devices allow fewer (2048 on NVIDIA). A storage view of such an array created its whole image, with more layers than the device allows: vkCreateImage is then invalid (VUID-VkImageCreateInfo-arrayLayers-02256) and a store into a layer the device did not back faults the GPU. Uncharted: Legacy of Thieves Collection (PPSA05684) views a 16x16 RGBA8 array of up to 7809 layers in slices of a few hundred to 2048 in its orphanage chapter, which lost the device (Xid 109, write invalid) a few minutes in. StorageArrayWindow describes the viewed slices as an array of their own starting at the first viewed slice when the whole array is past the limit; a non-XOR swizzle does not depend on the slice index, so the slices detile alike. Views whose slices alone exceed the limit, XOR swizzles that mix in the slice, and any other image past the limit now throw instead of creating an invalid image. AI-assisted: yes
A volume's depthOrLastArray is its depth, bounded by maxImageDimension3D, not by maxImageArrayLayers: a 2049 to 4096 deep volume is a valid image and threw since the window was introduced. Reported by Dextroid17. AI-assisted: yes
… their viewed slices The sampled lookup kept the whole array's layer count: a texture over an array of more than maxImageArrayLayers layers read and snapshotted the whole surface and then threw in the Texture constructor, on every lookup, even when the descriptor viewed a few slices. A menu that samples a 16x16 array of up to 8065 layers through its image tables spent ~55 ms per frame reading those surfaces only to reject them. cachedTexture now applies the window the storage path uses (StorageArrayWindow, renamed ArrayLayerWindow since both paths use it): a view whose slices fit the limit is served as an array of its own starting at its first slice, and a view whose slices alone exceed the limit throws before anything is read. A 2D image is one layer whatever its last array field holds and is no longer refused by the storage path. agc_driver_image_array_window_past_layer_limit samples and loads three slices at 8100-8102 of an 8103-layer 1D array (before: "a texture array with more layers than maxImageArrayLayers is not implemented") and checks that a view of all its layers is refused with the window's error on a device whose limit is lower. AI-assisted: yes
Texture and storage images now refuse more array layers than the device's maxImageArrayLayers. The mock context left that limit at 0, so agc_depth_copy_kept_bytes failed creating its 8x8 storage image. Give it the 2048 layers real devices report. AI-assisted: yes
04bc0ee to
f408eb7
Compare
|
Rebased onto AI-assisted: yes |
|
Re-checked after the rebase onto |
|
AMD RX 9070 XT (RADV GFX1201), Mesa 26.2.4, CachyOS, kernel 7.2.9. @adriadam10's pre-merge recheck above was green — on which stack? If that was a different GPU or Mesa, this may be NAVI4x- or Mesa-version-specific, in the pattern of #7766/#7784 (which pass on AMD's Windows proprietary driver and fail on RADV here). For transparency on the diagnosis path: the test did not exist on |
WinLibs GCC 15.2.0 rejects the alignas(65536) static array that agc_driver_image_array_window_past_layer_limit added in #6772 (requested alignment '65536' exceeds object file maximum 8192), which stops a Windows build with every AGC execution test. Take a 64 KiB aligned 4 MiB span of a larger static array instead, as #6452 did for the indirect arguments block, and check its alignment at start.
WinLibs GCC 15.2.0 rejects the alignas(65536) static array that agc_driver_image_array_window_past_layer_limit added in boykopovar#6772 (requested alignment '65536' exceeds object file maximum 8192), which stops a Windows build with every AGC execution test. Take a 64 KiB aligned 4 MiB span of a larger static array instead, as boykopovar#6452 did for the indirect arguments block, and check its alignment at start. (cherry picked from commit ca55e46)
What
A PS5 texture array may hold up to 8192 layers; Vulkan devices allow fewer (
maxImageArrayLayersis 2048 on NVIDIA). A storage view of such an array created the whole array's image anyway:vkCreateImagewith more layers than the device allows is invalid (VUID-VkImageCreateInfo-arrayLayers-02256), and a shader store into a layer the device did not back faults the GPU.Uncharted: Legacy of Thieves Collection (PPSA05684, #4001) binds a 16x16
R8G8B8A8_UNORMarray (kStandard64KB, 2 mips) of up to 7809 layers as a storage image, in windows of a few hundred to 2048 slices (for example layers 1536..3200, created as a 3201-layer image). A few minutes into its orphanage chapter the device was lost:Xid 109 CTX SWITCH TIMEOUT, thenVK_EXT_device_faultreportedwrite invalidat a page-aligned address. The validation layer named the cause (arrayLayers (3201) exceeds the maxArrayLayers (2048)).ArrayLayerWindow(TextureTiling.cpp) describes the viewed slices[baseArray, depthOrLastArray]as an array of their own, starting at the first viewed slice, when the whole array is past the limit. A non-XOR swizzle does not depend on the slice index, so those slices detile exactly as in the whole array.cachedStorageTextureuses it for every storage view andcachedTexturefor every sampled view; a 3D volume is not an array and a 2D image is one layer whatever its last array field holds, so both pass unchanged. A view whose slices alone exceed the limit, a view of an XOR swizzle whose equation mixes in the slice, and any otherTextureorStorageTextureimage past the limit throw instead of creating an invalid image. The sampled lookup throws before it reads the surface: the same menu samples that array through image-table T#s whose views span 2689 to 8065 slices, and reading and snapshotting the whole surface (~8 MB) before theTextureconstructor refused it cost ~55 ms per frame on the async queue.#6481 checks sampled image-table entries against the device's image limits before reading them; this serves the views that fit from their slices (sampled and storage) and refuses the rest at the lookup. If #6481 lands first, its layer check should see the windowed descriptor (
ArrayLayerWindow) so a fitting view of a larger array is not refused.Tested
agc_driver_graphics:ArrayLayerWindowrebases a 3201-layer view at slice 1536 to 1665 slices atbase + 1536 * layerByteswith the same slice layout, keeps an array within the limit, rebases a linear array, refuses a window wider than the limit, an XOR swizzle and a cube, and passes a volume deeper than the limit and a 2D image with a large last-array field unchanged.agc_driver_image_array_window_past_layer_limit: a compute shader samples (image_sample_lz) and loads (image_load) slices 8100-8102 of an 8103-layer 1D array and reads exactly those slices; before this change the sampled lookup threw "a texture array with more layers than maxImageArrayLayers is not implemented". A view of all 8103 layers is refused with the window's error on a device whose limit is lower.agc_driver_graphics,agc_driver_image_one_dimensional_array,agc_driver_image_gather_one_dimensional_array,agc_driver_image_store_array_through_volume,agc_driver_array_layer_target,agc_driver_layered_volume_draw,agc_driver_depth_plane_sampling,agc_driver_depth_plane_guest_layers; conventions and test registration checks pass.agc_tests pass exceptagc_driver_store_over_untracked, which fails on unchangedmainon this machine too.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)