bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
14.59k stars 1.92k forks source link

Improve c sharp binding generation #3261

Open TJHeuvel opened 4 months ago

TJHeuvel commented 4 months ago
TJHeuvel commented 4 months ago

There is one more change that i'd like to do, but wanted some feedback before attempting.

The bindings are now generated from the C API, with explicit this parameters. Although i do think this binding should stay very low level, this doesn't map very well to C#. Even the original C++ API just uses objects with methods.

I would like to update the binding generator to add methods to the structs, rather than the current C-style API.

[DllImport(DllName, EntryPoint="bgfx_vertex_layout_begin", CallingConvention = CallingConvention.Cdecl)]
public static extern unsafe VertexLayout* vertex_layout_begin(VertexLayout* _this, RendererType _rendererType = RendererType.Noop);

Would become private, and in VertexLayout a begin method would call it. This is however a breaking change for any existing C# projects. It would be possible to do both, however thats just adding more confusion in my opinion.

Likewise with init_ctor, which already caught me.

bkaradzic commented 4 months ago

For the handles, it was not possible to have Invalid as a default parameter. Instead i emit another method without the argument, that calls the extern with an Invalid handle. This also required some overhauling for those structs, and i've also implemented some recommended fields to improve performance (https://learn.microsoft.com/en-us/visualstudio/ide/reference/generate-equals-structs?view=vs-2022). This is an internal change, any existing project will still compile.

This change makes all code managed only. bgfx in C# should be unmanaged.

TJHeuvel commented 4 months ago

For the handles, it was not possible to have Invalid as a default parameter. Instead i emit another method without the argument, that calls the extern with an Invalid handle. This also required some overhauling for those structs, and i've also implemented some recommended fields to improve performance (https://learn.microsoft.com/en-us/visualstudio/ide/reference/generate-equals-structs?view=vs-2022). This is an internal change, any existing project will still compile.

This change makes all code managed only. bgfx in C# should be unmanaged.

I'd gladly change the struct additions, i'm not very attached to them. However i dont believe this makes them unmanaged, if you are referring to the C# concept.

A type is an unmanaged type if it's any of the following types:

    sbyte, byte, short, ushort, int, uint, long, ulong, nint, nuint, char, float, double, decimal, or bool
    Any enum type
    Any pointer type
    Any user-defined struct type that contains fields of unmanaged types only.

This is still satisfied since the only field is still an ushort. I'm not entirely sure how to test this, but for example the unmanaged generic type constraint is met. For the class, and a struct that fails above restrictions, it does not.

image

It does however add a whole lot of noise. There is also an entirely different strategy possible, where there is only a single handle type.


struct DynamicIndexBuffer { }

public readonly struct Handle<T>
{
    public readonly ushort idx;
}

Handle<DynamicIndexBuffer> someHandle;

This offers the same amount of type safety, a handle of one type cannot be used for another.

TJHeuvel commented 4 months ago

My latest commit undid most of the struct changes. The only changes that remain there are the static Invalid field, and a constructor to pass along the index.

    public struct DynamicVertexBufferHandle {
        public ushort idx;
        public static readonly DynamicVertexBufferHandle Invalid = new DynamicVertexBufferHandle(ushort.MaxValue);
        public DynamicVertexBufferHandle(ushort index) => idx = index;
        public bool Valid => this.idx != Invalid.idx;
    }