Zig bindings for orca (still WIP) #140

rdunnington · 2023-09-22T05:54:57Z

rdunnington commented

2023-09-22 05:54:57 +00:00

This is an incomplete set of zig bindings for orca. The included zig sample shows how to build and link the orca runtime with the zig app. It also shows how to use the orca bindings and provides a decent point of comparison to some of the C samples. It also proves that it works. ;)
Some of the API bindings are only partially complete, such as the oc_str8/16/32 functions, and others I haven't started at all, such as UI and GLES.
I tried to keep to zig paradigms and naming conventions, so in some cases functions are partially or entirely reimplemented in zig (such as a lot of the string code). Definitely welcome any feedback! And if we feel this PR isn't ready yet, we can wait for the bindings to be done before merging.

This is an incomplete set of zig bindings for orca. The included zig sample shows how to build and link the orca runtime with the zig app. It also shows how to use the orca bindings and provides a decent point of comparison to some of the C samples. It also proves that it works. ;) Some of the API bindings are only partially complete, such as the oc_str8/16/32 functions, and others I haven't started at all, such as UI and GLES. I tried to keep to zig paradigms and naming conventions, so in some cases functions are partially or entirely reimplemented in zig (such as a lot of the string code). Definitely welcome any feedback! And if we feel this PR isn't ready yet, we can wait for the bindings to be done before merging.

MartinFouilleul commented

2023-09-25 11:28:32 +00:00

@rdunnington Before I merge this, can you put a Readme in the samples folder stating that it is a WIP and describing the current state of the bindings and the potential caveats for people wanting to test them at this early stage?

👍 1

rdunnington force-pushed zig_ffi from 5d5f484744 to 1190966b66

2023-09-26 02:09:00 +00:00

Compare

rdunnington force-pushed zig_ffi from 1190966b66 to 6c4ddd298f

2023-09-26 02:10:47 +00:00

Compare

rdunnington commented

2023-09-26 02:13:45 +00:00

README added, let me know if you'd like me to change anything

MartinFouilleul commented

2023-09-27 09:30:05 +00:00

Last few questions:

Why is there a const added to oc_font_create_from_memory unicode range arg? Is this something Zig complains about?
Why is the default Keymap macro changed? The result is the same (since scancode matches keycodes for the printable keys), although your change might make it more explicit. I'm not against it but I would prefer to keep it in a separate PR

Last few questions: - Why is there a `const` added to `oc_font_create_from_memory` unicode range arg? Is this something Zig complains about? - Why is the default Keymap macro changed? The result is the same (since scancode matches keycodes for the printable keys), although your change might make it more explicit. I'm not against it but I would prefer to keep it in a separate PR

rdunnington commented

2023-09-28 02:08:01 +00:00

Mostly an attempt to keep to zig conventions - typically you would declare an array of constants like this with const instead of var. But if the function prototype takes a non-const that won't compile, which goes against the convention. The internals don't look like they change the array at all, so it seems like it was safe. Tbh I thought I had changed the C function as well to match but looks like I forgot. Is there a strong reason the C function doesn't take a const pointer?
I can revert this if you prefer - it looked like this was actually a bug since the macros to generate oc_key_code could generate different results in some cases than just straight up casting it. But I guess looking at the actual data we currently have, that's not the case.

1. Mostly an attempt to keep to zig conventions - typically you would declare an array of constants like this with `const` instead of `var`. But if the function prototype takes a non-const that won't compile, which goes against the convention. The internals don't look like they change the array at all, so it seems like it was safe. Tbh I thought I had changed the C function as well to match but looks like I forgot. Is there a strong reason the C function doesn't take a const pointer? 2. I can revert this if you prefer - it looked like this was actually a bug since the macros to generate oc_key_code could generate different results in some cases than just straight up casting it. But I guess looking at the actual data we currently have, that's not the case.

MartinFouilleul commented

2023-09-28 11:27:07 +00:00

I don't find const really useful in C, and it has an annoying virality. Also sometimes clang emits warnings when passing non-const pointers to const parameters (eg for nested pointer types). I find the rationale for this very contrived (https://c-faq.com/ansi/constmismatch.html). So I rarely use const, except eg for taking string literals.
In the particular instance of the utf8 ranges stuff it's fine I guess, because it's a leaf function so the virality of const is limited, but I'd rather not have that creep up if we can avoid it.
I think your change makes things clearer, so I'll apply it directly and we'll just rebase on that.

