From 0a25af4ca791eecfeeeb6fd89ba01bac880a15bf Mon Sep 17 00:00:00 2001 From: Jamie <55252868+chitters2004@users.noreply.github.com> Date: Fri, 30 Oct 2026 04:00:14 +0000 Subject: [PATCH] fix(agc): key a stage without push constants at offset 0 in every pipeline key Co-Authored-By: Claude Opus 5.5 --- .../libSceAgcDriver/Graphics/include/Shaders.hpp | 4 ++++ .../prx/libSceAgcDriver/Graphics/src/Draw.cpp | 6 +++--- .../prx/libSceAgcDriver/Graphics/src/Pipeline.cpp | 4 ++-- core/libs/prx/libSceAgcDriver/tests/Recorder.cpp | 8 +++++++- .../tests/execution/PreparedGraphics.cpp | 15 +++++++++++++++ 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Shaders.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Shaders.hpp index e2f279e8e4b0..659c2dbe100d 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Shaders.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Shaders.hpp @@ -41,6 +41,10 @@ struct CompiledShader { const std::vector* registeredImageViews = nullptr; }; +inline std::uint32_t KeyedPushOffset(const CompiledShader& shader) { + return shader.program->pushConstants.empty() ? 0u : shader.pushConstantOffset; +} + inline VkShaderStageFlagBits VulkanStage(ShaderRecompiler::ShaderStage stage) { switch (stage) { case ShaderRecompiler::ShaderStage::Vertex: diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp index 195b50198698..74ec1ad5bb25 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp @@ -1047,7 +1047,7 @@ bool ValidationKey(const Context& context, std::span shade } add(shader.stage); add(generated ? std::uint64_t{0} : program.PipelineVariantId()); - add(shader.pushConstantOffset); + add(KeyedPushOffset(shader)); add(program.pushConstants.size()); add(program.bdaAbiVersion); add(program.vertexAttributes.size()); @@ -3074,7 +3074,7 @@ bool sameStages(std::span kept, std::spanIdentity(); - if (identity.Empty() || kept[at] != static_cast(shader.stage) || kept[at + 1] != identity.low || kept[at + 2] != identity.high || kept[at + 3] != shader.pushConstantOffset) return false; + if (identity.Empty() || kept[at] != static_cast(shader.stage) || kept[at + 1] != identity.low || kept[at + 2] != identity.high || kept[at + 3] != KeyedPushOffset(shader)) return false; at += 4; } return kept[at] == PushConstantStages(shaders); @@ -3128,7 +3128,7 @@ void rememberShape(const Context& context, std::uint64_t shape, std::spanIdentity(); if (identity.Empty()) return; - choice.stages.insert(choice.stages.end(), {static_cast(shader.stage), identity.low, identity.high, shader.pushConstantOffset}); + choice.stages.insert(choice.stages.end(), {static_cast(shader.stage), identity.low, identity.high, KeyedPushOffset(shader)}); } choice.stages.push_back(PushConstantStages(shaders)); choice.bindings = input.bindings; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp index c6efd8fd70ae..f0e06e9b5c15 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp @@ -356,7 +356,7 @@ bool libraryKeys(PipelineLibraryKeys& keys, const Context& context, const State& appendKey(key, shader.stage); appendKey(key, identity.low); appendKey(key, identity.high); - appendKey(key, shader.pushConstantOffset); + appendKey(key, KeyedPushOffset(shader)); appendKey(key, state.meshLayerBase); appendKey(key, state.meshLayerCount); if (resources.split) { @@ -1035,7 +1035,7 @@ void pipelineKey(std::vector& key, const Context& context, const Stat append(key, identity.low); append(key, identity.high); // Where the stage's push constants sit in the block (AssemblePushConstants). - append(key, shader.pushConstantOffset); + append(key, KeyedPushOffset(shader)); } append(key, state.meshLayerBase); append(key, state.meshLayerCount); diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 357615442be3..dd3553706512 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -14349,11 +14349,17 @@ void pipelineLibraryTests(const Device& device) { const auto beforeSlots = counters().built; ShaderRecompiler::RecompileResult otherVertex = vertex; otherVertex.spirv[2] ^= 1u; - std::array slotted{CompiledShader{ShaderRecompiler::ShaderStage::Vertex, &vertex, 0}, CompiledShader{ShaderRecompiler::ShaderStage::Fragment, &fragment, PipelinePushSlotBytes}}; + ShaderRecompiler::RecompileResult slottedFragment = fragment; + slottedFragment.spirv[2] ^= 2u; + std::array slotted{CompiledShader{ShaderRecompiler::ShaderStage::Vertex, &vertex, 0}, CompiledShader{ShaderRecompiler::ShaderStage::Fragment, &slottedFragment, PipelinePushSlotBytes}}; static_cast(CachedPipeline(context, state, input, resources, slotted, VK_IMAGE_LAYOUT_GENERAL)); slotted.front().program = &otherVertex; static_cast(CachedPipeline(context, state, input, resources, slotted, VK_IMAGE_LAYOUT_GENERAL)); Require(counters().built[2] == beforeSlots[2] + 1u && counters().built[1] == beforeSlots[1] + 1u, "a pixel shader in its fixed slot built its library again for another vertex shader"); + const auto afterSlots = counters().built; + slotted.back().program = &fragment; + static_cast(CachedPipeline(context, state, input, resources, slotted, VK_IMAGE_LAYOUT_GENERAL)); + Require(counters().built == afterSlots, "a pixel shader without push constants built its libraries again at its fixed slot"); fragment.spirv[2] ^= 1u; static_cast(lookup()); Require(counters().built == std::array{2, 2, 3, 2}, "a new pixel shader rebuilt more than the fragment shader library"); diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp index ca80e465ebc5..f1d5c55cda90 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp @@ -397,6 +397,21 @@ void PrebuildPolicyReadback(AgcDriver::VulkanDevice& device, bool disabled) { device.Draw(state, {0u, 3u, 0u, 1u, 0u, false}, shaders); device.WaitIdle(); Require(std::ranges::all_of(pixels, [](const auto value) { return value == 0xffffffffu; }), "prebuild policy changed native prepared-shader readback"); + if (!disabled && device.GraphicsPipelineLibraries()) { + ShaderRecompiler::RecompileResult pushless; + pushless.spirv = {0x07230203u, 0x00010000u, 0u, 6u, 0u, 0x00020011u, 1u, 0x0003000eu, 0u, 1u, 0x0005000fu, 4u, 4u, 0x6e69616du, 0u, 0x00030010u, 4u, 7u, 0x00020013u, 2u, 0x00030021u, 3u, 2u, 0x00050036u, 2u, 4u, 0u, 3u, 0x000200f8u, 5u, 0x000100fdu, 0x00010038u}; + pushless.variantId = 0x70757368u; + pushless.runtimeAbiVersion = ShaderRecompiler::RuntimeAbi::Version; + const std::array pixelOnly{{{ShaderStage::Fragment, &pushless, AgcDriver::Graphics::PipelinePushSlotBytes}}}; + const auto prebuilt = AgcDriver::Graphics::PipelineLibraryCountersOf(device.Device()).built[2]; + Require(device.PrebuildGraphicsLibraries(decoded.state.stages, pixelOnly), "prebuild policy: a pixel stage's libraries were not prebuilt"); + const auto built = AgcDriver::Graphics::PipelineLibraryCountersOf(device.Device()).built[2]; + Require(built == prebuilt + 1u, "prebuild policy: a pixel stage without push constants did not build its fragment library"); + shaders.back() = {ShaderStage::Fragment, &pushless, 0u}; + device.Draw(state, {0u, 3u, 0u, 1u, 0u, false}, shaders); + device.WaitIdle(); + Require(AgcDriver::Graphics::PipelineLibraryCountersOf(device.Device()).built[2] == built, "prebuild policy: a draw built the fragment library its pixel stage had prebuilt"); + } } void RetainedPixelMatchesNullProgram(AgcDriver::VulkanDevice& device) {