Skip to content

Conversation

adamwych
Copy link
Contributor

@adamwych adamwych commented Aug 22, 2025

Not finished, but it does work. I started adding support for the compute pipeline, so I can play around and implement a GPU-driven renderer - might as well make a PR.

(I tried to make the API feel similar to how the graphics pipeline works.)

ToDo:

  • Documentation
  • SDL_GPUComputePipelineCreateInfo options: props

@adamwych adamwych force-pushed the feat/compute-pipeline branch 2 times, most recently from 119d1c9 to 8f27022 Compare August 22, 2025 18:52
@lithiumtoast
Copy link
Member

@adamwych Hey, thanks for the interest. I understand that compute is something that is of interest for people as one of major attractions of using SDL_GPU compared to other rendering libraries.

I took a look at this PR as it is. I've been working on compute also these past few weeks to add the example https://github.com/TheSpydog/SDL_gpu_examples/blob/main/Examples/BasicCompute.c

I pushed my changes so far (not finished yet) to dev branch here: 3342fef.

I'll compare and contrast to your MR and discuss more here as I go.

@adamwych
Copy link
Contributor Author

Okay. I see you've chosen to merge the compute shader with the compute pipeline into one class. I kept it separate, since that's how it works on the lower level, but I can update the code to work like that too. Looking at my user code it seems like it will make it somewhat cleaner too.

@lithiumtoast
Copy link
Member

I see you've chosen to merge the compute shader with the compute pipeline into one class.

Yeah, looking at the SDL_GPU example for compute it appears that SDL_GPUComputePipelineCreateInfo used by SDL_CreateGPUComputePipeline is very similar to the graphics shader equivalent. I think at a higher level in the idiomatic C# layer it makes more sense to use word "shader", at least for now. We will see how it goes and re-evaluate as idiomatic C# layer becomes more complete.

@lithiumtoast
Copy link
Member

Hey @adamwych

I made some progress on the dev branch. The basic compute found in the SDL_GPU examples repository works as expected. This is just the minimum code to support that example.

I was wrestling with passing two arrays with BeginComputePass. I wanted it to be just one struct that is passed in. I ended up adding a new abstraction called NativeArray which can be created from a Span<T>. Worked out quite well as now there is no stack allocations or additional iterating.

I'll be looking next into folding NativeArray upstream into C2CS.Runtime. Then pulling that down here in SDL3-cs as generated code or a NuGet package. When that's finished I'll merge dev into main to which after you could rebase this against main for the things which are not yet completed then.

@adamwych
Copy link
Contributor Author

adamwych commented Sep 4, 2025

Any reason why GpuComputePassParameters can't just be a ref struct? It could then store Spans directly, without need for NativeArray.

@lithiumtoast
Copy link
Member

Any reason why GpuComputePassParameters can't just be a ref struct? It could then store Spans directly, without need for NativeArray.

  1. record struct can't be used, not a huge deal breaker but it would be nice if record structs could be used to leverage automatically generated Equals method and so on.

  2. By using ref we force GpuComputePassParameters to be from the stack. Kind of annoying and limits the API for the developer on how it can be used but again not a deal breaker.

  3. It doesn't work for the following method to use params Span<T>.

    public Span<GpuComputePassBindingTextureReadWrite> TextureWriteBindingsArray;
    ...
    public void SetBindingsTextureWrite(
        params Span<GpuComputePassBindingTextureReadWrite> textureWriteBindings)
    {
        TextureWriteBindingsArray = textureWriteBindings;
    }

Error:

GpuComputePassParameters.cs(31, 37): [CS8352] Cannot use variable 'params Span<GpuComputePassBindingTextureReadWrite> textureWriteBindings' in this context because it may expose referenced variables outside of their declaration scope

I find this one to be the deal breaker because using params Span<T> really simplifies the API where the it will automatically stack alloc the span for the developer; the developer just needs to pass one or more GpuComputePassBindingTextureReadWrite structs.

@adamwych
Copy link
Contributor Author

adamwych commented Sep 4, 2025

  1. By using ref we force GpuComputePassParameters to be from the stack.

Since NativeArray takes the pointer through a fixed statement, this requirement already exists, it's just not enforced by the compiler. Consider this example:

private GpuStorageTextureReadWriteBinding[] _bindingsSomewhereOnHeap;

private void Foo() {
    var computePassParameters = default(GpuComputePassParameters);
    computePassParameters.SetBindingsTextureWrite(_bindingsSomewhereOnHeap);

    // <- if GC happens to run here, between those calls - `_bindingsSomewhereOnHeap` might be moved to a different location and the pointer inside `NativeArray` points to invalid data

    var computePass = commandBuffer.BeginComputePass(computePassParameters);
}

It seems to me like NativeArray does basically the same thing as Span, but doesn't enforce any security, so now it's up to the developer to ensure safety.

@lithiumtoast
Copy link
Member

I would be inclined to say that's on the developer to maintain that the pointer is valid for the call to the native function.

Also how are you calling the method BeginComputePass then?

If I change the struct to:

public ref struct GpuComputePassParameters
{
    public Span<GpuComputePassBindingTextureReadWrite> TextureWriteBindingsArray;
    public Span<GpuComputePassBindingDataBufferReadWrite> DataBufferWriteBindingsArray;
}

