Use immutable structs?

[dhaffey]

My understanding is that arrays of non-isbits types aren't compatible with the C layout. I'm pretty sure #45 is running afoul of this, as replacing SDL2.Point with struct SDLPoint x::Cint; y::Cint end fixes their example.

I'm fairly new to Julia, so apologies in advance if I'm missing something obvious here.

[jonathanBieler]

I generated the bindings automatically using Clang.jl so I'm not exactly why it's using a mutable struct there. To be honest I don't really understand the difference (I find the manual a bit confusing).

@cmcaine re-generated the bindings with more up-to-date SDL and Clang but it seems to have done the same :

https://github.com/cmcaine/LibSDL2.jl/blob/master/gen/bindings/libsdl2_types.jl#L1706

That said if you think that's wrong I'm fine with correcting it.

[cmcaine]

You can configure clang to do either. On balance, I think the mutable structs are better for how you have to use SDL, but opinions can differ :)

…

On Wed, 29 Jul 2020, 14:30 Jonathan Bieler, ***@***.***> wrote: I generated the bindings automatically using Clang.jl so I'm not exactly why it's using a mutable struct there. To be honest I don't really understand the difference (I find the manual a bit confusing). @cmcaine <https://github.com/cmcaine> re-generated the bindings with more up-to-date SDL and Clang but it seems to have done the same : https://github.com/cmcaine/LibSDL2.jl/blob/master/gen/bindings/libsdl2_types.jl#L1706 That said if you think that's wrong I'm fine with correcting it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#50 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNZA6OIO2JYD27CE2NY56LR6AP5RANCNFSM4PLGQ7WQ> .

[cmcaine]

Ah, I hadn't read the first post yet, yeah, that's an issue 👍

We can easily make them all immutable structs or fairly easily maintain a list of which to be mutable or immutable.

[dhaffey]

A quick grep through the headers for plural argument names turns up Color, Rect, FRect, Point, FPoint, and MessageBoxButtonData being used as array inputs. Event is used as an array output by PeepEvents. So these at least should probably be immutable.

[jonathanBieler]

Alright, I'll try to merge the new bindings. Where do you configure these things @cmcaine ?

[cmcaine]

https://github.com/cmcaine/LibSDL2.jl/blob/c1143efcd51e3a94166dcb6f09e39cf7699c8606/gen/generate_bindings_custom.jl#L108 Change that line to false and all the structs will come out immutable. To make some mutable and some immutable we'd need to write some custom stuff in the loop below, but all immutable is better than the status quo. I'd paused work on these bindings and there are a few outstanding issues, none of which are dealbreakers, probably: 1. I couldn't decide if I wanted to stick with stripping SDL from all the names or if I wanted to just export all the names as e.g. LibClang does. It's complicated because SDL doesn't prefix all exports. Anyway, this can just be dropped. 2. I wanted to generate the documentation statically in a two-pass generation rather than dynamically in package precompile. I think this would improve precomp speed a bit, but I'd have to measure again to be sure. 3. I haven't replaced the functionality of pollEvent. I think I was stuck deciding what names to put the functionality under. I did open a useful discourse thread about it and clarified how to do the work more safely. I probably don't have time today, but if you want to merge the work I can look at these. Finally, I should note that I/clang changed the field names and layouts slightly in a handful of structs. The Enums are also backwards incompatible unless we add something to automatically convert ints to CEnums, which we could do.

…

On Tue, 4 Aug 2020, 14:04 Jonathan Bieler, ***@***.***> wrote: Alright, I'll try to merge the new bindings. Where do you configure these things @cmcaine <https://github.com/cmcaine> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#50 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNZA6NIIJXOLKQVYBXTXNLR7ABODANCNFSM4PLGQ7WQ> .

[cmcaine]

Love how GitHub mangles emails. I sent that from my phone, so I don't even know why it has hard breaks.

Anyway, I also have this little if statement that selects whether to use the system SDL or LibSDL2.jll. I don't know if you'd want to keep that or not.

I mostly had that for speed, but it doesn't matter so much on v1.5 (or 1.6?).