Repository navigation
feat(shader): sample converted unorm images in the shader - #5958
Conversation
There was a problem hiding this comment.
Routing these samples through the existing compare emulation is a sensible way to get exact-format decoding without a Vulkan equivalent. The test is thorough: it fails when the bilinear neighbours are swapped.
The cost section is the part I'd like quantified, since it affects every title, not only those with converted formats: "a sampled image whose format is unknown at registration now also carries the converted modes in its runtime-mode switch", which is one more decode-and-blend path per sample instruction. Specialization folds the branch away at pipeline creation, but the module still grows. The driver parses and optimizes the larger module before dead-code elimination removes the unused path, so pipeline compile time rises for every registered shader with such samples. Earlier PRs that added arms to hot shader paths (#4322) were closed for exactly this kind of compile-time increase.
Two numbers would settle it:
- the SPIR-V word count, and the
vkCreate*Pipelinestime for a few registered pixel shaders of a title that never uses 10_11_11 (any UE or Unity title), before and after; - or a cold-cache start of such a title with
APS5_TRACE_SECONDS, comparing the shader-compile phases.
If the growth is significant, could the converted modes be added only once a converted descriptor has been seen for that shader, for example by preparing it again at that point? The cost would then fall only on titles that use the format.
jotitadev
left a comment
There was a problem hiding this comment.
Run on an Apple GPU, since the PR was tested only on an RTX 3080.
GPUs tested: Apple M2 Max, MoltenVK 1.4.2 (Vulkan 1.4.357), macOS 27.2, x86-64 under Rosetta
main does not run the GPU tests on macOS yet, so I tested on macos-port at b7eb6b6b (mugurc/AnyPS5) plus the ImportWatched change from #1, with this PR's own diff (merge-base..head 8338defe5) applied. Not main. Release build, LunarG Vulkan SDK 1.4.363.0. Suite: ctest -R "^agc" -E "agc_display_buffer_dcc|agc_driver_flip|agc_driver_lds_atomics64" -j1 --timeout 30. Without the PR, 13 tests fail on this setup (7 with the #2704 build), all known macOS failures listed in #1.
The diff does not apply cleanly to that branch. Two hunks conflict:
ResourceMaterializer.cpp: the branch has neitherrejectImage/PoisonReasonnorImageResource::table.SpirvImageEmitter.cpp: the branch still has the bindless-slot check.
I adapted them: the same new checks, written as throw, with indirectRoot != NoIndirectImage in place of table != NoTable, and the slot check kept. Treat this as an approximation of the PR, not the PR itself.
agc_driver_image_converted_unormpasses. That includes the new point and bilinearimage_sample_lzchecks with clamp-to-edge and wrap, and both formats.- With only the new test on the unchanged branch, it fails with
samples or gathers a converted float image, or queries its LOD, which is not implemented. So the sample paths run on this device. - Suite: the same 13 failures with and without the PR. None new, none fixed.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Thanks for the Apple GPU run and for adapting the two hunks by hand. Good to see the point and bilinear |
|
Another data point from PPSA01317 on current main, Windows 11, RTX 5090 Laptop: this PR fixes its abort, There's one limit for later, not a request for this PR: after a few more fixes the same title samples a format-43 image (packed 11_11_10 float) as a cube and stops on AI-assisted: yes |
|
Heads-up: I opened #7271, which samples texture format 43 (10_11_11 float) natively. It copies the texels losslessly into a The two PRs touch the same test. If both merge, the bilinear AI-assisted: yes |
adriadam10
left a comment
There was a problem hiding this comment.
#7271 is merged, and it overlaps this PR:
- Format 43 (
R10_G11_B11_FLOAT) sampled images now go through a lossless B10G11R11 copy and the hardware sampler. InResourceMaterializer.cppthey arenativeRepacked(sampled, non-storage, non-packed) and no longer getconversionFormat, so the float half of the converted-sample emulation added here no longer runs for them. ImageConvertedUnorm.cppconflicts with main in three hunks. Main replaces the floatimage_sample_lzrefusal withCheckFloatSample(point sampling, bit-exact). The float bilinear and off-grid checks here model weights quantized toroundEven(frac * 256) / 256; the native sampler does not do that on every GPU, so they cannot be kept as they are.libSceAgcDriver/CMakeLists.txtalso conflicts (adjacenttarget_include_directorieslines, trivial).
Please rebase on main and decide the float part explicitly:
- Keep this PR to the 10_11_11 UNORM (format 30) path, which main still refuses, and drop the float emulation and its test runs; or
- Keep float only for the cases #7271 leaves converted (storage / packed images) and say which test covers them.
The UNORM path itself was already checked (1/256 weights match gfx1035) and needs no change. Re-run agc_driver_image_converted_unorm after the rebase, with the negative check (bilinear neighbours swapped / rounding removed) still failing.
10_11_11_UNORM (format 30) images are read and written through an R32_UINT view, so the hardware sampler cannot filter them. image_sample of a 2D or 2D array view now fetches the one or four texels at LOD 0, decodes them and blends them in the shader, with bilinear weights on the 8-bit grid, for point or bilinear filtering with wrap or clamp-to-edge addressing. A shader gets the converted sample mode only after a converted descriptor reaches one of its sampled images: that capture throws ConvertedSampleRequired and the driver prepares the program again with GuestContext::convertedSamples. The R10_G11_B11 float format is sampled natively since boykopovar#7271 and keeps its refusal on the converted path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
87a9b8d to
8545c23
Compare
|
@adriadam10 rebased on main (91ddd9e) as one commit, 8545c23, taking the first option: this PR now covers only the 10_11_11 UNORM (format 30) path, and the float emulation is gone.
RTX 3080 (NVIDIA 610.74), Windows 11, MinGW GCC 15.2, Release:
In the full CTest, AI-assisted: yes |
|
Tested this PR at
|
The data-hit verification keeps this branch's pending-span query on its ShaderMemory and takes #5958's CaptureInvocation, which retries a capture with converted samples.
What
image_sampleon a 10_11_11_UNORM (format 30) image threwsampling, gathering or querying LOD of a converted unorm image is not implemented. The format has no Vulkan equivalent, so it is bound through an R32_UINT view that cannot be filtered. God of War Ragnarök (PPSA08330) stopped on a converted-image sample in compute shader0x10527dd300, about 60 s in, after its MFSR setup (#5796) and the SINT D16 stores (#5899).These samples now go through the emulation that color comparisons already use (
EmitEmulatedCompareSample, renamedEmitEmulatedSample):One texel (point) or four texels (bilinear) are fetched at LOD 0 through the R32_UINT view.
Each texel is decoded and swizzled like
image_load(UnpackImageTexel), then blended. Bilinear weights are rounded to the hardware's 8-bit grid (roundEven(fraction * 256) / 256), as adriadam10 measured on gfx1035 in this PR.The sampler state (point or bilinear, wrap or clamp-to-edge) reaches the shader through the existing comparison specialization words, or through the runtime metadata for registered shaders.
EmulatedCompareStatenow fills it for these images (convertedSampleState), and the binding builder provides those words whenever the converted-sample mode is compiled in.The tracker sets two new bits in
emulatedCompare:ConvertedSampleUnsupportedfor gathers and LOD queries;ConvertedSampleRequiresSingleLevelfor samples that are not_lz.RuntimeImageModesadds the converted UNORM mode for sampled images that these two bits do not exclude.These cases still throw (
rejectImageat decode, or when the sampler state is built):_lz.The R10_G11_B11 float format (43) is not part of this PR: since feat(agc): sample texture format 43 through a lossless B10G11R11 copy #7271 its sampled images use a lossless B10G11R11 copy and the hardware sampler. On the converted path (storage or packed images) a float sample keeps main's refusal.
Cost: none for a title that never samples this format. A shader gets the converted sample mode only once a converted descriptor reaches one of its sampled images:
GuestContext::convertedSamplesadds it (ImageResource::convertedSamples, set when the program is prepared). The flag is part of every cache key, the disk cache included.ConvertedSampleRequired.CaptureInvocation) records the program offset in the registered program'sPreparedShaders, gets the invocation again (now with the flag, so the static key differs and the existing prepare-at-use path builds the new artifact) and repeats the capture.Recompile(request)retries the same way.Tested
BUILD_TESTING=ON, on main 91ddd9e (head 8545c23).agc_driver_image_converted_unorm:image_sample_lzchecks: point and bilinear filtering, clamp-to-edge and wrap addressing, on-grid and off-grid steps, swizzles X Y Z 1, Z Y X 1 and X 0 1 1. Every component is compared against a decode-and-blend model with 8-bit-grid weights.CheckFloatSample) are kept as they are. Clamp-to-border is refused. The gather and LOD-query refusals stay.off-grid wrapped bilinear image_sample_lz X Y Z 1: thread 2 component 0 is 0.994141, expected 0.992188). With the bilinear neighbours swapped it fails (bilinear image_sample_lz Z Y X 1: thread 0 component 0 is 0.000489, expected 0.000244).ConvertedSampleRequired, while one through the flagged artifact succeeds.pr_overlap_toolsandcheck_main_sync(git 2.30 here lacksmerge-tree --write-tree),guest_sce_net(DNS timeout) andagc_driver_bda_device, which fails the same way on plain main 91ddd9e (Vulkan result -4; perf(shader): look up the BDA range of a user-data pointer once at program entry #5859 on NVIDIA, fixed by fix(shader): leave the BDA lookup functions through one return #7571).tools/check_conventions.py --base origin/mainandtools/check_test_registration.pypass.0x10527dd300and ran to about 180 s, to an unrelated stop (runtime storage image mip capacity exceeded). Those runs used the version that also covered the float format and were not repeated after the rebase.Checklist
main; no other open PR implements the same functions.mdfiles or images added to the repository🤖 Generated with Claude Code