Then try to call the method like so:

        var computePassParameters = default(GpuComputePassParameters);
        Span<GpuComputePassBindingTextureReadWrite> textureWriteBindingsArray = stackalloc GpuComputePassBindingTextureReadWrite[1];
        textureWriteBindingsArray[0].SetTexture(_texture);
        computePassParameters.TextureWriteBindingsArray = textureWriteBindingsArray;

        var computePass = commandBuffer.BeginComputePass(computePassParameters);

I get a compile time error:

  E011_BasicCompute.cs(134, 59): [CS8352] Cannot use variable 'textureWriteBindingsArray' in this context because it may expose referenced variables outside of their declaration scope

@adamwych
Copy link
Contributor Author

adamwych commented Sep 4, 2025

I would be inclined to say that's on the developer to maintain that the pointer is valid for the call to the native function.

Doesn't that defeat the purpose of providing a more idiomatic API? Unless someone looks at what SetBindingsTextureWrite and NativeArray do internally, they wouldn't even know that it captures a pointer to the array.

I'm also not sure if there is a clear way for a dev to ensure that the pointer is valid other than by than creating a local copy of the entire array, using GCHandle or creating another fixed statement, all of which don't seem very intuitive.

Also how are you calling the method BeginComputePass then?

In my proposal there was no need for separate variables, you can just use it like so:

var computePass = commandBuffer.BeginComputePass(
    new GpuComputePassParameters
    {
        StorageTextureBindings = [],
        StorageBufferBindings =
        [
            new() { Buffer = _drawCommandsBuffer },
            new() { Buffer = _meshDataBuffer },
            // ...
        ]
    }
);

(Note that this requires copying in BeginComputePass to create the actual SDL_GPUStorageTextureReadWriteBinding, since the structures don't match. There might be a way to make them compatible, though I would need to think harder about how to do that exactly)

Generally I'm all for removing the need to create copies in BeginComputePass and also making GpuComputePassParameters non-ref, since in many cases they probably won't change frame-by-frame, but SetBindingsTextureWrite just gives a false sense of security by providing "safe" C# API that uses Spans while actually doing unsafe stuff under the hood and pushing responsibility to ensure safety onto the user. It's up to you though.

@lithiumtoast
Copy link
Member

In my proposal there was no need for separate variables, you can just use it like so

OK this wasn't in this MR and is new information to me but nice. I did not know one could use spans like that inline. I would say that then this is most likely the best way to get about doing it. I'll try it out.

@lithiumtoast
Copy link
Member

Yup, that works great 👍 , updated dev branch to use this. I don't need NativeArray then directly as mentioned.

@lithiumtoast
Copy link
Member

I merged dev in to main. You can rebase this MR if you want. I'll stop using dev branch moving forward and do more independent PRs.

@adamwych adamwych force-pushed the feat/compute-pipeline branch 5 times, most recently from c078649 to 4b07205 Compare September 9, 2025 03:12
@adamwych
Copy link
Contributor Author

adamwych commented Sep 9, 2025

I updated the PR with main. A couple of things still need some attention:

  • GpuCommandBuffer.Push(Vertex/Fragment/Compute)ShaderUniformData as a generic replacement for specific PushFragmentShaderUniformColor and PushVertexShaderUniformMatrix.
  • GpuComputePass.DispatchIndirect - GpuDataBuffer is untyped, so there's no way to enforce that it matches the type SDL expects... but even if it was generic, the bindings generator seems to skip SDL_GPUIndirectDispatchCommand struct (I guess because it is not directly used by any function, so it's optimized out).
  • What about SDL_GPUComputePipelineCreateInfo.props? I don't see any safe API for SDL properties struct, so I guess it must be skipped for now.

@adamwych adamwych force-pushed the feat/compute-pipeline branch from 4b07205 to 33fc459 Compare September 9, 2025 03:33
@lithiumtoast
Copy link
Member

GpuCommandBuffer.Push(Vertex/Fragment/Compute)ShaderUniformData as a generic replacement for specific PushFragmentShaderUniformColor and PushVertexShaderUniformMatrix.

👍

GpuComputePass.DispatchIndirect - GpuDataBuffer is untyped, so there's no way to enforce that it matches the type SDL expects... but even if it was generic, the bindings generator seems to skip SDL_GPUIndirectDispatchCommand struct (I guess because it is not directly used by any function, so it's optimized out).

There is only offset on the API function call for SDLCALL SDL_DispatchGPUComputeIndirect? No count to say when to stop reading from the buffer? I guess it just reads until the end of the buffer starting from offset.
I think it's safe to assume that the developer has to make sure the buffer has the correct size/stride. Yes since SDL_GPUIndirectDispatchCommand is hanging (not used by any SDL function declaration directly) it is left out from automatic bindgen.

What about SDL_GPUComputePipelineCreateInfo.props? I don't see any safe API for SDL properties struct, so I guess it must be skipped for now.

SDL properties should be handled internally by C# idiomatic API. See internal GpuDevice(ILogger<GpuDevice> logger, GpuDeviceOptions? options) for example. I'm not sure how they can be used for SDL_GPUComputePipelineCreateInfo. If it doesn't make sense at this time they can be skipped for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants