Repository navigation
fix(shader,agc): support multisampled 2D array descriptor image shape and runtime views - #6635
Conversation
…d dimension check
|
I traced this into |
|
@MarcZX100 Good catch — addressed in 729558c and 026e69f:
|
|
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
There was a problem hiding this comment.
The sampled-view shape concern from my earlier review is addressed on this head: the requested view type is threaded through the sampled color and depth paths, included in the texture cache keys, and covered by view-shape plus execution test cases. I found no remaining blocker in this change. The repository checks pass, while platform build/runtime jobs are skipped by path policy, so I am not claiming those new execution cases ran in CI.
Amoenus
left a comment
There was a problem hiding this comment.
Independent source review and cross-vendor validation of head 026e69fa6024858744eb1ae17995d9b84507356a on main 0615cd2b82926b2f4daf1d31ac9bc7354b6d0a17: no blocker found in the reviewed change.
Reproducible validation composition fd9f3d707: clean main + this PR, plus only #6225's fixture GPU selector and #6452's WinLibs test-alignment correction. These are test-infrastructure additions; this PR's production diff and new array assertions are unchanged.
- Earlier main7dc composition
41a94acb2passes the full Windows app/libraries and patchedlibsbuild, 27 Windows and 18 Linux runs. That full-build result stays at its original pin. - Latest tested
fd9f3d707includes main0615's dispatch/pipeline/journal and shader-cache changes. Affected Windows libraries/six targets and the same six Linux targets build; conventions and registration pass. Targeted revalidation passes 21 Windows runs and 12 Linux runs: three CPU groups once each (agc_shader_memory,agc_driver_graphics,agc_shader_disk_cache), and three GPU groups x3 per physical vendor/software device (agc_multisample_guest_access,agc_driver_sample_multisample_target,agc_driver_native_multisample_depth). GPU identity is verified in every GPU process; no whole-test skips/timeouts, maximum 7.140s. - Linux: Ubuntu24.04 / GCC13.3 / Mesa25.2.8 llvmpipe. Native-depth cases check D16/D32 at 4x (4 checked, 8 unavailable per process); both Windows GPUs check all 12 cases. CPU heap-mirror omissions are excluded from coverage. Dispatch uses the default mode; the optional
APS5_PIPELINE_DISPATCHmode and journal replay are outside this matrix. - Color array loads agree with the checked non-array sample values; depth array fetches verify imported and rasterized values. The view tests check distinct 2D/2D-array cache entries and mixed consecutive bindings. Source inspection confirms the new enum is appended, preserving existing values.
GPUs tested: NVIDIA RTX5080 (10de:2c02), driver610.62/32.0.16.1062; AMD Radeon(TM) Graphics (1002:13c0), driver32.0.21036.18; Windows11 Enterprise26200 / WinLibs GCC15.2r7. Serial fresh processes, Vulkan required, project shader cache disabled; driver caches uncontrolled.
Coverage is array-shaped views of one-layer native targets, using layer zero. Source inspection confirms the decoder retains its nonzero MSAA depth/base-array refusal. Multiple resident layers, console equivalence and title startup remain untested here.
Main subsequently advanced to aff725bf6d8d61ed33e80507421fd131aeae1305 (#5884 mesh line-strip output and #5886 unnormalized-sampler base-level proof). I inspected the relevant hunks: the native view-type/cache plumbing and sampler-free array-load paths are unchanged, and the complete merge preview is clean. These later commits are outside the executed pins above; no unrelated matrix restart. AI-assisted: Codex; I ran these local builds/tests.
|
cc @adriadam10 @oneandonlydean rebase |
|
cc @adriadam10 @oneandonlydean rebased again |
|
I have a focused candidate for issue #6957: sample a multisampled target in |
|
Thanks for checking, @MarcZX100! It would be great if #6635 lands first since it's already approved and ready for the maintainers. If you don't mind holding off on opening yours until it's merged, that will prevent any unnecessary conflicts. |
|
Thanks for the guidance. I had opened #6971 shortly before seeing your reply, after #6683’s author confirmed that either order was workable. I have now marked #6971 as draft to avoid parallel review/merge conflicts. Once #6635 lands, I’ll rebase it and recheck the array-view/cache-key integration before marking it ready. |
|
Hi @adriadam10 @oneandonlydean, hope you're doing well! Just bumping this so it doesn't get lost in the backlog. The changes are purely focused on optimizations, all checks are passing, and there are no merge conflicts. Would love to get your eyes on it when you have a few minutes! |
|
Refining this report after checking the shader type and specification: I compiled an independent Earlier hardware control: our main55 integration with the older #6635 array-view work and #7233 completion proof emitted54 VUID08740 errors per GPU in the native clear/refusal groups repeated3x. Enabling the host storage-MSAA feature eliminated those errors, with both groups3/3 on RTX5080 (32.0.16.1062) and AMD Radeon Graphics (32.0.21036.18), Windows11 Enterprise26200 / Khronos1.4.363. That control satisfies the declared capability, but introduces an unnecessary feature prerequisite and is not the preferred portable fix. Linux/lavapipe passed its available cases; no Linux validation layer is available. These are integration/control results, not execution of unchanged latest head ebc6ea3. No production feature-negotiation change is proposed. The separate command-pool reset issue remains #7311. AI-assisted: Codex; author responsible. Actual fixture validation completed: focused one-line capability removal9e17f1522 is in integration4c2cab/main5de. The prior host-feature addition is reverted. Both native clear/refusal groups pass3/3 on each physical GPU with Khronos1.4.363 loaded and zero observed VUIDs, while the available Linux cases also pass3/3. Broader volume/packed-color/publication controls pass in that same composition. This validates the sampled-only correction on our composition, not unchanged latest #6635 head. Please prefer this capability removal to the earlier feature-enabling control. |
…-descriptor-shape # Conflicts: # core/libs/prx/libSceAgcDriver/Graphics/src/MultisampleTarget.cpp # core/libs/prx/libSceAgcDriver/tests/execution/SampleMultisampleTarget.cpp
|
I checked the current head |
|
@Amoenus Thank you for the thorough SPIR-V specification breakdown ( @MarcZX100 Confirmed: executed the focused
|
|
Thanks for rerunning the current head. The 12/12 focused native-depth cases with no validation-layer errors on the capability-removal head answer the current-head validation request; the earlier host-feature workaround is unnecessary. I have no further request on this capability issue. |
Resolves MultisampleTarget against #6635: the sampled view takes the requested view type and keeps the sampled-only usage when the target has a storage view for the guest's sample order.
What
DescriptorImageShape::Image2DMultisampleArrayinRecompiler.hpp.ImageDimension::Dim2DMsaaArray(isArray && isMsaa) toDescriptorImageShape::Image2DMultisampleArrayinDescriptorBindingBuilder::Populateinstead of throwingmultisampled array image resources have no descriptor image shape.DescriptorImageShape::Image2DMultisampleArrayagainstTextureDimension::k2DMsaainGuestTextureResource::MatchesGuestDimension.VkImageViewType(VK_IMAGE_VIEW_TYPE_2D_ARRAYforDescriptorImageShape::Image2DMultisampleArray,VK_IMAGE_VIEW_TYPE_2DforDescriptorImageShape::Image2DMultisample) throughShaderResources,MultisampleTarget::Sampled, andDepthSurface::Sampled, including the view type in the sampled texture cache keys.agc_shader_memory,agc_driver_graphics(Recorder.cpp),agc_driver_sample_multisample_target, andagc_driver_native_multisample_depthcoveringDim2DMsaaArraydescriptor binding generation, guest texture dimension matching,VK_IMAGE_VIEW_TYPE_2D_ARRAYruntime view binding/caching, andImageMSArrayOpImageFetchexecution on MSAA color and depth targets.Tested
ctest --test-dir build-portable -R "^(agc_driver_graphics|agc_shader_memory|agc_multisample_guest_access|agc_driver_sample_multisample_target|agc_driver_native_multisample_depth)$" --output-on-failure(all passed).shader library prebuild skipped: DescriptorBindingBuilder::Populate multisampled array image resources have no descriptor image shapeerrors duringunity default resourcesshader registration are resolved.Before (
main):After (this PR):
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)