[.Net & BindingGenerator] Generate `ReadOnlySpan<T>` Overloads for GodotSharp APIs
By adding ReadOnlySpan<T> API overloads, this PR aims to make the overall GodotSharp API more flexible by addressing the need for a dedicated data transaction T[] when making API calls.
Using the example from this proposal, the original RenderingServer.MultimeshSetBuffer
public static void MultimeshSetBuffer(Rid multimesh, float[] buffer)
will have an additional overload that takes ReadOnlySpan<float>:
public static void MultimeshSetBuffer(Rid multimesh, ReadOnlySpan<float> buffer)
Bugsquad edit:
- Closes https://github.com/godotengine/godot-proposals/issues/10012.
Does this also include to add overloads which take a buffer span instead of returning an Array?
Currently one of my biggest performance issues are tied to GetBufferData in the RenderingDevice.BufferGetData. Internally most of these methods act on a pinned array which is created every single time you call this method.
For RenderingDevice.BufferGetData the array is a byte[] which is incompatible with most of the other Apis so you have to allocate yet another array (float[]) and memory copy again. Doing this allows to use pooled arrays / instances and avoid allocations alltogether
I have modified my PR to fit the compatibility requirements; now that span-related APIs are generated as overloads, no spans are returned by public APIs.
@dotlogix In my original unmodified PR, yes, most arrays are directly replaced into Spans, including the return value. But for compatibility reasons, we can't do this, not on the main branch.
@dotlogix In my original unmodified PR, yes, most arrays are directly replaced into Spans, including the return value. But for compatibility reasons, we can't do this, not on the main branch.
Guess you forgot to revert: GD.VarToBytes Compat.*
I don't mean return a ReadOnlySpan<T> instead of a T[] but instead of creating an array and return it, take a Span<T> and write to that instead.
Guess you forgot to revert: GD.VarToBytes
Good catch, thanks!
I don't mean return a ReadOnlySpan instead of a T[] but instead of creating an array and return it take a Span and write to that instead.
Oh, do you mean to return a Span for developers to write into, like a temporary buffer? As you mentioned, that would work by utilizing an array pool, but it would be challenging to manage the lifetime of the rented arrays. It sounds like a job for a custom struct-based, disposable array wrapper, but any of it is beyond the compatibility requirements. You can create a customized fork based on 34f85daa9a798be82d7839acfef4a662cdb6dd1d if you wish.
Guess you forgot to revert: GD.VarToBytes
Good catch, thanks!
I don't mean return a ReadOnlySpan instead of a T[] but instead of creating an array and return it take a Span and write to that instead.
Oh, do you mean to return a Span for developers to write into, like a temporary buffer? As you mentioned, that would work by utilizing an array pool, but it would be challenging to manage the lifetime of the rented arrays. It sounds like a job for a custom struct-based, disposable array wrapper, but any of it is beyond the compatibility requirements. You can create a customized fork based on 34f85da if you wish.
No I mean in addition to this:
public byte[] BufferGetData(Rid buffer, uint offsetBytes = 0u, uint sizeBytes = 0u)
Have an overload like this:
public void BufferGetData(Rid buffer, Span<byte> target, uint offsetBytes = 0u, uint sizeBytes = 0u)
No I mean in addition to this:
public byte[] BufferGetData(Rid buffer, uint offsetBytes = 0u, uint sizeBytes = 0u)Have an overload like this:
public void BufferGetData(Rid buffer, Span<byte> target)
I don't think this is possible. The C# binding layer does not contain Godot logic; the binding generator creates the APIs you see.
No I mean in addition to this:
public byte[] BufferGetData(Rid buffer, uint offsetBytes = 0u, uint sizeBytes = 0u)Have an overload like this:
public void BufferGetData(Rid buffer, Span<byte> target)I don't think this is possible. The C# binding layer does not contain Godot logic; the binding generator creates the APIs you see.
Hm that's quite unfortunate. I would know how to implement that in C# pretty easily, but I just don't understand the binding generator (C++) so I can't just add it myself.
Request removing breaks_compact & feature_proposal.
Did the stance on spans change? I saw this a bit ago.
@TinkerWorX From my understanding, these span APIs merely remove the unnecessary allocations on the C# side; spans are marshaled to godot-packed-arrays just as they were in array APIs.
@TinkerWorX From my understanding, these span APIs merely remove the unnecessary allocations on the C# side; spans are marshaled to godot-packed-arrays just as they were in array APIs.
Alright, I was just not sure, maybe Juan meant C++ spans on the engine side.
I should probably generate ROS overloads for vararg methods as well.
Edit: Done, vararg methods now have overloads like:
public Error Rpc(StringName method, params Variant[] @args)
public Error Rpc(StringName method, ReadOnlySpan<Variant> @args)
public Variant New(params Variant[] @args)
public Variant New(ReadOnlySpan<Variant> @args)
public void CallGroupFlags(long flags, StringName group, StringName method, params Variant[] @args)
public void CallGroupFlags(long flags, StringName group, StringName method, ReadOnlySpan<Variant> @args)
Thanks!
The only drawback is that RenderingDevice.TextureCreate is not affected.