tsdl Generate bindings using ctypes-stubs instead of ctypes-foreign (Minimize the use of libffi)

Depends on #108

Pros:

(Independently from removing 1 definition to silence the warning from the C compiler that says this is a deprecated function that shouldn't be use (which we can very well reintroduce at the price of living with this warning)), tsdl.mli is not touched at all, so compatibility is fully preserved
Removes the overhead of libffi (but for 1 case, see below)
Contradict me if you disagree but I would claim that maintenance/extension is as easy as it was if not even a bit simplified because this binding methods does a bit more checks statically so we will be informed by the compiler about some categories of changes if they occurs on SDL side...
I struggled having the build system right but now that it is, I don't think it's too bad.

Meh:

There is so many cut/paste/reexport that we rely on the type-checkers to be sure nothing has been neither forgotten nor mis-exported. Hopefully, this method of binding is suppose to check more aspects statically...
It does not get fully rid of libffi. 1 single occurrence remains:

You cannot do without libffi (the ~thread_registration:true ~runtime_lock:true of

let as_callback =
  Foreign.funptr_opt ~thread_registration:true ~runtime_lock:true C.Types.as_callback_type

in audio_specs

Cons:

See below, I put as comments in order to quote them the definitions I'm unhappy with.

The direction taken by the discussion convinces me more and more to take the plunge but still, I repeat, this PR was open as a request for comments now that I've spent on these hours working on it...

For the one interested (because it was the first version of this PR description so I don't erase it but I now think that we don't care), here is my story for drowning into this mess:

I was experimenting with compiling this library using the "mingw" backend on windows (I know work has been done to compile it using the "msvc" backend and there exist a world where it works (which is no the vanilla opam-repository world (wherectypes is not compatible with msvc so no way to get tsdl)))
It was a complete failure (because symbols that gcc don't see any static use of are erased (while libffi will use them dynamically here) unless you explicitly put the no-as-needed flag which (as far as I understand) does not exist in mingw gcc/ld/...)
what seems to work with mingw is ctypes bindings built using "stubs" and well, actually, if doable, getting rid of libffi is a win for everybody independently of being on windows, isn't it? And migrating to this binding generation method is a matter of fighting the build system and a LOT of copy/paste, right?
let's give it a try...

Conclusion: See above I still don't have a successful build under mingw windows port... Not because of missing definitions this time but because of the ocaml compiler that hardwired the argument -municode when calling the C compiler while SDL would need -mwindows...

Mar 03 '25 16:03 pirbo

Do you envision to actually switch one day?

Can you remind me exactly why we want to get rid of libffi ? I remember being mentioned that it's not supported on Windows but that's not what I read here.

Something that would drive me to switch would be users of the binding complaining that libffi is stealing to many cycles for their use. But then I wonder if case arising we could take an incremental approach to that.

I'm not sure about the build system at all. At the very least, it's very ugly but it might even be wrong. I just hammered until having the tests passing.

They look rather minimal. IIRC I suggested you could simply try to use dune. Would it work more seamlessly or would you run into same problems (the mingw things) ?

You don't get rid of libfii! There is one thing you cannot do without libffi (the ~thread_registration:true ~runtime_lock:true of
let as_callback =
  Foreign.funptr_opt ~thread_registration:true ~runtime_lock:true C.Types.as_callback_type

Well I suppose nothing prevents you from doing one manual (non-ctypes) binding on the side for that one no ?

Mar 03 '25 16:03 dbuenzli

Now perhaps more heavy users than I am like @sanette or @zoggy have more opinions on that.

Mar 03 '25 16:03 dbuenzli

I'm afraid my experience with Windows is way too old to be of interest now, sorry.

Back in the times I was able to code some SDL games in Windows with vanilla ocaml+cygwin, but this was not with tsdl, but ocamlsdl (now https://github.com/fccm/OCamlSDL2)

Mar 03 '25 17:03 sanette

@sanette the opinion I'm seeking is more about having ctypes-generated bindings vs libffi baked ones.

Mar 03 '25 19:03 dbuenzli

Preamble: I just don't care about Windows (or MacOS or any other proprietary OS), and I even prefer if it does not work on these OS :-)

Regarding my use of TSdl in Stk, I don't think the libffi performance is a problem (I did not know there was such a difference between libffi and ctypes stubs), so event if I would be interested in comparing both, I have no real opinion. As it seems like a quite big work, which could introduce bugs, I can help for testing if needed.

Mar 04 '25 08:03 zoggy

I'm really not a specialist of this question. Theoretically, a library like SDL which focuses on performance would benefit from static ctypes stubs, so if this is feasible, that seems a good idea. However, this means also working on tsdl_ttf and tsdl_image. And the cost of maintenance is likely to be higher than what it is right now. Also, maintaining compat with ocaml 4 branch, which is necessary IMHO, might not be so obvious: I made some quick experiments to install https://github.com/pirbo/tsdl.git#pirbo-rewords, it works nicely with ocaml 5.3.0 but fails with 4.14.0:

[ERROR] The compilation of tsdl.1.1.0 failed at "ocaml pkg/pkg.ml build --dev-pkg true".

#=== ERROR while compiling tsdl.1.1.0 =========================================#
# context     2.2.1 | linux/x86_64 | ocaml-base-compiler.4.14.0 | pinned(git+https://github.com/pirbo/tsdl.git#pirbo-rewords#a1eed9aaf19499ef2de32523d7d2582b7ab92353)
# path        ~/.opam/4.14.0/.opam-switch/build/tsdl.1.1.0
# command     ~/.opam/opam-init/hooks/sandbox.sh build ocaml pkg/pkg.ml build --dev-pkg true
# exit-code   1
# env-file    ~/.opam/log/tsdl-34217-201d00.env
# output-file ~/.opam/log/tsdl-34217-201d00.out
### output ###
# /home/essai/.opam/4.14.0/lib/ocaml/caml/compatibility.h:71:63: error: ‘struct SDL_AudioSpec’ has no member named ‘caml_callback’; did you mean ‘callback’?
# [...]
# support/types_stubs_gen.c:22:47: note: in expansion of macro ‘callback’
#    22 |                offsetof(struct SDL_AudioSpec, callback));
#       |                                               ^~~~~~~~
# Command exited with code 2.
# pkg.ml: [ERROR] cmd ['ocamlbuild' '-use-ocamlfind' '-classic-display' '-j' '4' '-tag' 'debug'
#      '-build-dir' '_build' 'opam' 'pkg/META' 'CHANGES.md' 'LICENSE.md'
#      'README.md' 'src/tsdl.a' 'src/tsdl.cmxs' 'src/tsdl.cmxa' 'src/tsdl.cma'
#      'src/tsdl.cmx' 'src/tsdl.cmi' 'src/tsdl.mli' 'src/dlltsdl_stubs.so'
#      'src/libtsdl_stubs.a' 'src/top/tsdl_top.a' 'src/top/tsdl_top.cmxs'
#      'src/top/tsdl_top.cmxa' 'src/top/tsdl_top.cma' 'src/top/tsdl_top.cmx'
#      'src/tsdl_top_init.ml' 'doc/index.mld' 'test/min.ml' 'test/minc.c']: exited with 10

Mar 04 '25 10:03 sanette

I will rewrite the description of the PR to say first what it is (which is indeed completely unrelated to Windows©) then the points of attention I think reviewer should watch before explaining in which context I've done it (Window :heavy_dollar_sign: ©)
Using dune as the build system (which I did during the development) suppose the diff https://github.com/pirbo/tsdl/pull/1 . Now that I've retrieved the ocamlbuild incantation, I don't have a strong opinion. (Well, if we are certain this build system is correct, I'm now wondering if tsdl.mllib is OK: shouldn't the generated modules Type_description Async_functions_description Functions_description and Types_generated appear explicitly here? I'm confused) In any case, it is orthogonal to the fact that it compiles and work using mingw.
libffi on windows (to the best of my understanding):
- what works is dynamically "dlopen"ing a dll (and therefore having at runtime the machinery to find the dll on the disk) to then look for definition into it.
- What we never manage to make work is (what ctypes-foreign rely on:) having the C compiler find statically at compile time the dll and links it to the binary once for all so that at runtime libffi "introspect the loaded executable" and finds definition into it...(sorry not rigorous terminology)

Mar 04 '25 15:03 pirbo

The OCaml 4.14.1 compatibility raised by @sanette is a big deal indeed!

In OPAM_ROOT/4.14.1/lib/ocaml/caml/compatibility.h, there is

   71 | #define callback CAML_DEPRECATED("callback", "caml_callback") caml_callback

So the preprocessor "happily" rewrites the mention of the field of SDL_AudioSpec named callback into caml_callback :facepalm: That's an interresting one...

Mar 04 '25 16:03 pirbo

shouldn't the generated modules Type_description Async_functions_description Functions_description and Types_generated appear explicitly here?

I think so, yes, because if I try to compile bogue I get:

[ERROR] The compilation of bogue.20250224 failed at "dune build -p bogue -j 3 @install".

#=== ERROR while compiling bogue.20250224 =====================================#
# context     2.2.1 | linux/x86_64 | ocaml-base-compiler.5.3.0 | https://opam.ocaml.org#c940a5f1a97ea0abf72b5f8a3319f15551c41337
# path        ~/.opam/5.3.0/.opam-switch/build/bogue.20250224
# command     ~/.opam/opam-init/hooks/sandbox.sh build dune build -p bogue -j 3 @install
# exit-code   1
# env-file    ~/.opam/log/bogue-71484-961171.env
# output-file ~/.opam/log/bogue-71484-961171.out
### output ###
# (cd _build/default && /home/essai/.opam/5.3.0/bin/ocamlopt.opt -w -40 -g -o examples/examples.exe /home/essai/.opam/5.3.0/lib/xdg/xdg.cmxa -I /home/essai/.opam/5.3.0/lib/xdg /home/essai/.opam/5.3.0/lib/ocaml/str/str.cmxa /home/essai/.opam/5.3.0/lib/stdlib-shims/stdlib_shims.cmxa /home/essai/.opam/5.3.0/lib/integers/integers.cmxa -I /home/essai/.opam/5.3.0/lib/integers /home/essai/.opam/5.3.0/[...]
# File "_none_", line 1:
# Error: No implementation provided for the following modules:
#          "Async_functions_generated" referenced from "Tsdl" (/home/essai/.opam/5.3.0/lib/tsdl/tsdl.cmxa)
#          "Types_generated" referenced from "Tsdl" (/home/essai/.opam/5.3.0/lib/tsdl/tsdl.cmxa)
#          "Async_function_description" referenced from "Tsdl" (/home/essai/.opam/5.3.0/lib/tsdl/tsdl.cmxa)
#          "Functions_generated" referenced from "Tsdl" (/home/essai/.opam/5.3.0/lib/tsdl/tsdl.cmxa)
#          "Function_description" referenced from "Tsdl" (/home/essai/.opam/5.3.0/lib/tsdl/tsdl.cmxa)

Mar 04 '25 17:03 sanette

shouldn't the generated modules Type_description Async_functions_description Functions_description and Types_generated appear explicitly here?

I think so, yes, because if I try to compile bogue I get: [...]

Should be better now. At least, works for me :) (And it is actually less changes in the build system so this is good!)

Mar 05 '25 08:03 pirbo

The OCaml 4.14.1 compatibility raised by @sanette is a big deal indeed!

Fixed

Mar 05 '25 10:03 pirbo

great! I made some test with boguex 50 which makes thousands of calls to SDL functions per frame, and while it used to draw about 140-170 circles per frame, now I get about 230 circles per frame!

Mar 05 '25 21:03 sanette