Repository navigation
fix(agc): draw a last provoking vertex that no pixel input sees without the extension - #6741
DavidSamir wants to merge 14 commits into
Conversation
…ut the extension A draw with PA_SU_SC_MODE_CNTL.PROVOKING_VTX_LAST threw on a device without VK_EXT_provoking_vertex, whatever its pixel shader reads. MoltenVK 1.4.2 on Apple M2 does not expose the extension, and ProsperoLichess (PPSA99009) stops there on macOS: AGC graphics: last provoking vertex requires VK_EXT_provoking_vertex with provokingVertexLast enabled The provoking vertex selects the values of flat-shaded inputs and the order of per-vertex values, and nothing else the driver uses: no transform feedback is recorded. The draw Lichess stops on is a triangle fan whose pixel shader has no inputs at all. Without the extension, last mode now throws only when a pixel shader could see the provoking vertex: a flat input, a flat P10 read, or a per-vertex or custom per-vertex input as the recompiler records them, a Flat or PerVertexKHR decoration in its SPIR-V (a shader built without that metadata, or a member of an input block), a module that is truncated, or a pixel shader that was not compiled. Otherwise the pipeline is created without the provoking vertex state, so the device draws in first-vertex mode, as it already did for a draw without a pixel shader. Devices with the extension are unchanged. The existing check that a draw without shaders throws on such a device now expects it to pass.
…nsion agc_driver_unobserved_provoking_vertex runs, for points, lines, line strips, triangle lists, strips and fans on a device without VK_EXT_provoking_vertex: - draws without a pixel shader, with a pixel shader without inputs and with only a smooth input, which pass; - a pixel shader with a flat input, a flat P10 read, a per-vertex or a custom per-vertex input, a Flat or PerVertexKHR decoration on a variable or a block member, a truncated module, no module, or none compiled, which throw; - a vertex-stage program carrying such metadata and decorations beside a smooth pixel shader, which passes, since only the pixel shader's inputs are read. It also reruns the existing provoking vertex checks.
|
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.
|
…rovoking vertex check Without VK_EXT_provoking_vertex, a draw in last-vertex mode threw when its pixel shader's SPIR-V had any Flat or PerVertexKHR decoration. Vulkan requires Flat on integer pixel inputs, built-ins included, so a shader reading gl_SubgroupInvocationID has one, although the pixel stage generates that value itself and no vertex supplies it. Cloud Play (PPSA99600, a triangle list with no inputs) and PS5 OpenGL Showcase (PPSA99005, a triangle fan with one smooth input) stop there on macOS: their only Flat decoration is on a BuiltIn SubgroupLocalInvocationId input. The check now reads BuiltIn decorations on variables and block members too, and skips a Flat or PerVertexKHR decoration on FragCoord, PointCoord, FrontFacing, SampleId, SamplePosition, SampleMask, HelperInvocation, SubgroupSize or SubgroupLocalInvocationId. Every other built-in, Layer, ViewportIndex and PrimitiveId among them, still counts as seeing the provoking vertex.
agc_driver_unobserved_provoking_vertex now passes a Flat SampleId, FrontFacing or SubgroupLocalInvocationId, as a variable in either decoration order or as a block member, and still refuses a Flat input next to an unrelated built-in, a Flat block member next to a built-in member, and a Flat Layer, ViewportIndex or PrimitiveId.
|
Windows run, since Windows is untested. Windows 11, WinLibs GCC 15.2.0 r7 (MinGW), Release,
AI-assisted: yes (Claude Code) |
… no pixel input sees without the extension (@DavidSamir) Open upstream PR that adds macOS or Metal behaviour this branch did not have; merged so the fork carries the community's current macOS work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… no pixel input sees without the extension (@DavidSamir) Open upstream PR that adds macOS or Metal behaviour this branch did not have; merged so the fork carries the community's current macOS work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st-provoking-vertex # Conflicts: # docs/dev/debt/libSceAgcDriver.md
Keeps the macOS port current with 50 upstream commits. - Upstream's boykopovar#7487 keeps the barycentric tessellation stages' clip distances in the blocks between the stages, which MoltenVK's SPIRV-Cross translates; it replaces the fork's MoltenVK-only switch (clipDistancesInPerVertexBlock), so the switch, its device query and its debt note are removed and every driver takes upstream's stages. - The provoking vertex debt entry takes upstream's geometry program and strip facts and keeps the fork's rule for devices without VK_EXT_provoking_vertex (boykopovar#6741). - The f16 interpolation debt entries keep the fork's 64-bit integer text and take upstream's clip distance description. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st-provoking-vertex # Conflicts: # docs/dev/debt/libSceAgcDriver.md
adriadam10
left a comment
There was a problem hiding this comment.
Thanks, the approach and the evidence are good. One gap before this can merge:
Pipeline.cpp:113-157: only the fragment stage is inspected, but the provoking vertex also selects which vertex suppliesLayerandViewportIndex(Vulkan spec, primitive layer/viewport selection), and that happens whether or not the pixel shader reads anything. The recompiler emits both as vertex-stage outputs (StageOutputKind::Layer/ViewportIndex,SpirvEmitterHelpers.cpp:396-399). A vertex shader writing a per-vertexgl_Layerorgl_ViewportIndexwith a pixel shader without inputs now passesValidateProvokingVertexand renders into the first vertex's layer/viewport instead of the last's, silently.- Fix direction: also treat a non-fragment stage whose SPIR-V decorates
LayerorViewportIndexasBuiltInoutputs (a variable or block member) as observing the provoking vertex, so last mode still throws there without the extension. Add cases toUnobservedProvokingVertexTests(vertex stage withBuiltIn Layer/ViewportIndex, and your existing "vertex-stage metadata beside a smooth pixel shader" case must still pass without those built-ins), and mention it in the debt entry.
…hrough the barycentric tessellation stages (@DavidSamir) Apple GPUs under MoltenVK have neither geometry shaders nor VK_EXT_provoking_vertex, so a last-provoking-vertex draw that needs the barycentric emulation stopped on macOS; DavidSamir's PR draws it through the generated tessellation stages instead. In Pipeline.cpp its early return for those draws sits before this branch's early return for draws no pixel input sees (boykopovar#6741), and both stay. The barycentric emulation test takes the PR's flat and provoking parameters and keeps this branch's built-in switch after them. The debt files take the PR's entries and keep this branch's short MoltenVK ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… no pixel input sees without the extension (@DavidSamir) This branch already carried an earlier head of this PR; DavidSamir has pushed newer commits since, and this merge brings the branch up to them. The only conflict is the provoking vertex debt entry, where the branch already holds boykopovar#7426's entry and its own short entry for the draws no pixel input sees; both are kept as they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st-provoking-vertex # Conflicts: # core/libs/prx/libSceAgcDriver/CMakeLists.txt
|
Title evidence for #6741 and #7426 together, from a macOS catalog sweep: Apple M2, MoltenVK 1.4.x (no On that base, 12 catalog titles stop at
So on a device without the extension these titles need both changes: #6741 for their last-vertex draws whose pixel shader cannot see which vertex provokes, and #7426 for the ones whose pixel shader reads emulated barycentrics through the generated tessellation stages. Either alone still stops at the same check. Nothing past 30 s is tested; MoltenVK-specific limits (#6235, #6528) are not in the way of these 11. The two PRs touch the same function ( AI-assisted: yes (Claude Code) |
|
Validation-layer comparison, as AGENTS.md now asks for AGC changes. Apple M2, MoltenVK 1.4.x (Vulkan SDK 1.4.363), macOS 26, x86-64 under Rosetta 2, same machine and build directory, on
No new validation errors with the two changes. The title results for the pair are in the comment above (11 catalog titles run past the provoking-vertex stop with both). AI-assisted: yes (Claude Code) |
|
No-regression run on a GPU with The nine titles listed for the Mac all run the full 60 s on both builds: SymphonyStation5 (PPSA17641), VITA5 (PPSA39410), Signal Companion (PPSA69426), PokeMMO (PPSA98001), ProsperoLight (PPSA99002), PS5LM (PPSA99575), Cloud Play (PPSA99600), PShare (PPSA99773) and Pulse (PPSA99993). None stops early. This device takes the extension's native path, so this run checks only that nothing regresses. The emulated path is covered by the Mac runs in the description. AI-assisted: yes |
…st-provoking-vertex
…ng the provoking vertex
…oking vertex throwing without the extension
|
@adriadam10 thanks, you're right. Addressed in 507e6b5..3684f25:
Validation-layer comparison with AI-assisted: yes |
|
Apple M2 check of head The 11 titles that need the two provoking-vertex PRs still run the full 40 s with no AI-assisted: yes |
|
@adriadam10 could you take another look when you have a moment? On 3684f25 your Layer/ViewportIndex point looks addressed to us: the non-fragment stages that decorate |
What
A draw with
PA_SU_SC_MODE_CNTL.PROVOKING_VTX_LASTthrows on a device withoutVK_EXT_provoking_vertex, whatever its pixel shader reads. MoltenVK 1.4.2 on Apple M2 doesn't expose the extension (vulkaninfolists neither it norprovokingVertexLast). Once #6535 clears its viewport-depth stop, ProsperoLichess (PPSA99009) stops here on macOS:A temporary log line before the refusal showed the draw: a triangle fan on the vertex path whose pixel shader has no inputs (
fragmentParametersempty). The provoking vertex selects the values of flat-shaded inputs and the order of per-vertex values. The driver records no transform feedback, so nothing else depends on it.Without the extension, last mode now throws only when a pixel shader could see the provoking vertex:
FlatorPerVertexKHRdecoration in its SPIR-V, on a variable or a block member. This also catches a shader built without the recompiler's metadata, as inagc_driver_provoking_vertex. It skips a built-in the pixel stage generates itself:FragCoord,PointCoord,FrontFacing,SampleId,SamplePosition,SampleMask,HelperInvocation,SubgroupSizeandSubgroupLocalInvocationId. VUID-StandaloneSpirv-Flat-04744 requiresFlaton every integer pixel input, built-ins included, but no vertex supplies those.Layer,ViewportIndex,PrimitiveIdand every other built-in still count;LayerorViewportIndexbuilt-in output (variable or block member). Vulkan takes the layer and viewport from the provoking vertex whether or not the pixel shader reads anything. One also decoratedPerPrimitiveEXT, the mesh path's per-primitive layer, is skipped, because it belongs to the primitive. The recompiler declares these outputs only when the guest exports them (PA_CL_VS_OUT_CNTL), so other vertex shaders are unaffected. A stage whose module can't be read also counts.Otherwise
ValidateProvokingVertexaccepts the draw. ThePipelineconstructor already leaves out the provoking vertex state on such a device, so it draws in first-vertex mode, as it already did for a draw without a pixel shader. Devices with the extension are unchanged. ThePROVOKING_VTX_LASTdebt entry says when last mode throws.Commits:
Flatdecoration is on aBuiltIn SubgroupLocalInvocationIdinput.main.LayerorViewportIndexobserves the provoking vertex.Tested
macOS 26.7.1, Apple M2, MoltenVK 1.4.2 from the LunarG Vulkan SDK 1.4.363, AppleClang 21.0.0, x86-64 under Rosetta, Release.
maindoesn't build the system libraries on macOS by itself, so the tests ran in a local tree:maince991f1 with #4627, #4630, #6124, the four macOS relinker PRs and mugurc's macOS port branches merged, and #6535 applied. This branch'sPipeline.cppis identical to the one tested.agc_driver_unobserved_provoking_vertexpasses 5 runs of 5. It covers points, lines, line strips, triangle lists, strips and fans on a device without the extension:It also reruns
ProvokingVertexTests.With
main'sPipeline.cppthe new test fails. It also fails for each of 12 deliberately wrong versions of the fix, each dropping one of the conditions above or reading every stage's inputs.agc_driver_provoking_vertex(GPU), run with the SDK's loader on the path (as plainctestit skips: nolibvulkan.1.dylibfound), passes 3 runs of 3 on MoltenVK. It checks first-vertex rendering and that its flat GLSL shader is still refused in last mode. An earlier version of this branch read only the recompiler's metadata, drew that shader and failed there, which is why the SPIR-V check is in; the two SPIR-V variants among the 12 above fail it too.All 597
agc_tests in this tree, with this branch and withmain'sPipeline.cpp:agc_write_trackingandagc_shared_writefail with both;agc_driver_graphicsand the new test fail only withmain's;Cloud Play (PPSA99600, 1.0.0) and PS5 OpenGL Showcase (PPSA99005, 1.0.2), both with catalog sha256 checked, 40 s runs:
OpDecorate %10 Flaton%10, aBuiltIn SubgroupLocalInvocationId32-bit unsigned input. Cloud Play's draw is a triangle list with no recorded inputs; Showcase's is a triangle fan with one smooth input;Barycentric geometry SPIR-V: geometry shaders are unavailable(Macos m4 #6222's area). Showcase stops on64-bit float instructions need the Float64 capability, whose only 64-bit operation here isFPInterpolateF16.The built-in cases fail for each of 4 more wrong versions: no built-in skipped, every built-in skipped, block members keyed as variables, and a built-in on one variable excusing another.
ProsperoLichess (PPSA99009, catalog release 01.000.000), 60 s runs:
viewport depth outside [0, 1] requires VK_EXT_depth_range_unrestricted. A log line showed that draw has depth −1..1 with a depth target bound, which fix(agc): normalize unused viewport depth ranges #6535 leaves alone and feat(agc): remap a viewport depth range the device cannot express into the vertex shader #4650 addresses. feat(agc): remap a viewport depth range the device cannot express into the vertex shader #4650 conflicts withmain, so I couldn't add it to this tree.tools/check_conventions.py --base origin/mainandgit diff --checkpass.After review (commits 7 to 11). Windows 11 Pro 26300, NVIDIA RTX 3070 (driver 610.60), WinLibs GCC 15.2.0, Release; and WSL Ubuntu 24.04 with Mesa lavapipe, on
main02fb1ce merged in.New cases in
agc_driver_unobserved_provoking_vertex, for vertex, tessellation-evaluation, geometry and mesh stages beside a smooth pixel shader on a device without the extension:BuiltIn LayerorBuiltIn ViewportIndexon a variable or block member, including one wherePerPrimitiveEXTis on a different member: these throw. With the extension they pass;PerPrimitiveEXT, in either order: these pass;PositionandPointSizeonly: these pass;The existing case, vertex-stage metadata and
Flatbeside a smooth pixel shader, still passes.With commit 8 (no fix) and the new cases, the test fails with
expected graphics rejection: last provoking vertex requires VK_EXT_provoking_vertex with provokingVertexLast enabled. With commit 9 it passes.Each of commits 8, 9 and 10 builds, and
agc_driver_unobserved_provoking_vertex,agc_driver_provoking_vertex,agc_driver_mesh_provoking_layerandagc_driver_mesh_provoking_vertexpass on the RTX 3070.Validation (as AGENTS.md asks for AGC changes):
ctest -R provoking -Vonmainand on this head, same machine, lavapipe ICD, Khronos validation layer 1.3.275, withANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation.was not found but was requested by env var VK_INSTANCE_LAYERS.VUID-VkDeviceCreateInfo-pNext-pNext, the 1.3.275 layer not knowing a newer device-create struct. Nothing new from this change.Commits 1 to 6 were not tested on Windows, Linux or devices with the extension. Their path is unchanged, since the new early return needs
provokingVertexLastto be false. Commits 7 to 11 were tested on Windows and Linux, as below.Limits of this Mac's GPU.
vulkaninfoon Apple M2 with MoltenVK 1.4.2 reports no geometry shaders, noshaderFloat64, noVK_EXT_depth_range_unrestrictedand noVK_EXT_provoking_vertex. AnyPS5 turns MoltenVK'sVK_KHR_fragment_shader_barycentricoff (5333a20), so barycentric pixel shaders need the geometry-shader fallback. These are limits of the device. Apart from the provoking-vertex case below, this PR doesn't remove them. A title named here as stopping on one of them is not tested past that stop:Barycentric geometry SPIR-V: geometry shaders are unavailable: no geometry shaders; Macos m4 #6222 has a route around it.viewport depth outside [0, 1] requires VK_EXT_depth_range_unrestricted: feat(agc): remap a viewport depth range the device cannot express into the vertex shader #4650 works around it.last provoking vertex requires VK_EXT_provoking_vertex: this PR removes it only for draws whose pixel shader can't observe the provoking vertex. A draw that does observe it still stops.Overlap: #6451 and #6683 register tests next to
agc_driver_covered_centroid_stateinCMakeLists.txt, as this does; whichever lands second rebases. The overlap check couldn't compare the pull requests that conflict withmain. Of those, #1896, #5925 and #6222 editPipeline.cpp, and none of them changesValidateProvokingVertexor the provoking vertex state. I found no open pull request that changes this check.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)