ebitengine / purego

Apache License 2.0
1.95k stars 63 forks source link

Support Linux callbacks #163

Closed jwijenbergh closed 9 months ago

jwijenbergh commented 9 months ago

Fixes #124

Most of the work was already there, it's just some re-ordering. The main blocker was the ARM64 crash with callbacks which was nicely fixed by @TotallyGamerJet in https://github.com/ebitengine/purego/issues/124#issuecomment-1730540730 and merged in #165

I had to split the callback tests into separate tests for floats as purego only supports float arguments with CGO_ENABLED=0 currently.

Tested on my amd64 laptop, and an arm64 device. Both with CGO_ENABLED=0 and CGO_ENABLED=1. The test suite passes and my GTK program leveraging https://github.com/jwijenbergh/puregotk/ also works on both architectures.

Edit: Floats are now supported with CGO_ENABLED=1 so this is basically just changing some build tags

hajimehoshi commented 9 months ago

Could you submit a PR only with sys_unix_arm64.s for iOS arm64 so that I can cherry-pick the change to the stable branch?

hajimehoshi commented 9 months ago

Or I hope @TotallyGamerJet would do that. Anway, let's wait for @TotallyGamerJet as we need their review.

Thank you for your hard work!

jwijenbergh commented 9 months ago

Or I hope @TotallyGamerJet would do that. Anway, let's wait for @TotallyGamerJet as we need their review.

Thank you for your hard work!

Okay! Let's wait indeed. Makes more sense that @TotallyGamerJet makes that PR and I will then rebase this PR on top of that one once it is merged

jwijenbergh commented 9 months ago

Rebased.

TotallyGamerJet commented 9 months ago

Linux amd64 and arm64 should be able to support floats even with CGO_ENABLED=1. You just need to change the build tags in syscall_cgo_linux.go to exclude amd64 and arm64 and then include them in sysycall_sysv.go, dlfcn_nocgo_linux.go and dlfn_stubs.s.

Hopefully that's all of them

TotallyGamerJet commented 9 months ago

@hajimehoshi do you want to look at it then merge?

jwijenbergh commented 9 months ago

Thanks for the review @hajimehoshi. Should be good now

jwijenbergh commented 9 months ago

Thanks!