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

Fix Odin bindgen type mappings #1158

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

Barinzaya
Copy link
Contributor

@Barinzaya Barinzaya commented Nov 19, 2024

This affects intptr_t, uintptr_t, and size_t, which are variable-sized. The previous mappings were mapped directly to 64-bit values, and they've been changed to use the corresponding core:c aliases.

This closes #1157. An additional fix is required in sokol-odin itself to correct helpers.Allocator.alloc_fn, which uses u64 for a size_t argument.

This affects intptr_t, uintptr_t, and size_t, which are variable-sized.
The previous mappings were mapped directly to 64-bit values.
@Barinzaya Barinzaya changed the title Fixed Odin bindgen type mappings Fix Odin bindgen type mappings Nov 19, 2024
Barinzaya added a commit to Barinzaya/sokol-odin that referenced this pull request Nov 19, 2024
This corresponds to a change in the bindgen mappings, see
floooh/sokol#1157 and floooh/sokol#1158, except this function is not
automatically generated.
@floooh
Copy link
Owner

floooh commented Nov 19, 2024

Thanks!

@jakubtomsu when you have a minute, can you also have a look in case you have any objections?

(also at: floooh/sokol-odin#22)

@floooh
Copy link
Owner

floooh commented Nov 19, 2024

Looks like at least one sample needs to be fixed as well:

/home/runner/work/sokol/sokol/examples/instancing/main.odin(126:16) Cannot assign value 'u64(state.cur_num_particles * size_of(m.vec3))' of type 'u64' to 'uint' in structure literal
	size = u64(state.cur_num_particles * size_of(m.vec3)),
	       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^

...but that may be a chicken egg problem (e.g. first commit this MR to update the bindings, then fix the sample(s)).

@jakubtomsu
Copy link
Contributor

yep, looking at the code everything seems fine.

@Barinzaya
Copy link
Contributor Author

Barinzaya commented Nov 19, 2024

Should I update the sokol-odin PR to include the change to the sample as well, then?

@floooh
Copy link
Owner

floooh commented Nov 19, 2024

Should I update the sokol-odin PR to include the change to the sample as well, then?

I think that makes sense, even if it might not compile until the sokol PR is merged.

@floooh
Copy link
Owner

floooh commented Nov 19, 2024

Oki thanks for the PRs. I'm currently busy working an unrelated sokol PR, but I'll try to get this stuff merged tomorrow.

@floooh floooh merged commit 485fba6 into floooh:master Nov 20, 2024
30 of 32 checks passed
@floooh
Copy link
Owner

floooh commented Nov 20, 2024

...and that one also merged, watching the CI pipeline out if everything looks in order...

@floooh
Copy link
Owner

floooh commented Nov 20, 2024

...also thanks for the PR! :)

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 this pull request may close these issues.

Odin Bindgen Type Mismatches
3 participants