sokol-zig icon indicating copy to clipboard operation
sokol-zig copied to clipboard

sokol_nuklear zig bindings

Open chakany opened this issue 1 year ago • 9 comments

Hello! I was looking to add sokol_nuklear bindings to this repository for usage inside of my project, but I noticed that the bindings were all machine-generated. I couldn't find a tool anywhere that would let me generate these bindings, but I did see your blog post.

Could you shed some light on how I can make these bindings please? Thank you

chakany avatar Jul 09 '24 22:07 chakany

The important stuff is here:

https://github.com/floooh/sokol/tree/master/bindgen

(I just notice that the readme is incomplete, you'll also need to clone the new D bindings into the bindgen directory: git clone https://github.com/floooh/sokol-d

I think it's best to do roughly the same as for sokol_imgui.h (just search for sokol_imgui and simgui). sokol_imgui.h is a bit special in that there's no example in sokol-zig, because it would require a cimgui+Dear ImGui dependency, instead the example is in a separate repository:

https://github.com/floooh/sokol-zig-imgui-sample

If you get stuck, let me know. Also if you get it working let me also know because then we can add it to the official bindings ;)

floooh avatar Jul 10 '24 06:07 floooh

Basically:

  • add a new line here, similar to the sokol_imgui.h entry: https://github.com/floooh/sokol/blob/7b20c1936229370277d1c61bde950bce194de584/bindgen/gen_all.py#L31
  • add a new line here: https://github.com/floooh/sokol/blob/7b20c1936229370277d1c61bde950bce194de584/bindgen/gen_zig.py#L25
  • ...and here: https://github.com/floooh/sokol/blob/7b20c1936229370277d1c61bde950bce194de584/bindgen/gen_zig.py#L39
  • ...in the cloned sokol-zig subdirectory, add a sokol_nuklear.c file here: https://github.com/floooh/sokol-zig/tree/master/src/sokol/c, with content similar to sokol_imgui.h, don't worry about the Nuklear header dependency yet, this is only needed for building the sokol C library later, not for generating the bindings

