floooh / sokol-zig

Zig bindings for the sokol headers (https://github.com/floooh/sokol)
zlib License
375 stars 48 forks source link

[Help] Zig sokol-fetch bindings #63

Closed trace-andreason closed 6 months ago

trace-andreason commented 6 months ago

I'd love to get the sokol fetch bindings available. I'd be happy to help with this, but I'm not sure how the zig bindings that currently exist are auto generated (a simple zig translate-c doesn't seem to align with the existing bindings). From the last issue I opened, it didn't sound like there was a reason they couldn't exist. Any direction on this would be appreciated!

floooh commented 6 months ago

First, in the original sokol repo, go to sokol/bindgen/ and clone all the currently existing bindings (not just the zig bindings):

git clone https://github.com/floooh/sokol-zig
git clone https://github.com/floooh/sokol-nim
git clone https://github.com/floooh/sokol-rust
git clone https://github.com/floooh/sokol-odin

...next make sure that clang and python3 are in the path, and run python3 gen_all.py. This should run through without any errors.

Next, add [ '../sokol_fetch.h', 'sfetch_', [] ], to this array: https://github.com/floooh/sokol/blob/67339198b75a7d04da3676db2b56514bf363d4b1/bindgen/gen_all.py#L3-L13

...run python3 gen_all.py again and check what happens. Potential errors are that the generator scripts don't know how to convert some of the structs or function signatures, or generate code that doesn't work. In that case report back and let's figure out a solution :)

trace-andreason commented 6 months ago

ok, got the code generation working. Three issues found during the process:

  1. Generation code doesn't handle the callback function defined on the request. This stops code gen with this error: ERROR gen_struct: callback: sfetch_callback_t;. I just threw in a case inside the gen_struct function to catch this field specifically and write out a manually defined function. Not sure if this would even work because its the sokol library that would be calling the callback and its a Zig style function I'm defining. I'll have some time to test this out tomorrow, but maybe you'll know this isn't going to work before I get a chance to test.

  2. Continue seems to be a problematic function name in zig. Easy add to the overrides

    pub extern fn sfetch_continue(Handle) void;
    pub fn continue(h: Handle) void {
    sfetch_continue(h);
    }
  3. This:

    pub extern fn sfetch_setup([*c]const Desc) void;
    pub fn setup(desc: Desc) void {
    sfetch_setup(&desc);
    }

    shadows this:

    pub extern fn sfetch_desc() Desc;
    pub fn desc() Desc {
    return sfetch_desc();
    }

    easiest solution I guess is to add desc to the overrides as well.

Opened a draft pull request with the changes mentioned above. Would also need to add the sokol_fetch.h and corresponding sokol_fetch.c files to this repo.

floooh commented 6 months ago

For the name collisions you can define mappings to a different name here (for instance I was running into the same problem with sgl_error() which mapped to Zig's reserved error keyword):

https://github.com/floooh/sokol/blob/f70cc07f0fcd763ab6ff0171a6b8fe1dca3ed367/bindgen/gen_zig.py#L50-L63

E.g. you could map sfetch_desc to sfetch_get_desc.

Similar if you want to ignore a function, just add it here, and then provide your manual replacement.

https://github.com/floooh/sokol/blob/f70cc07f0fcd763ab6ff0171a6b8fe1dca3ed367/bindgen/gen_zig.py#L38-L43

I would prefer if we could handle the callback function signature not as special case though.

I'll try provide better feedback in the evening.

floooh commented 6 months ago

Would also need to add the sokol_fetch.h and corresponding sokol_fetch.c files to this repo.

The sokol_fetch.h file should be copied automatically during gen_all.py, only a manual sokol_fetch.c file should be needed in a sokol-zig PR. Best would be to run gen_all.py, and then provide an initial PR from the result (this would include both sokol_fetch.h/.c, but the .h file should have been copied automatically).

floooh commented 6 months ago

...ok, let's continue the discussion in the PR: https://github.com/floooh/sokol/pull/1048 :)

I found a fix for the sfetch_callback_t problem => detailed in the PR comment.

floooh commented 6 months ago

Ok, last remaining steps in this comment:

https://github.com/floooh/sokol/pull/1048#issuecomment-2113012476

trace-andreason commented 6 months ago

PR for that here: https://github.com/floooh/sokol-zig/pull/67/files

floooh commented 6 months ago

Doh I overlooked something :D

The module is currently called sfetch.zig, but it should be called fetch.zig (and imported as sokol.fetch) to be consistent with the other modules.

Let me do that change in the sokol repo, this will then update sokol-fetch and then you'd need to update your PR again, one sec...

trace-andreason commented 6 months ago

fixed the PR on my side

floooh commented 6 months ago

sokol and sokol-zig also fixed, looking at the pr now

floooh commented 6 months ago

Ok, that's it, thanks! If you notice any problems when actually using the bindings let me know and we can figure out the solution (I'll write a reminder ticket to add a simple example, preferably one that doesn't add to much data or new dependencies to sokol-zig).