1. I don't find const really useful in C, and it has an annoying virality. Also sometimes clang emits warnings when passing non-const pointers to const parameters (eg for nested pointer types). I find the rationale for this very contrived (https://c-faq.com/ansi/constmismatch.html). So I rarely use const, except eg for taking string literals. In the particular instance of the utf8 ranges stuff it's fine I guess, because it's a leaf function so the virality of const is limited, but I'd rather not have that creep up if we can avoid it. 2. I think your change makes things clearer, so I'll apply it directly and we'll just rebase on that.

👍 1

ilidemi reviewed 2023-09-28 23:05:33 +00:00

src/orca.zig Outdated

													
				@ -0,0 +1389,4 @@

				// [GRAPHICS]: resources

				//------------------------------------------------------------------------------------------

				pub const Surface = extern struct {

ilidemi commented

2023-09-28 23:05:33 +00:00

Is there a reason for the function structs to be extern?

rdunnington commented

2023-09-28 23:16:46 +00:00

No, structs that only have functions don't need to be extern, but I think in this case Surface does since it's a handle and has some data inside

No, structs that only have functions don't need to be extern, but I think in this case `Surface` does since it's a handle and has some data inside

ilidemi marked this conversation as resolved

ilidemi reviewed 2023-09-28 23:07:39 +00:00

src/orca.zig Outdated

													
				@ -0,0 +1633,4 @@

				// [FILE IO] basic API

				//------------------------------------------------------------------------------------------

				const File = extern struct {

ilidemi commented

2023-09-28 23:07:39 +00:00

Should non-core data structs be put into the namespace as well? Arena/Vec2 makes sense at the top level, File/IO/UI stuff seems like it can be contained

rdunnington commented

2023-09-28 23:18:40 +00:00

You mean like putting File/FileType/IoReq/etc into a struct named io, kind of like how the log stuff is namespaced into log? If so, I think that makes sense.

You mean like putting File/FileType/IoReq/etc into a struct named `io`, kind of like how the log stuff is namespaced into `log`? If so, I think that makes sense.

ilidemi commented

2023-09-28 23:28:48 +00:00

What looked confusing to me was how functions are namespaced but the structs local to that module are not. Guess something like File is used for fonts and images too so it's a judgement call but something like FileDialog is pretty clear. I'm coming at this from UI angle where pretty much all of the structs are local.

ilidemi commented

2023-09-28 23:38:12 +00:00

Ohh I think I get it. File and Surface are legit structs here, just with a ton of methods. I thought they're more like function namespaces. Apologies for confusion.

ilidemi commented

2023-09-28 23:41:23 +00:00

I am still confused, oc_file_size and oc_file_open_with_request are declared in the exact same way but former has File as the self argument and the latter doesn't. Does that translate into a method call and a namespaced function respectively?

I am still confused, `oc_file_size` and `oc_file_open_with_request` are declared in the exact same way but former has File as the self argument and the latter doesn't. Does that translate into a method call and a namespaced function respectively?

rdunnington commented

2023-09-29 01:37:26 +00:00

In Zig, calling funtions on structs is just syntax sugar for passing it as the first argument to the function. So these are equivalent:

_ = oc.File.size(file);
_ = file.size();

And you can have other functions that don't pass the struct type as the first argument, you just have to call them explicitly on the struct type and not an instance, otherwise you'll get an error:

var file1: File = File.openWithRequest(path, rights, flags); // OK
var file2: File = file.openWithRequest(path, rights, flags); // Compile error saying that File is not a Str8

Hopefully that clears things up?

In Zig, calling funtions on structs is just syntax sugar for passing it as the first argument to the function. So these are equivalent: ``` _ = oc.File.size(file); _ = file.size(); ``` And you can have other functions that don't pass the struct type as the first argument, you just have to call them explicitly on the struct type and not an instance, otherwise you'll get an error: ``` var file1: File = File.openWithRequest(path, rights, flags); // OK var file2: File = file.openWithRequest(path, rights, flags); // Compile error saying that File is not a Str8 ``` Hopefully that clears things up?

rdunnington commented

2023-09-29 01:55:21 +00:00

So I guess to answer your question, directly, yes. :P

ilidemi commented

2023-09-29 02:50:55 +00:00

And the latter one goes under File because it's a sort of a factory method. All makes sense, thank you for the explanation!

👍 1

ilidemi marked this conversation as resolved

rdunnington force-pushed zig_ffi from 625301de56 to b14567753a

2023-09-29 02:58:03 +00:00

Compare

rdunnington force-pushed zig_ffi from b14567753a to e6feba1ccd

2023-09-29 03:04:43 +00:00

Compare

rdunnington commented

2023-09-29 03:06:23 +00:00

@MartinFouilleul There is one other case I ran into tonight where const is important to zig - all function inputs are always const. So if I want to pass one of them to a function, I'll need to make a non-const copy if the function doesn't handle it. This is another pretty compelling reason to have "const correctness" on the zig side.

I think it's probably fine if the zig function interfaces decorate types as const and the C versions don't... oc_font_create_from_path works just fine without it.

@MartinFouilleul There is one other case I ran into tonight where const is important to zig - all function inputs are always const. So if I want to pass one of them to a function, I'll need to make a non-const copy if the function doesn't handle it. This is another pretty compelling reason to have "const correctness" on the zig side. I think it's probably fine if the zig function interfaces decorate types as const and the C versions don't... `oc_font_create_from_path` works just fine without it.

rdunnington commented

2023-09-29 03:09:50 +00:00

I also made one more change you may want to look at in memory.h. Check out oc_arena_push_aligned(). This was necessary because zig aborts in debug mode if you do an unaligned pointer type cast, which I ran into when implementing Arena.pushType() and Arena.pushArray().

I also made one more change you may want to look at in `memory.h`. Check out `oc_arena_push_aligned()`. This was necessary because zig aborts in debug mode if you do an unaligned pointer type cast, which I ran into when implementing `Arena.pushType()` and `Arena.pushArray()`.

MartinFouilleul commented

2023-09-29 08:16:18 +00:00

Ha, I wanted to get to that eventually, but you beat me to it, nice! I'll just add this to main and rebase, so that this is separate from the zig-specific commits

MartinFouilleul force-pushed zig_ffi from e6feba1ccd to 9cab6031d0

2023-09-29 09:05:08 +00:00

Compare

rdunnington force-pushed zig_ffi from 9cab6031d0 to e6feba1ccd

2023-09-30 00:11:31 +00:00

Compare

rdunnington force-pushed zig_ffi from e6feba1ccd to 9c4e2125c1

2023-09-30 00:17:30 +00:00

Compare

rdunnington added 1 commit 2023-09-30 03:03:43 +00:00

eb60729721

zig bindings: string refinements/bugfixes

rdunnington added 1 commit 2023-09-30 03:16:57 +00:00

3e6cd07b34

revert const changes to oc_font_create_from_*

* Martin has said the orca style is to avoid const, so we will avoid adding it where it isn't already present
* Zig bindings doesn't need the corresponding C function to have const-decorated types

rdunnington added 1 commit 2023-09-30 03:42:20 +00:00

49213353fc

zig bindings: namespace clock/canvas functions

rdunnington added 3 commits 2023-10-03 03:17:24 +00:00

23378edfec

zig bindings: namespace File data types and Io data/functions

c31c81bd9c

zig bindings: minor API cleanup

* deleting invalid function binding
* minor reorg stuff

024a528afa

zig sample:

* generate a gradient image and draw it
* rbga8 color helpers and some const tweaks

rdunnington added 1 commit 2023-10-03 03:18:41 +00:00

47cba86c03

zig bindings: oc_surface_render_commands

rdunnington added 3 commits 2023-10-05 04:51:20 +00:00

fd308f9a3a Zig UI demo

30d45cd7fd Comptime enum counts, namespace UI, clarify names/labels

f1727dbc8d Text box arena, ui init func

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

Sign in to join this conversation.

No reviewers

No Label

macOS

windows

No Milestone

No project

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hmn/orca#140