Fix not setting graphics pipeline primitive type for Metal

[tuzz]

This information is needed when a vertex shader outputs [[render_target_array_index]] for a 2D texture array.

The GPU needs to know the primitive topology at pipeline compile time to determine which vertex's array index value to use when routing fragments to the layered texture.

Fixes this pipeline creation error:

ERROR: Creating render pipeline failed: Error Domain=AGXMetal13_3 Code=3 "Vertex shader writes render_target_array_index but inputPrimitiveTopology is not specified" UserInfo={NSLocalizedDescription=Vertex shader writes render_target_array_index but inputPrimitiveTopology is not specified}

Existing Issue(s)

None found.

Other platforms

I haven't checked D3D12, Vulkan and XR because I'm only running macOS. Would someone be able to check other other GPU backends and apply the same patch there if needed? Thanks!

Reproduction

#include <SDL3/SDL.h>

static const char *VERT_MSL =
  "#include <metal_stdlib>\n"
  "using namespace metal;\n"
  "struct VSOut {\n"
  "    float4 position [[position]];\n"
  "    uint   layer    [[render_target_array_index]];\n"
  "};\n"
  "struct VSIn {\n"
  "    float2 position [[attribute(0)]];\n"
  "    uint   layer    [[attribute(1)]];\n"
  "};\n"
  "vertex VSOut vert_main(VSIn in [[stage_in]]) {\n"
  "    VSOut out;\n"
  "    out.position = float4(in.position, 0.0, 1.0);\n"
  "    out.layer    = in.layer;\n"
  "    return out;\n"
  "}\n";

static const char *FRAG_MSL =
  "#include <metal_stdlib>\n"
  "using namespace metal;\n"
  "struct FSIn {\n"
  "    float4 position [[position]];\n"
  "};\n"
  "fragment float4 frag_main(FSIn in [[stage_in]]) {\n"
  "    return float4(1.0, 1.0, 1.0, 1.0);\n"
  "}\n";

int main(int argc, char *argv[]) {
  SDL_Init(SDL_INIT_VIDEO);
  SDL_Window *window = SDL_CreateWindow("SDL3 Metal Repro", 640, 480, 0);
  SDL_GPUDevice *device = SDL_CreateGPUDevice(SDL_GPU_SHADERFORMAT_MSL, false, NULL);
  SDL_ClaimWindowForGPUDevice(device, window);

  SDL_GPUTexture *array_texture = SDL_CreateGPUTexture(device, &(SDL_GPUTextureCreateInfo){
    .type = SDL_GPU_TEXTURETYPE_2D_ARRAY,
    .format = SDL_GPU_TEXTUREFORMAT_R8G8B8A8_UNORM,
    .usage = SDL_GPU_TEXTUREUSAGE_COLOR_TARGET,
    .width = 500,
    .height = 500,
    .layer_count_or_depth = 4,
    .num_levels = 1,
    .sample_count = SDL_GPU_SAMPLECOUNT_1,
  });

  SDL_GPUShader *vert = SDL_CreateGPUShader(device, &(SDL_GPUShaderCreateInfo){
    .stage = SDL_GPU_SHADERSTAGE_VERTEX,
    .format = SDL_GPU_SHADERFORMAT_MSL,
    .code = (const Uint8 *)VERT_MSL,
    .code_size = SDL_strlen(VERT_MSL),
    .entrypoint = "vert_main"
  });

  SDL_GPUShader *frag = SDL_CreateGPUShader(device, &(SDL_GPUShaderCreateInfo){
    .stage = SDL_GPU_SHADERSTAGE_FRAGMENT,
    .format = SDL_GPU_SHADERFORMAT_MSL,
    .code = (const Uint8 *)FRAG_MSL,
    .code_size = SDL_strlen(FRAG_MSL),
    .entrypoint = "frag_main"
  });

  SDL_GPUGraphicsPipeline *pipeline = SDL_CreateGPUGraphicsPipeline(device, &(SDL_GPUGraphicsPipelineCreateInfo){
    .vertex_shader   = vert,
    .fragment_shader = frag,
    .primitive_type  = SDL_GPU_PRIMITIVETYPE_LINELIST,
    .vertex_input_state = {
      .num_vertex_buffers = 1,
      .vertex_buffer_descriptions = (SDL_GPUVertexBufferDescription[]){
        { .slot = 0, .pitch = sizeof(float) * 2 + sizeof(Uint32), .input_rate = SDL_GPU_VERTEXINPUTRATE_VERTEX },
      },
      .num_vertex_attributes = 2,
      .vertex_attributes = (SDL_GPUVertexAttribute[]){
        { .buffer_slot = 0, .location = 0, .format = SDL_GPU_VERTEXELEMENTFORMAT_FLOAT2, .offset = 0 },
        { .buffer_slot = 0, .location = 1, .format = SDL_GPU_VERTEXELEMENTFORMAT_UINT, .offset = sizeof(float) * 2 },
      },
    },
    .target_info = {
      .num_color_targets = 1,
      .color_target_descriptions = (SDL_GPUColorTargetDescription[]){
        { .format = SDL_GPU_TEXTUREFORMAT_R8G8B8A8_UNORM },
      },
    },
  });

  if (pipeline) {
    SDL_Log("PASS: pipeline created successfully (SDL3 is patched)");
    SDL_ReleaseGPUGraphicsPipeline(device, pipeline);
  } else {
    SDL_Log("FAIL (expected on unpatched SDL3): %s", SDL_GetError());
  }

  SDL_ReleaseGPUShader(device, vert);
  SDL_ReleaseGPUShader(device, frag);
  SDL_ReleaseGPUTexture(device, array_texture);
  SDL_ReleaseWindowFromGPUDevice(device, window);
  SDL_DestroyGPUDevice(device);
  SDL_DestroyWindow(window);
  SDL_Quit();
}