Back in the bindgen directory, try running python3 gen_all.py and look for errors. If it works, the sokol_nuklear.h header should have been copied to sokol-zig/src/sokol/c/, and a file `sokol-zig/src/sokol/nuklear.zig should have been created.

floooh avatar Jul 10 '24 06:07 floooh

Hello, I did the following steps, and I got something simpler than I imagined.

// machine generated, do not edit

const builtin = @import("builtin");
const sg = @import("gfx.zig");
const sapp = @import("app.zig");

// helper function to convert a C string to a Zig string slice
fn cStrToZig(c_str: [*c]const u8) [:0]const u8 {
    return @import("std").mem.span(c_str);
}

So, none of the actual bindings are there. Very strange.

chakany avatar Jul 10 '24 18:07 chakany

Hmm, I get a different problem which I didn't anticipate:

In file included from sokol-zig/src/sokol/c/sokol_nuklear.c:6:
sokol-zig/src/sokol/c/sokol_nuklear.h:322:2: error: "Please include nuklear.h before sokol_nuklear.h"
#error "Please include nuklear.h before sokol_nuklear.h"
 ^
sokol-zig/src/sokol/c/sokol_nuklear.h:440:24: error: unknown type name 'nk_handle'
SOKOL_NUKLEAR_API_DECL nk_handle snk_nkhandle(snk_image_t img);
                       ^
sokol-zig/src/sokol/c/sokol_nuklear.h:441:60: error: unknown type name 'nk_handle'
SOKOL_NUKLEAR_API_DECL snk_image_t snk_image_from_nkhandle(nk_handle handle);

Problem is that sokol_nuklear.h needs the nuklear.h header even for the declarations, because of the use of nk_handle here:

https://github.com/floooh/sokol/blob/7b20c1936229370277d1c61bde950bce194de584/util/sokol_nuklear.h#L440

...this is actually tricky to solve because requiring the nuklear.h header for bindings generation is a no-go.

I don't have a quick solution to offer, we basically need to get rid somehow of all nuklear.h dependencies (like nk_handle). Maybe with a preprocessor define somewhere...

floooh avatar Jul 10 '24 20:07 floooh

I don't have a quick solution to offer, we basically need to get rid somehow of all nuklear.h dependencies (like nk_handle). Maybe with a preprocessor define somewhere...

would it be possible to activate the preprocessor ifs from the command line at compile time? and only when building for bindings?

chakany avatar Jul 10 '24 22:07 chakany

Yep, cleanest solution would be to add a define in the clang call here:

https://github.com/floooh/sokol/blob/7b20c1936229370277d1c61bde950bce194de584/bindgen/gen_ir.py#L102

...maybe this works:

cmd = ['clang', '-Xclang', '-ast-dump=json', '-D', 'SOKOL_BINDGEN', '-c' ]

...and then in the sokol_nuklear.c file we can check for SOKOL_BINDGEN and provide the required Nuklear types directly before sokol_nuklear.h is included, e.g.:

#if defined(SOKOL_BINDGEN)
typedef union {void *ptr; int id;} nk_handle;
#endif

...but hmm, next problem will be that the code generation ignores this typedef because it doesn't start with the right prefix... but let's fix those things step by step...

...the sokol_nuklear.h public API directly using Nuklear types is definitely a code smell...

In sokol_imgui.h this is solved via a 'type-erased' void*:

https://github.com/floooh/sokol/blob/7b20c1936229370277d1c61bde950bce194de584/util/sokol_imgui.h#L533

...but for Nuklear that wouldn't work unfortunately...

There's also this function which needs a solution:

https://github.com/floooh/sokol/blob/7b20c1936229370277d1c61bde950bce194de584/util/sokol_nuklear.h#L435

...if it would just be the snk_nkhandle function we could probably filter that out for now (it's not a critical function unless the user code wants to work with images). But we can't simply kick out the snk_new_frame function.

floooh avatar Jul 11 '24 07:07 floooh

been working on this for a little while, i have some basic types.

#if defined(IMPL)
#define SOKOL_NUKLEAR_IMPL
#define NK_IMPLEMENTATION
#include "nuklear.h"
#endif
#include "sokol_defines.h"
#include "sokol_app.h"
#include "sokol_gfx.h"
#ifdef SOKOL_BINDGEN
struct nk_context;
typedef union {void *ptr; int id;} nk_handle;
typedef unsigned int nk_flags;
typedef int(*nk_plugin_filter)(const struct nk_context*, nk_handle, int*, int);
#endif
#include "sokol_nuklear.h"

generations go until it reaches a struct nk_context * type. it outputs

Error as_c_arg_type(): struct nk_context *

I believe that the nk_context struct is not being included in the AST dump, and that's why it cannot be found. So, i'm trying to get the nk_ prefix included in the gen_zig.py/gen_ir.py scripts, and I think i'm stuck.

Thanks again.

chakany avatar Jul 17 '24 21:07 chakany

Hi @chakany, do you have a work-in-progress published somewhere? I'm interested in hacking on this a bit myself as well.

Edit: I'm caught up to where you are in my own fork now.

I believe that the nk_context struct is not being included in the AST dump, and that's why it cannot be found. So, i'm trying to get the nk_ prefix included in the gen_zig.py/gen_ir.py scripts, and I think i'm stuck.

I've tried this also, and I can get the nk_ prefix included by piggy-backing off dep_prefixes, but it runs into issues elsewhere and doesn't feel like a nice solution.

...the sokol_nuklear.h public API directly using Nuklear types is definitely a code smell...

It seems to me like getting the Nuklear types out of sokol_nuklear.h would be the quickest route?

In sokol_imgui.h this is solved via a 'type-erased' void*: ...but for Nuklear that wouldn't work unfortunately...

Why wouldn't this work for Nuklear? If snk_new_frame returns a void *, the user would just have to know that it is guaranteed safe to cast it to struct nk_context *, which at that point they have visible. It's not ideal to require casting a void *, but it's not entirely out of the ordinary for such APIs especially with context pointers.

Or, rather than void *, perhaps a snk_context_t opaque handle which when !defined(SOKOL_BINDGEN) is simply struct nk_context *, but when generating bindings is an opaque void *? Then from the perspective of the user, the compiler should consider it equivalent, but from the perspective of generating bindings, we don't need to know anything about struct nk_context or its layout? Similar for nk_handle and nk_flags, perhaps.

– Dylan

DylanLukes avatar Sep 02 '24 01:09 DylanLukes

Ah... looks like struct nk_context is used inside _snk_state_t[1], so its layout actually has to be known. That would seem to preclude the usual opaque pointer tricks without some more significant rewriting of sokol_nuklear.h to remove any dependencies on nuklear.h in the public API.

Doesn't seem egregiously difficult, as nine times out of ten the usage is just taking the address of it (e.g. &_snuklear.ctx). There are some exceptions like _snuklear.ctx.clip.copy = _snk_clipboard_copy and if (_snuklear.atlas.default_font) { ... }, but these are implementation details and not important for sokol_nuklear.h bindgen purposes.

[1] As is struct nk_font_atlas for that matter...

DylanLukes avatar Sep 02 '24 02:09 DylanLukes