From 79c8d3690133a35102eb3ac0095a99153dcf8456 Mon Sep 17 00:00:00 2001 From: XairRrR Date: Sun, 11 Oct 2026 14:58:57 +0200 Subject: [PATCH 1/2] fix(agc): reject constant color factors and memory-writing stages in split SWAP_ALT blends Review follow-up to the 8_8 SWAP_ALT blending. The second channel of such a target holds the guest's alpha. Vulkan applies the green blend constant to it for CONSTANT_COLOR and ONE_MINUS_CONSTANT_COLOR, where the guest's alpha equation uses the alpha constant. Decoding now throws for these factors on a SWAP_ALT 8_8 target, unless the equation is MIN or MAX, which ignore factors. The second pass of a split draw runs every stage again, not only the pixel shader, so a vertex or geometry stage that writes memory would write twice. The memory-write check (now ShadersWriteMemory) covers all the draw's stages. --- .../Graphics/include/Pipeline.hpp | 1 + .../libSceAgcDriver/Graphics/src/Pipeline.cpp | 32 +++++++++---------- .../libSceAgcDriver/Graphics/src/State.cpp | 2 ++ .../prx/libSceAgcDriver/tests/Graphics.cpp | 30 +++++++++++++++++ 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Pipeline.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Pipeline.hpp index c4fa93353..5258bc026 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Pipeline.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Pipeline.hpp @@ -107,6 +107,7 @@ void ValidateDepthBounds(const Context& context, const State& state); void ValidateProvokingVertex(const Context& context, const State& state, std::span shaders = {}); void ValidateShaderPair(const ShaderRecompiler::RecompileResult& vertex, const ShaderRecompiler::RecompileResult& fragment); +bool ShadersWriteMemory(std::span shaders); // Returns the color attachment locations the pixel shader writes. std::set ValidateShaders(std::span shaders, const State& state, const VkPhysicalDeviceSubgroupProperties& subgroup, bool fragmentShaderBarycentric, bool descriptorIndexing = false, bool imageInt64Atomics = false, bool geometryShader = false, bool sampleRateShading = false, bool bufferInt64Atomics = false); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp index d47dcb3da..43dbbbe4b 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp @@ -115,23 +115,10 @@ void ValidateViewport(const Context& context, const VkViewport& viewport) { Require(std::min(viewport.y, viewport.y + viewport.height) >= context.limits.viewportBoundsRange[0] && std::max(viewport.y, viewport.y + viewport.height) <= context.limits.viewportBoundsRange[1], "viewport Y exceeds device bounds"); } -namespace { - -VkShaderStageFlags pushStagesOf(const Context& context) { - return PreRasterizationPushStages(context.meshShader, context.tessellationShader, context.geometryShader); -} - -template -void appendKey(std::vector& key, const TValue& value) { - static_assert(std::is_trivially_copyable_v); - const auto bytes = std::as_bytes(std::span(&value, 1)); - key.insert(key.end(), bytes.begin(), bytes.end()); -} - -bool fragmentWritesMemory(std::span shaders) { +bool ShadersWriteMemory(std::span shaders) { const auto any = [](const std::vector& written, std::uint32_t count) { return written.size() < count || std::ranges::any_of(written, [](bool element) { return element; }); }; for (const auto& shader : shaders) { - if (shader.stage != ShaderRecompiler::ShaderStage::Fragment || shader.program == nullptr) continue; + if (shader.program == nullptr) continue; for (const auto& binding : shader.program->bindings) { switch (binding.role) { case ShaderRecompiler::DescriptorRole::GuestBuffers: @@ -154,6 +141,19 @@ bool fragmentWritesMemory(std::span shaders) { return false; } +namespace { + +VkShaderStageFlags pushStagesOf(const Context& context) { + return PreRasterizationPushStages(context.meshShader, context.tessellationShader, context.geometryShader); +} + +template +void appendKey(std::vector& key, const TValue& value) { + static_assert(std::is_trivially_copyable_v); + const auto bytes = std::as_bytes(std::span(&value, 1)); + key.insert(key.end(), bytes.begin(), bytes.end()); +} + bool libraryKeys(PipelineLibraryKeys& keys, const Context& context, const State& state, const VertexInputLayout& input, const ShaderResources& resources, std::span shaders) { using Stage = ShaderRecompiler::ShaderStage; if (!context.graphicsPipelineLibrary || context.pipelineExecutableInfo || state.depthBiasPerFace || state.rectList || SplitsAlternateAlpha(state)) return false; @@ -379,7 +379,7 @@ Pipeline::Pipeline(const Context& context, const State& state, const VertexInput if (alternateAlpha) { const auto keepsStencil = [](const VkStencilOpState& face) { return face.writeMask == 0 || (face.failOp == VK_STENCIL_OP_KEEP && face.passOp == VK_STENCIL_OP_KEEP && face.depthFailOp == VK_STENCIL_OP_KEEP); }; Require(!(state.depth && (state.depthWrite || (state.stencilTest && (!keepsStencil(state.stencilFront) || !keepsStencil(state.stencilBack))))) && !state.depthBiasPerFace, "blending into an 8_8 color target with the alternate component order and a separate alpha blend from a draw that writes depth or stencil is not implemented"); - Require(!fragmentWritesMemory(shaders), "blending into an 8_8 color target with the alternate component order and a separate alpha blend from a pixel shader that writes memory is not implemented"); + Require(!ShadersWriteMemory(shaders), "blending into an 8_8 color target with the alternate component order and a separate alpha blend from a draw whose shaders write memory is not implemented"); for (std::size_t index = 0; index < secondChannels.size(); ++index) { const bool split = index < state.colors.size() && state.colors[index].componentMapping == 0xecu && state.blends[index].blendEnable; if (!split) { diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp index cf0d6b1ea..f1eec43a6 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp @@ -771,6 +771,8 @@ State DecodeState(const QueueState& queue, bool vertexWorkgroup) { if (mapping == 0xecu) { const auto destinationAlpha = [](VkBlendFactor factor) { return factor == VK_BLEND_FACTOR_DST_ALPHA || factor == VK_BLEND_FACTOR_ONE_MINUS_DST_ALPHA || factor == VK_BLEND_FACTOR_SRC_ALPHA_SATURATE; }; if (destinationAlpha(state.srcColorBlendFactor) || destinationAlpha(state.dstColorBlendFactor) || destinationAlpha(state.srcAlphaBlendFactor) || destinationAlpha(state.dstAlphaBlendFactor)) throw std::runtime_error("AGC graphics: blending into an 8_8 color target with the alternate component order with a factor of the destination alpha, which the target keeps in its second channel, is not implemented"); + const auto weighted = [](VkBlendOp operation) { return operation != VK_BLEND_OP_MIN && operation != VK_BLEND_OP_MAX; }; + if ((weighted(state.colorBlendOp) && (constantColor(state.srcColorBlendFactor) || constantColor(state.dstColorBlendFactor))) || (weighted(state.alphaBlendOp) && (constantColor(state.srcAlphaBlendFactor) || constantColor(state.dstAlphaBlendFactor)))) throw std::runtime_error("AGC graphics: blending into an 8_8 color target with the alternate component order with a constant color factor, which Vulkan applies to the second channel with the green blend constant instead of the alpha one, is not implemented"); } } if (color.packing != ShaderRecompiler::ColorExportPacking::None) { diff --git a/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp b/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp index 78a2e9a03..4fa978039 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp @@ -878,6 +878,29 @@ void TuningFieldTests() { } } +void ShadersWriteMemoryTests() { + const auto binding = [](ShaderRecompiler::DescriptorKind kind, ShaderRecompiler::DescriptorRole role, std::vector written) { + ShaderRecompiler::DescriptorBinding result{}; + result.kind = kind; + result.role = role; + result.count = 1; + if (role == ShaderRecompiler::DescriptorRole::GuestImages) result.imageWritten = std::move(written); + else result.bufferWritten = std::move(written); + return result; + }; + ShaderRecompiler::RecompileResult reads; + reads.bindings.push_back(binding(ShaderRecompiler::DescriptorKind::StorageBuffer, ShaderRecompiler::DescriptorRole::GuestBuffers, {false})); + reads.bindings.push_back(binding(ShaderRecompiler::DescriptorKind::StorageImage, ShaderRecompiler::DescriptorRole::GuestImages, {false})); + ShaderRecompiler::RecompileResult stores; + stores.bindings.push_back(binding(ShaderRecompiler::DescriptorKind::StorageBuffer, ShaderRecompiler::DescriptorRole::GuestBuffers, {true})); + using Stage = ShaderRecompiler::ShaderStage; + const auto writes = [](std::initializer_list shaders) { return AgcDriver::Graphics::ShadersWriteMemory(std::vector(shaders)); }; + Require(!writes({{Stage::Vertex, &reads, 0}, {Stage::Fragment, &reads, 0}}), "shaders that only read memory were reported as writing it"); + Require(writes({{Stage::Vertex, &reads, 0}, {Stage::Fragment, &stores, 0}}), "a pixel shader that stores to a buffer was not reported"); + Require(writes({{Stage::Vertex, &stores, 0}, {Stage::Fragment, &reads, 0}}), "a vertex shader that stores to a buffer was not reported"); + Require(writes({{Stage::Geometry, &stores, 0}, {Stage::Fragment, &reads, 0}}), "a geometry shader that stores to a buffer was not reported"); +} + void ReversedComponentOrderTests() { { auto queue = makeState(); @@ -891,6 +914,12 @@ void ReversedComponentOrderTests() { Require(!AgcDriver::Graphics::SplitsAlternateAlpha(blended), "one blend equation for color and alpha split an 8_8 SWAP_ALT draw"); queue.context[0x1e0] = 0x60000001u; Require(AgcDriver::Graphics::SplitsAlternateAlpha(AgcDriver::Graphics::DecodeState(queue)), "a separate alpha blend into an 8_8 SWAP_ALT target did not split its second channel"); + queue.context[0x1e0] = (1u << 30u) | 13u; + expectFailure([&] { AgcDriver::Graphics::DecodeState(queue); }, "constant color factor"); + queue.context[0x1e0] = (1u << 30u) | (1u << 29u) | 1u | (1u << 16u) | (14u << 24u); + expectFailure([&] { AgcDriver::Graphics::DecodeState(queue); }, "constant color factor"); + queue.context[0x1e0] = (1u << 30u) | (2u << 5u) | 13u; + Require(AgcDriver::Graphics::DecodeState(queue).blends[0].colorBlendOp == VK_BLEND_OP_MIN, "a constant color factor of a MIN blend into an 8_8 SWAP_ALT target was rejected"); } for (const auto& [swap, mapping] : {std::pair{2u, 0x1bu}, std::pair{3u, 0x93u}}) { auto queue = makeState(); @@ -4305,6 +4334,7 @@ int main(int argc, char** argv) { OpaqueDestinationAlphaTests(); CompactedExportTests(); ReversedComponentOrderTests(); + ShadersWriteMemoryTests(); DualSourceBlendTests(); metadataPassTests(); cmaskTests(); From 0ad1a4688429d36d6911711ed1446d0d209db0b4 Mon Sep 17 00:00:00 2001 From: XairRrR Date: Sun, 11 Oct 2026 22:44:57 +0200 Subject: [PATCH 2/2] test(agc): reject a split SWAP_ALT draw whose vertex shader stores to memory The 8_8 SWAP_ALT blend test now also draws with a vertex shader that stores a dword to a buffer (buffer_store_dword through user data 4-7) and a separate alpha blend, and requires the draw to be rejected for its shaders writing memory. With the former pixel-only check the draw went through and the test fails. --- .../tests/execution/SwapAltBlend.cpp | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/SwapAltBlend.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/SwapAltBlend.cpp index 432c19bb8..393d98a1f 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/SwapAltBlend.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/SwapAltBlend.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include namespace { @@ -53,6 +54,12 @@ alignas(256) constexpr std::array VertexCode{ 0xe0382000, 0x80000005, 0xbf8c3f70, 0xf80008cf, 0x03020100, 0xbf810000, }; +alignas(256) constexpr std::array StoringVertexCode{ + 0xe0382000, 0x80000005, 0xbf8c3f70, 0xe0700000, 0x80010000, 0xbf8c3f70, 0xf80008cf, 0x03020100, 0xbf810000, +}; + +alignas(256) std::array Stored{}; + constexpr std::array, 3> Triangle{{ {-1.0f, -1.0f, 0.5f, 1.0f}, {3.0f, -1.0f, 0.5f, 1.0f}, {-1.0f, 3.0f, 0.5f, 1.0f} }}; @@ -163,16 +170,19 @@ VkPipelineColorBlendAttachmentState Blend(VkBlendFactor source, VkBlendFactor de return blend; } -void Draw(AgcDriver::VulkanDevice& device, const AgcDriver::Graphics::ColorTarget& color, const VkPipelineColorBlendAttachmentState& blend, std::span pixelCode) { +void Draw(AgcDriver::VulkanDevice& device, const AgcDriver::Graphics::ColorTarget& color, const VkPipelineColorBlendAttachmentState& blend, std::span pixelCode, std::span vertexCode = VertexCode) { const auto target = device.Target(); constexpr std::uint32_t waveSize = 64; - std::vector vertexUserData(4, 0u); + std::vector vertexUserData(8, 0u); const auto address = reinterpret_cast(Triangle.data()); const std::array vertexBuffer{static_cast(address), static_cast((address >> 32u) & 0xffffu) | (16u << 16u), static_cast(Triangle.size()), 0x01016facu}; std::copy(vertexBuffer.begin(), vertexBuffer.end(), vertexUserData.begin()); - const std::array vertexMemory{{{reinterpret_cast(VertexCode.data()), std::as_bytes(std::span(VertexCode))}}}; + const auto stored = reinterpret_cast(Stored.data()); + const std::array storeBuffer{static_cast(stored), static_cast((stored >> 32u) & 0xffffu), static_cast(sizeof(Stored)), 0x01016facu}; + std::copy(storeBuffer.begin(), storeBuffer.end(), vertexUserData.begin() + 4); + const std::array vertexMemory{{{reinterpret_cast(vertexCode.data()), std::as_bytes(vertexCode)}}}; ShaderRecompiler::RecompileRequest vertex{ - {ShaderStage::Vertex, reinterpret_cast(VertexCode.data()), VertexCode, 0, {}}, + {ShaderStage::Vertex, reinterpret_cast(vertexCode.data()), vertexCode, 0, {}}, {waveSize, 0, vertexUserData, std::nullopt, std::nullopt, ShaderRecompiler::ShaderVertexStageInfo{}, vertexMemory}, target, {0, 0, 0, 64} @@ -281,6 +291,18 @@ int main() { {"ONE + ONE_MINUS_SRC_ALPHA for color and alpha", Blend(VK_BLEND_FACTOR_ONE, VK_BLEND_FACTOR_ONE_MINUS_SRC_ALPHA, VK_BLEND_OP_ADD, VK_BLEND_FACTOR_ONE, VK_BLEND_FACTOR_ONE_MINUS_SRC_ALPHA, VK_BLEND_OP_ADD), false, [&](float r, float g) { return std::array{x + r * (1.0f - w), w + g * (1.0f - w)}; }}, }}; + { + Fill(block, layout); + const auto color = DecodeTarget(block); + bool rejected = false; + try { + Draw(*device, color, cases[0].blend, Program(exported), StoringVertexCode); + } catch (const std::exception& error) { + rejected = std::string_view(error.what()).find("a draw whose shaders write memory") != std::string_view::npos; + if (!rejected) throw; + } + Require(rejected, "a split 8_8 SWAP_ALT draw whose vertex shader stores to memory was not rejected"); + } for (const auto& test : cases) { const auto before = Fill(block, layout); const auto color = DecodeTarget(block);