Repository navigation
fix(agc): resolve and render a multisampled target through its sRGB twin format - #8847
Conversation
|
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.
|
|
Thanks for checking the adjacent stack. Source-only coordination at your
The same constructor/release overlap propagates into #6987/#6989/#6995/#7002 because they retain that parent. All our PRs still merge cleanly with current main; this is a between-PR conflict, not evidence of a duplicated fix. I can adapt our stack if yours lands first. No combined build, GPU qualification, or claim that this fixes Sonic from this source review. AI-assisted: yes (Codex); author responsible. |
|
Thanks, I checked #6973 at
Neither PR depends on the other, so no |
|
Independent Linux check at exact head
The focused command was I also tested the twin fixture at 8 samples, matching Curved Space's sample count: the same image/view identity, return-to-original-view, unrelated-format separation and exact clear-byte checks pass 3/3 on each physical GPU. The temporary variant changes Ubuntu 24.04.3, GCC 13.3, Release. No title execution, combined #6973 stack, or Khronos validation-layer A/B was run here; the validation layer is unavailable on this host. All temporary source changes were removed, the exact PR was rebuilt and its two regressions rerun successfully on all three ICDs; checkout is clean. |
|
Heads-up from #8604, which gates Measured on
Whichever lands second takes the second form. If #8847 goes first, I'll rebase #8604 that way. |
|
Thanks, that's right, and it applies to this PR alone, not only after #8604: 40ed859 now runs it only when Checked on an RX 6650 XT (RADV Mesa 26.2.4), which has the feature: |
|
Windows run of 40ed859 merged onto Built
Both devices report the feature ( |
|
Ran
On a device without the feature the guarded One validation difference from
The instance is created with |
40ed859 to
5659579
Compare
|
Thanks, you're right. The format list needs 386d7d0: the device enables Rerun here (RX 6650 XT, RADV Mesa 26.2.4, which lists the extension; layer 1.4.363; the same 24-test focused set): 24/24, and the same On the storage path: agreed. With this PR alone, |
|
Reran
The |
|
Thank you for rerunning the refreshed head and separating the baseline failures from this change. I am recording these as your independently run results: the sRGB twin test passes 3/3 on HD 630, GTX 1050 Ti and llvmpipe, and the image-format-list report is absent. The HD 630 retirement failure remains a main baseline issue; the #8604 result is a skip, not a retirement pass. The device-create VUID on NVIDIA/llvmpipe also remains in the baseline. All three devices expose VK_KHR_image_format_list, so the unsupported-extension path remains untested by this set. |
…win format Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eMultisample Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Rebased on |
386d7d0 to
12223bf
Compare
|
Rerun on head
AI-assisted: yes (Claude Code). |
|
Windows rerun of head 12223bf, merged onto
Each log names the requested device in all 19 of its |
|
@ycbuzi thanks for this. It's the first run of the full 23-test set on Windows, with all three GPUs reporting both |
What
A multisampled color target matched a later use only when the use had the same
VkFormat. When a surface was rendered as sRGB and then CB-resolved as UNORM, or the other way round, the resolve got a new, cleared multisampled image. It resolved that image instead of the rendered samples, so the destination was all zeros. Both formats are the same bytes on the console. The CB format only says how they are encoded.Curved Space (PPSA02817) renders its 3D scene into an 8x
B8G8R8A8_SRGBtarget. The CB resolve then decodes the same surface asB8G8R8A8_UNORM. A RenderDoc replay of a gameplay frame onmainshows the scene drawn correctly into the 8x sRGB image (event 6961, max 1.0). The resolve pass reads a different 8xB8G8R8A8_UNORMimage that nothing drew to. Its 1x destination stays 0, and so do the bloom chain and the composite that read it. Only the HUD is drawn over black.The change:
SrgbTwinFormat, both directions) is created withMUTABLE_FORMATand a two-entryVkImageFormatListCreateInfo. This happens only when the device can render the twin at the target's sample count.acquirealso matches a live target in the twin format. The use is rewritten to the creation format beforeRecreate,ApplyFastClearand the guard, so fill and CMASK clear values are still computed in the format the image was created in.MultisampleTargetViewreturns a view in the requested format.ResolveMultisampleTargetresolves through a view and render pass in the source's format, so a UNORM resolve averages the stored bytes as UNORM, as the CB format says.Not changed: sampling a multisampled target (
MultisampleTargetTexture) or binding it as a storage image in its twin format still throws as before.Tested
Linux 7.2.9 (Arch), Ryzen 9 7950X, Radeon RX 6650 XT, RADV Mesa 26.2.4.
Curved Space (PPSA02817) 01.000, converted with
--module-dir Media/Modules --module-dir Media/Plugins. Same scripted 100 s run with input, screenshot at 95 s in the first level:main20de555: HUD over black, 2.1 % of pixels non-black (0.6 % in 3 earlier runs on da5ea87).Focused tests on
main20de555 (with only the driver source reverted) and on this PR:ctest -R 'multisample|multisampled|^agc_recorder' -VwithVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1. The Khronos layer 1.4.363 was loaded withVK_ADD_LAYER_PATH, andVK_LOADER_DEBUG=layershows it inserted in all 20 tests in both logs. Neither log reportswas not found but was requested by env var VK_INSTANCE_LAYERS.main: 18/20. The two new tests fail on their asserts:agc_driver_multisample_srgb_twin("a use of a multisampled surface in its sRGB twin format does not reach the samples rendered in the other format") andagc_driver_multisample_retirement("an sRGB use of an MSAA surface rendered as UNORM does not reach its samples").VUID-errors: 0 in both logs.New tests:
MultisampleSrgbTwin: a UNORM/sRGB pair in both directions gets the same image and a separate view, and goes back to the first view. An unrelated format of the same size gets its own image. A guest fill taken as a clear through the twin stores exactly the guest's bytes, read back withvkCmdResolveImage:0x80808080on a UNORM-created target used as sRGB, and0xff0000ffon an sRGB-created target used as UNORM. With the creation-format rewrite removed, the first case fails ("a fill over the color memory of a multisampled target has no exact clear value in its format").MultisampleRetirement::StorageTwin: after storage promotion, the sRGB use views the promoted image. It runs only whencontext.storageImageMultisampleis set, becauseMultisampleTargetStorageViewrefuses on devices withoutshaderStorageImageMultisample(40ed859, after @GiannisG1's report from an HD 630). The fixture now aborts when an image is destroyed while one of its views still exists. With the twin view retired after its image, it fails with "an MSAA image would be destroyed before one of its views". The existing cases pass with this check onmain.tools/check_conventions.py --base origin/mainpasses. The PR overlap tool finds only adjacent lines inMultisampleTarget.cpp, shared with #6973, #6987, #6989, #6995 and #7002, and in themain()ofMultisampleRetirement.cpp, shared with #8604. None of them changes format matching.Independent check by @simonbbby (comment), at exact head
84854e9116, on Ubuntu 24.04.3 with GCC 13.3:MultisampleTarget.cpprestored, both new tests fail on their asserts. With the PR rebuilt, they pass again.Windows check by @ycbuzi (comment), at head
40ed8597famerged ontomainb77bd58 (48c389eb56). Setup: Windows 11 Home 10.0.26200, MinGW-w64 GCC 15.2.0 (ucrt-posix-seh r7), Release,BUILD_TESTING=ON, NVIDIA driver 616.92.libSceAgcDriverand the two test targets this PR changes build with no errors. The twomemcmpbound warnings come fromGraphics/src/PipelineLibrary.cpp, which this PR does not change, andmainprints them the same way.ctest --repeat until-fail:3onagc_driver_multisample_retirementandagc_driver_multisample_srgb_twinpasses all three repeats on an RTX 4070 SUPER and on a GTX 1650 SUPER (ANYPS5_GPU=GTX 1650).shaderStorageImageMultisample, soStorageTwinran on each and the skip branch did not.84854e9116, the two tests also passed twice on the RTX 4070 SUPER.Not tested: a device without
shaderStorageImageMultisample(theStorageTwinskip branch), a validation layer on Windows, and a CB resolve driven through PM4 in a unit test (covered only by the title run).Rebased on
main2abfb1beb4(head56595793ca).mainnow creates a multisampled image withMUTABLE_FORMAT | EXTENDED_USAGEand adds storage usage when samples may be reordered (the sample-order view). With those flags, the twin format list is not added, and the twin view relies onMUTABLE_FORMAT.orderFlags | MUTABLE_FORMAT) and only the twin view's usage (color attachment and transfer). Otherwise, the added storage usage would have turned the twin off for sRGB targets.release()destroys the order view, the twin view and the resolve passes before the image.Same focused set, now 24 tests including
main's sample-order tests, on the same machine with the Khronos layer:mainwith onlyMultisampleTarget.cpprestored: 22/24. The two new tests fail on their asserts.VUID-lines in the same tests: 12VkImageMemoryBarrier-image-03320inagc_driver_sample_order, and 32VkWriteDescriptorSet-descriptorType-00337inagc_driver_sample_order,..._grid_referenceand..._recipe_depth_locations. They come frommain's sample-order tests; the PR adds none.Curved Space at 95 s on the rebased build: the arena, ring, ship and HUD render.
VK_KHR_image_format_list (386d7d0, after @GiannisG1's report):
VK_API_VERSION_1_1, whereVkImageFormatListCreateInfobelongs toVK_KHR_image_format_list. Validation layers 1.3.275 reportedVUID-VkImageCreateInfo-pNext-pNexton an HD 630, a GTX 1050 Ti and llvmpipe.VK_KHR_image_format_listwhen the device lists it (Context::imageFormatList). The twin's format list is chained only then; otherwise the image relies onMUTABLE_FORMATalone.VUID-lines asmain, all in its sample-order tests. That layer never reported the pNext VUID here, so I could not reproduce it; the older-layer result needs a rerun on those devices.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)12223bf208merged ontomaina6dcea9279, the full 23-test focused set: NVIDIA RTX 4070 SUPER and GTX 1650 SUPER (616.92) and AMD Radeon Graphics, the Ryzen 9 7900 iGPU (25.10.30.02), Windows 11 Home 10.0.26200 (MinGW-w64 GCC 15.2.0)🤖 Generated with Claude Code