From 1bf43f9b02a7e544cc332185598e4deb99035361 Mon Sep 17 00:00:00 2001 From: LeandroLP23 Date: Tue, 20 Oct 2026 17:10:37 +0200 Subject: [PATCH] fix(recompiler): read P20 of a flat input as the provoking vertex A FLAT_SHADE input without pass-through that a pixel shader reads through v_interp_mov_f32 P20 and not P10 stayed an explicit per-vertex input and read vertex 2 minus vertex 0. For a flat value that is the same on every vertex the difference is 0, and with the first provoking vertex nothing refused it. ps5-opengl lowers every flat input to a load of one vertex: vertex 1 (P10) by default, which #4030 reads as the provoking vertex, and vertex 2 (P20) for triangles when GL's flatshade_first is set. SuperTuxKart's deferred point lights are drawn that way, so all twelve flat inputs of pointlight.frag read 0, the attenuation 0 * -inf was NaN, nothing was discarded, and the starting-line referee's light quad blacked out the scene behind it (#8364). A P20-only read now returns the provoking vertex's value, as P10 and P0 do. The lane was not measured on the console either, so FragmentParameter records it and a pipeline using the last provoking vertex throws. An input read through both P10 and P20 keeps its per-vertex differences. Co-Authored-By: Claude Opus 5.5 --- .../prx/libSceAgcDriver/Graphics/src/Pipeline.cpp | 4 ++++ core/libs/prx/libSceAgcDriver/tests/Graphics.cpp | 8 ++++++++ .../Optimization/src/ShaderInfoCollector.cpp | 6 +++++- core/shader/recompiler/Recompiler.hpp | 1 + core/shader/recompiler/ShaderDiskCache.cpp | 2 ++ .../src/SpirvMemory/SpirvInputOutput.cpp | 12 +++++++++--- docs/dev/debt/libSceAgcDriver.md | 2 +- 7 files changed, 30 insertions(+), 5 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp index 746a4c53cecc..aaa08ba07f20 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp @@ -112,6 +112,10 @@ void ValidateProvokingVertex(const Context& context, const State& state, std::sp } return; } + for (const auto& shader : shaders) { + if (shader.stage != ShaderRecompiler::ShaderStage::Fragment || shader.program == nullptr) continue; + Require(std::none_of(shader.program->fragmentParameters.begin(), shader.program->fragmentParameters.end(), [](const ShaderRecompiler::FragmentParameter& parameter) { return parameter.flatP20; }), "v_interp_mov p20 of a flat input with the last provoking vertex is not measured"); + } if (BarycentricPatches(state, shaders)) { for (const auto& shader : shaders) { if (shader.stage != ShaderRecompiler::ShaderStage::Fragment || shader.program == nullptr) continue; diff --git a/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp b/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp index d9d764d50d11..41b0fb6ec4e7 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp @@ -9210,6 +9210,14 @@ void ProvokingVertexTests() { const std::array flatP10Shaders{{{ShaderRecompiler::ShaderStage::Fragment, &flatP10Pixel, 0}}}; ValidateProvokingVertex(supported, last, flatP10Shaders); expectFailure([&] { ValidateProvokingVertex(supported, first, flatP10Shaders); }, "v_interp_mov p10 of a flat input with the first provoking vertex is not measured"); + const std::array flatP20Color{0xc8120001u, 0xc8160101u, 0xc81a0201u, 0xc81e0301u, 0xf800180fu, 0x07060504u, 0xbf810000u}; + const auto flatP20Pixel = recompilePixel({0x400u}, flatP20Color); + const auto p20Inputs = locatedInputs(flatP20Pixel.spirv.Words()); + Require(p20Inputs.size() == 1 && p20Inputs[0].location == 0 && p20Inputs[0].flat && !p20Inputs[0].perVertex, "flat v_interp_mov P20 did not compile to a flat input"); + Require(flatP20Pixel.fragmentParameters.size() == 1 && flatP20Pixel.fragmentParameters[0].flatP20 && !flatP20Pixel.fragmentParameters[0].flatP10 && !flatP20Pixel.fragmentParameters[0].perVertex && !flatP10Pixel.fragmentParameters[0].flatP20, "flat v_interp_mov P20 was not recorded apart from P10"); + const std::array flatP20Shaders{{{ShaderRecompiler::ShaderStage::Fragment, &flatP20Pixel, 0}}}; + ValidateProvokingVertex(supported, first, flatP20Shaders); + expectFailure([&] { ValidateProvokingVertex(supported, last, flatP20Shaders); }, "v_interp_mov p20 of a flat input with the last provoking vertex is not measured"); auto lastStrip = last; lastStrip.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_STRIP; auto lastList = last; diff --git a/core/shader/recompiler/Optimization/src/ShaderInfoCollector.cpp b/core/shader/recompiler/Optimization/src/ShaderInfoCollector.cpp index b7f35bb4c146..aa37749e51aa 100644 --- a/core/shader/recompiler/Optimization/src/ShaderInfoCollector.cpp +++ b/core/shader/recompiler/Optimization/src/ShaderInfoCollector.cpp @@ -285,6 +285,8 @@ void CollectPixelInputs(const IrProgram& program, const ShaderPixelInputInfo* pi std::array perVertex {}; std::array interpolated {}; std::array halfRead {}; + std::array readsP10 {}; + std::array readsP20 {}; for (const auto& block : program.Blocks()) { for (const IrValue* inst : block->Instructions()) { if (inst->Opcode() == IrOpcode::GetAttribute) { @@ -298,7 +300,9 @@ void CollectPixelInputs(const IrProgram& program, const ShaderPixelInputInfo* pi const auto input = inst->Argument(0)->Resolve()->ImmediateU32(); const auto mode = inst->Argument(2)->Resolve()->ImmediateU32(); read[input] = true; - perVertex[input] = perVertex[input] || mode == 1u || !IsPixelParameterFlat(*pixel, input); + readsP10[input] = readsP10[input] || mode == 0u; + readsP20[input] = readsP20[input] || mode == 1u; + perVertex[input] = perVertex[input] || (readsP10[input] && readsP20[input]) || !IsPixelParameterFlat(*pixel, input); } else if (inst->Opcode() == IrOpcode::GetInterpolationParameterF16) { const auto input = inst->Argument(0)->Resolve()->ImmediateU32(); const bool high = inst->Argument(3)->Resolve()->ImmediateU32() != 0u; diff --git a/core/shader/recompiler/Recompiler.hpp b/core/shader/recompiler/Recompiler.hpp index 127ed88d5b21..23c4658d37ce 100644 --- a/core/shader/recompiler/Recompiler.hpp +++ b/core/shader/recompiler/Recompiler.hpp @@ -377,6 +377,7 @@ struct FragmentParameter { bool custom = false; bool flatP10 = false; bool vertexOrder = false; + bool flatP20 = false; bool operator==(const FragmentParameter&) const = default; }; diff --git a/core/shader/recompiler/ShaderDiskCache.cpp b/core/shader/recompiler/ShaderDiskCache.cpp index 912e93e8443c..2362ce871acd 100644 --- a/core/shader/recompiler/ShaderDiskCache.cpp +++ b/core/shader/recompiler/ShaderDiskCache.cpp @@ -504,6 +504,7 @@ void encodeArtifact(Writer& writer, const CompiledShaderArtifact& result) { out.Value(parameter.custom); out.Value(parameter.flatP10); out.Value(parameter.vertexOrder); + out.Value(parameter.flatP20); }); writer.Value(result.barycentricEmulation.active); writer.Value(result.barycentricEmulation.smooth); @@ -562,6 +563,7 @@ void decodeArtifact(Reader& reader, CompiledShaderArtifact& result) { in.Value(parameter.custom); in.Value(parameter.flatP10); in.Value(parameter.vertexOrder); + in.Value(parameter.flatP20); }); reader.Value(result.barycentricEmulation.active); reader.Value(result.barycentricEmulation.smooth); diff --git a/core/shader/recompiler/SpirvBackend/src/SpirvMemory/SpirvInputOutput.cpp b/core/shader/recompiler/SpirvBackend/src/SpirvMemory/SpirvInputOutput.cpp index 4c05ffe09434..645d3d167788 100644 --- a/core/shader/recompiler/SpirvBackend/src/SpirvMemory/SpirvInputOutput.cpp +++ b/core/shader/recompiler/SpirvBackend/src/SpirvMemory/SpirvInputOutput.cpp @@ -49,11 +49,15 @@ namespace ShaderRecompiler const auto& pixel = *inputInfo.pixel; std::vector result; std::array readsP10{}; + std::array readsP20{}; for (const auto& block : program.Blocks()) { for (const auto* instruction : block->Instructions()) { - if (instruction->Opcode() != IrOpcode::GetInterpolationParameter || instruction->Argument(2)->Resolve()->ImmediateU32() != 0u) continue; + if (instruction->Opcode() != IrOpcode::GetInterpolationParameter) continue; + const auto mode = instruction->Argument(2)->Resolve()->ImmediateU32(); const auto attribute = instruction->Argument(0)->Resolve()->ImmediateU32(); - if (attribute < readsP10.size()) readsP10[attribute] = true; + if (attribute >= readsP10.size()) continue; + readsP10[attribute] = readsP10[attribute] || mode == 0u; + readsP20[attribute] = readsP20[attribute] || mode == 1u; } } for (const auto& input : program.Info().inputs) { @@ -61,14 +65,16 @@ namespace ShaderRecompiler const auto location = PixelInputLocation(pixel, input.location); const bool flat = PixelInputIsFlat(pixel, input.location); const bool flatP10 = flat && !input.perVertex && input.location < readsP10.size() && readsP10[input.location]; + const bool flatP20 = flat && !input.perVertex && input.location < readsP20.size() && readsP20[input.location]; const bool vertexOrder = input.perVertex && (input.location >= 32u || (program.Metadata().pixelVertexOrderInputs >> input.location & 1u) != 0u); const auto existing = std::find_if(result.begin(), result.end(), [&](const auto& parameter) { return parameter.location == location; }); if (existing != result.end()) { existing->flatP10 = existing->flatP10 || flatP10; + existing->flatP20 = existing->flatP20 || flatP20; existing->vertexOrder = existing->vertexOrder || vertexOrder; continue; } - result.push_back({location, location, flat, input.perVertex, PixelInputIsCustom(pixel, input.location), flatP10, vertexOrder}); + result.push_back({location, location, flat, input.perVertex, PixelInputIsCustom(pixel, input.location), flatP10, vertexOrder, flatP20}); } return result; } diff --git a/docs/dev/debt/libSceAgcDriver.md b/docs/dev/debt/libSceAgcDriver.md index 40077e5d763b..a2628074c5dc 100644 --- a/docs/dev/debt/libSceAgcDriver.md +++ b/docs/dev/debt/libSceAgcDriver.md @@ -135,7 +135,7 @@ - [PA_CL_CLIP_CNTL](../../../core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp) (libSceAgcDriver) - `CLIP_DISABLE` (bit 16) and `DX_LINEAR_ATTR_CLIP_ENA` (bit 24) are accepted without changing pipeline state: Vulkan clips primitives against viewport guard bands and clips `NoPerspective` outputs linearly; what clearing bit 24 or setting bit 16 changes on the console beyond standard clipping is undocumented. - [10_11_11 unorm color targets](../../../core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp) (libSceAgcDriver) render into R32_UINT attachments: the pixel shader packs each export, rounded to nearest even like format-30 image stores, which is assumed to match the CB's conversion. - [11_11_10 float color targets](../../../core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp) (libSceAgcDriver) render into R32_UINT attachments: the pixel shader packs each export toward zero like format-43 image stores (overflow to the largest finite value, negatives to zero, NaN kept), which is assumed to match the CB's conversion; only the reversed component order (COMP_SWAP 2) is accepted. -- [PA_SU_SC_MODE_CNTL.PROVOKING_VTX_LAST](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp) (libSceAgcDriver) is not measured on the console: bit 19 follows opengnm's `flatshade_first` and Vulkan's vertex selection; rect lists take flat inputs from the third vertex (first mode: first); a geometry program gets odd strip triangle i as (i+1, i, i+2) in both modes (an RX 6700 XT, first mode: (i, i+2, i+1)); in last mode its primitives take flat inputs and layer from the last index, per-vertex ones in first-mode order; a flat input without pass-through read only through `v_interp_mov_f32` P10 gives the provoking vertex's value (ps5-opengl, #1413; first mode throws). Barycentric tessellation stages give each corner the last vertex's flat values. Else last mode throws without `VK_EXT_provoking_vertex`, for tessellation, geometry-program fans, explicit per-vertex flat inputs, and strips whose pixel shader reads a vertex slot (`v_interp_mov_f32`; 16-bit interpolation unchecked). +- [PA_SU_SC_MODE_CNTL.PROVOKING_VTX_LAST](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Pipeline.cpp) (libSceAgcDriver) is not measured on the console: bit 19 follows opengnm's `flatshade_first` and Vulkan's vertex selection; rect lists take flat inputs from the third vertex (first mode: first); a geometry program gets odd strip triangle i as (i+1, i, i+2) in both modes (an RX 6700 XT, first mode: (i, i+2, i+1)); in last mode its primitives take flat inputs and layer from the last index, per-vertex ones in first-mode order; a flat input without pass-through read only via `v_interp_mov_f32` P10 (P20) is the provoking vertex's (ps5-opengl, #1413; first (last) mode throws). Barycentric tessellation stages give each corner the last vertex's flat values. Else last mode throws without `VK_EXT_provoking_vertex`, for tessellation, geometry-program fans, explicit per-vertex flat inputs, and strips whose pixel shader reads a vertex slot (`v_interp_mov_f32`; 16-bit interpolation unchecked). - [Triangle strips through the barycentric tessellation stages](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp) (libSceAgcDriver) - without `geometryShader`, a triangle strip without primitive restart is expanded on the CPU into one three-point patch per triangle in Vulkan's strip order: (i, i+1, i+2), and for odd triangles (i, i+2, i+1), or (i+1, i, i+2) with `PROVOKING_VTX_LAST`. That is the order the geometry stage receives, so both routes give the same weights and per-vertex values; which order the console uses for odd triangles is not measured. Indirect strips throw. - [Dual-source blending](../../../core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp) (libSceAgcDriver) takes the MRT1 export as the second source of the MRT0 blend, as RADV and radeonsi program it; what the CB does with other export or target layouts is unknown, so they throw. - [Blending into reversed-order color targets](../../../core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp) (libSceAgcDriver) - 8_8_8_8 and 16_16_16_16 targets with `COMP_SWAP` 2 or 3 keep the exported alpha in their first channel and red or blue in their fourth, which Vulkan would blend with the color and the alpha equation respectively. The color block is taken to blend in shader export order and apply `COMP_SWAP` afterwards: the alpha equation and every source-alpha factor use the exported alpha, and the destination alpha is the first channel. This follows the `CB_TARGET_MASK` text of the AMD [SI](https://xorg.freedesktop.org/docs/AMD/old/SI_3D_registers.pdf) (p. 229) and [CIK](https://xorg.freedesktop.org/docs/AMD/old/CIK_3D_registers_v2.pdf) (p. 238) register references ("the order of components after blending and before COMP_SWAP is applied"). RADV builds `CB_BLENDn_CONTROL` without regard to the swap ([radv_pipeline_graphics.c](https://gitlab.freedesktop.org/mesa/mesa/-/blob/mesa-26.2.2/src/amd/vulkan/radv_pipeline_graphics.c#L650-L709)) and renders R4G4B4A4, B4G4R4A4, R5G5B5A1 and B5G5R5A1 with `SWAP_STD_REV` and `SWAP_ALT_REV` ([vk_format.c](https://gitlab.freedesktop.org/mesa/mesa/-/blob/mesa-26.2.2/src/vulkan/util/vk_format.c#L43-L48), [ac_formats.c](https://gitlab.freedesktop.org/mesa/mesa/-/blob/mesa-26.2.2/src/amd/common/ac_formats.c#L609-L624)). Mesa CI's Navi10 (gfx1010) CTS run ([job 112381553](https://gitlab.freedesktop.org/mesa/mesa/-/jobs/112381553), [112381554](https://gitlab.freedesktop.org/mesa/mesa/-/jobs/112381554)) passes all 400 `blend.format..states` cases of each of those formats, including separate alpha and destination-alpha factors. It is not measured on gfx1013 or on 8_8_8_8 and 16_16_16_16 targets. Factor pairs Vulkan applies per channel are used directly. Otherwise the pixel shader only copies values: it writes the exported alpha, each channel's own export or the constants 0 and 1 into a second color at location 0 index 1 (and 1 into a channel of the first color whose source factor is ONE, moving its export into the factor), and the blend forms every factor, `1 -` and product from those copies through `SRC1_COLOR`/`SRC1_ALPHA` factors. The blend unit therefore rounds them as it rounds a plain Vulkan blend of the same export; Vulkan does not require a second-source color to be converted like the first, but on an RTX 5080 and lavapipe every reversed-order case of the execution test is bit-identical to the same draw into a `SWAP_STD` target, which the test checks. Factor combinations that copies cannot express (for example `ONE_MINUS_SRC_ALPHA` in the alpha equation with `SRC_COLOR` in the color equation), sRGB, SNORM and integer targets, constant and second-source factors, destination-alpha and `SRC_ALPHA_SATURATE` factors in the color equation, different color and alpha operations, MIN or MAX with factors other than ONE, other color exports when the blend needs the second color, and devices without `dualSrcBlend` throw. Gran Turismo 7 (PPSA01317) blends `CB_BLEND0_CONTROL` 0x65010504 into such a target (#4782).