[TheSpydog]

It looks like layered rendering is only supported on A12 chips and above, which is above the min spec for SDL_GPU (A9+ devices).

Per Apple’s documentation it also looks like this feature requires specifying a renderTargetArrayLength, which is not included in the PR.

[flibitijibibo]

From an outside perspective (having little knowledge of the Metal side of things) what specifically does this do that we don't already do to support primitive types in Metal overall? This is important for others to understand since we would presumably need to port this feature to Vulkan, D3D12, and PS4/5, and even knowing how those backends work I can't quickly figure out what this is doing and why it's doing it, other than it simply being possible for the Metal API to accept for some reason.

Once we have a better understand of the above it should be easier to see how this applies to the other backends and what restrictions might apply (other than the system requirements that @TheSpydog already mentioned)

[tuzz]

From my perspective, I just followed the error trail and noticed we weren't setting the inputPrimitiveTopology (as per the error message) but I don't understand the broader consequences of adding this change. Maybe it only helps in the specific circumstance I encountered of trying to use 2D texture arrays? It doesn't seem detrimental to set this information in the Metal API given we have it available. I can try to set renderTargetArrayLength as well if we want to go ahead with this change.

[slouken]

@flibitijibibo, I don't think anything needs to be done for the other backends, we just weren't specifying the GPU primitive type correctly. @tuzz, please add setting renderTargetArrayLength to this PR.

[flibitijibibo]

Huh, so we weren't setting it anywhere? So point and line rendering just didn't work at all until now...? (I could have sworn we tested all types as part of the SDL_Render backend...)

[slouken]

Huh, so we weren't setting it anywhere? So point and line rendering just didn't work at all until now...? (I could have sworn we tested all types as part of the SDL_Render backend...)

The SDL_Renderer backend doesn't exercise render_target_array_index functionality.

[TheSpydog]

@flibitijibibo In Metal, the primitive type is passed in to draw commands rather than on the pipeline itself. You only need to provide it to the pipeline if you’re doing layered rendering, which is what this PR is about.

I haven’t yet investigated what capabilities might be necessary to support layered rendering on other backends, but I can definitely imagine this being a Vulkan extension or device feature.

[tuzz]

I've tried adding renderTargetArrayLength. Building here: https://github.com/tuzz/SDL/actions/runs/22870114197

More docs here:
https://developer.apple.com/documentation/metal/mtlrenderpassdescriptor/rendertargetarraylength

[flibitijibibo]

Looks like this also needs an @available check for iOS/tvOS

[TheSpydog]

@tuzz What was the motivation for using render_target_index in a vertex shader?

It looks like support for this feature is very spotty on Vulkan (VK_EXT_shader_viewport_index_layer has 41.39% support according to gpuinfo) and there may be performance penalties for using it on some D3D12 hardware (see the VPAndRTArrayIndexFromAnyShaderFeedingRasterizerSupportedWithoutGSEmulation feature).

Since this would raise our min spec even on Metal, I'm inclined to avoid adding this feature, at least not without significantly more investigation and design consideration. But if there's a very compelling use case, we'll see what we can do.

[tuzz]

I don't mind closing this. My use case was for lightmap generation. I was using a compute shader to raytrace some lines for light geometry and needed to partition the vertices into separate texture layers, with additive blending.

I can workaround it in a few differents ways, e.g. adding a conditional to the vertex shader and issuing separate draw calls or by moving more of the processing into the compute shader. So it's definitely not a blocker for me.

[thatcosmonaut]

First off, the PR title is misleading, the problem here is that an unsupported shader feature is being used.

The concept being used here is known as "Single Draw Multiple Viewport".

In Vulkan this is covered by VK_EXT_shader_viewport_index_layer which was promoted to Core in 1.2 with about 87% hardware support and 80% on Android. In D3D12 it is covered by the VPAndRTArrayIndexFromAnyShaderFeedingRasterizerSupportedWithoutGSEmulation (that's a mouthful!) feature.

This feature actually is quite useful for a common use case (point light shadow mapping) but I think we should have a lot more discussion about this before officially supporting it in some way.

EDIT: Looks like Caleb got to this first, but thankfully we came to the same conclusion...

[slouken]

I'm going to close this for now, because it looks offhand like a bug fix and I don't want to accidentally merge it, but we can keep this in mind for the future!