Repository navigation
Convert triangle fans from the effective primitive topology - #2851
Open
carbonimax wants to merge 2 commits into
Open
carbonimax wants to merge 2 commits into
carbonimax wants to merge 2 commits into
Conversation
The render state only kept the Metal primitive type, which cannot tell a triangle fan from a triangle list. It now also keeps the Vulkan topology, static from the pipeline or dynamic from vkCmdSetPrimitiveTopology, and the encoder state reads the effective one through getPrimitiveTopology(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The draw paths decided whether to convert a triangle fan to triangles from the topology the pipeline was created with, ignoring a primitive topology set with vkCmdSetPrimitiveTopology. A pipeline created with another triangle topology drew a dynamic triangle fan as a triangle list, and a pipeline created as a triangle fan drew a dynamic triangle list or strip through the fan conversion. The four draw paths now read the effective topology from the encoder state, and the pipeline accessor they used, left without callers, is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
carbonimax
force-pushed
the
fix-dynamic-topology-fans
branch
from
October 9, 2026 12:20
aa9133c to
368e50d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MoltenVK draws triangle fans by converting them to indexed triangles, but the draw paths decided whether to convert from the topology the pipeline was created with. With
VK_DYNAMIC_STATE_PRIMITIVE_TOPOLOGY, a pipeline created as a triangle list or strip drew a dynamic triangle fan as a triangle list, and a pipeline created as a triangle fan drew a dynamic triangle list or strip through the fan conversion.The render state now keeps the Vulkan topology next to the Metal primitive type, which cannot tell a fan from a list, and the four draw paths read the effective topology from the encoder state. The pipeline accessor they used has no caller left and is removed.
Testing: a standalone reproducer (https://gist.github.com/carbonimax/6737e3169ba9cc2254e5343a5111934d) draws each topology of a class through one dynamic pipeline and compares the image and the vertex shader invocation count with one static pipeline per topology, in direct, indexed, indirect and indexed indirect forms, under Metal API and GPU validation. Before this change, the 8 triangle cases fail and the 4 line cases pass; after it, all 12 pass. Tested on Apple M4 Pro, macOS 27.2, on top of a63d486, with MoltenVK built with CMake (Debug, arm64) and
MVK_USE_METAL_PRIVATE_APIoff. The change also builds withMVK_USE_METAL_PRIVATE_APIon, but that configuration was not run on the GPU. Not tested on Intel Macs.🤖 Generated with Claude Code