flibitijibibo / SDL3-CS

Other
26 stars 4 forks source link

Ensure structs are blittable #5

Closed flibitijibibo closed 1 month ago

flibitijibibo commented 1 month ago

Apparently bool needs special care in C# and needs something like [MarshalAs(UnmanagedType.U1)] so that .NET Framework doesn't get mad about being a nonblittable type. SDL_Event in particular is affected by this; we need to cast to SDL.SDL_Event* in a couple places and there's no real way around this.

CC @kg

kg commented 1 month ago

For now you can replace all the public bool in struct definitions with public byte, and then adapt code or add property accessors. MarshalAs might work though, but I think it may not fix pointers-to-T, only out and ref.

flibitijibibo commented 1 month ago

Will push the workaround soon!

flibitijibibo commented 1 month ago

https://github.com/flibitijibibo/SDL3-CS/commit/aca89c6c127a7a6a1a0e2c8e8d510fa48fb27a48

cryy22 commented 1 month ago

if this is only necessary for struct blitting, we could reduce the blast radius of the workaround by only mapping _Bool to byte when the TypeContext is StructField

https://github.com/flibitijibibo/SDL3-CS/blob/aca89c6c127a7a6a1a0e2c8e8d510fa48fb27a48/GenerateBindings/Program.cs#L732-L734

https://github.com/flibitijibibo/SDL3-CS/blob/aca89c6c127a7a6a1a0e2c8e8d510fa48fb27a48/GenerateBindings/Program.cs#L11-L16

kg commented 1 month ago

I should point out that while MarshalAs in the struct may work, it won't work for WebAssembly .NET right now, which may actually matter to someone. This is because the marshaling implementation for WASM is much simpler than the one for other targets.

kg commented 1 month ago

if this is only necessary for struct blitting, we could reduce the blast radius of the workaround by only mapping _Bool to byte when the TypeContext is StructField

https://github.com/flibitijibibo/SDL3-CS/blob/aca89c6c127a7a6a1a0e2c8e8d510fa48fb27a48/GenerateBindings/Program.cs#L732-L734

https://github.com/flibitijibibo/SDL3-CS/blob/aca89c6c127a7a6a1a0e2c8e8d510fa48fb27a48/GenerateBindings/Program.cs#L11-L16

Another option we could try would be to generate 'ref' parameters instead of pointer-typed parameters for delegates. I'm not really sure what the advantage of using i.e. SDL_Event * instead of ref SDL_Event is, since you can use Unsafe to map refs to/from pointers.

thatcosmonaut commented 1 month ago

https://github.com/ppy/SDL3-CS/pull/147

This project creates a blittable bool type this way:

    public readonly record struct SDLBool
    {
        private readonly byte value;

        internal const byte FALSE_VALUE = 0;
        internal const byte TRUE_VALUE = 1;

        [Obsolete("Never explicitly construct an SDL bool.")]
        public SDLBool()
        {
        }

        internal SDLBool(byte value)
        {
            this.value = value;
        }

        public static implicit operator bool(SDLBool b) => b.value != FALSE_VALUE;

        public static implicit operator SDLBool(bool b) => new SDLBool(b ? TRUE_VALUE : FALSE_VALUE);

        public bool Equals(SDLBool other) => (bool)other == (bool)this;

        public override int GetHashCode() => ((bool)this).GetHashCode();

        private bool PrintMembers(StringBuilder builder)
        {
            builder.Append($"0x{value:x2}");
            return true;
        }
    }
Odex64 commented 1 month ago

fyi: when runtime marshalling is enabled, bool types must be handled with a wrapper struct or by using the [MarshalAs(UnmanagedType.U1)] attribute... that's because a bool in C# can take up to 4 bytes.

However, when runtime marshalling is disabled you should be able to pass bool as a 1 byte value, thus making it blittable. Not sure about returns though.

Note: that this will have some effects about blittable types. See here.

thatcosmonaut commented 1 month ago

We have to support .NET Framework and disabling runtime marshaling is not an option there. I think at some point we'll look into also generating Core-specific bindings.