Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use immutable structs? #50

Closed
dhaffey opened this issue Jul 29, 2020 · 7 comments · Fixed by #54
Closed

Use immutable structs? #50

dhaffey opened this issue Jul 29, 2020 · 7 comments · Fixed by #54

Comments

@dhaffey
Copy link

dhaffey commented Jul 29, 2020

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
Copy link
Collaborator

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
Copy link

cmcaine commented Jul 29, 2020 via email

@cmcaine
Copy link

cmcaine commented Jul 29, 2020

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
Copy link
Author

dhaffey commented Jul 30, 2020

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
Copy link
Collaborator

jonathanBieler commented Aug 4, 2020

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

@cmcaine
Copy link

cmcaine commented Aug 4, 2020 via email

@cmcaine
Copy link

cmcaine commented Aug 4, 2020

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?).

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 a pull request may close this issue.

3 participants