search

floooh / sokol-zig

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

wasm support and zig-0.12.0 fixes #50

Closed kassane closed 7 months ago

kassane commented 7 months ago

Tested

Steps

Note: This is experimental, and some examples get black screen on browser or get errors!!

Reference

floooh commented 7 months ago

I only skimmed the changes so far (will try to take some time to actually understand it later). But in the meantime: Some of those changes might overlap with the changes I did for the sokol module in the package branch:

https://github.com/floooh/sokol-zig/blob/2a0d82e1716974d4ff6f9e8a4bf8622b50e5704a/build.zig#L88-L114

...this basically just expects that a --sysroot parameter is provided when the project is built for wasm which points into the Emscripten SDK and sets up a required header search path and C define for building the sokol module.

...the build.zig doesn't directly allow to build the samples as wasm though (as this requires more work to use emcc as the linker), I've only done this in the pacman.zig repository in the package branch:

https://github.com/floooh/pacman.zig/blob/d3d82da497d6cf39d1b99d66d94c0ea07fe10d51/build.zig#L50-L124

PS: also note this workaround around a package manager issue, not sure if that's fixed yet:

https://github.com/floooh/pacman.zig/blob/d3d82da497d6cf39d1b99d66d94c0ea07fe10d51/build.zig#L11-L15

PPS: also see:

https://github.com/ziglang/zig/issues/16711

kassane commented 7 months ago

wow! I'll analyze the suggestion and adjust with the new zig update which breaks the previous build.

kassane commented 7 months ago

My suggestion

target.result.isWasm(): std.Target: wasm32 or wasm64 https://github.com/ziglang/zig/blob/04ac028a2cd20ea430dd6a05b956363ec182b6ff/lib/std/Target.zig#L942-L947

from: https://github.com/floooh/sokol-zig/blob/2a0d82e1716974d4ff6f9e8a4bf8622b50e5704a/build.zig#L18-L26 https://github.com/floooh/sokol-zig/blob/2a0d82e1716974d4ff6f9e8a4bf8622b50e5704a/build.zig#L106-L109 to: https://github.com/floooh/sokol-zig/blob/e176d2b6bc79b5b13b7fbf7822aec958ba1c2792/build.zig#L19-L34 https://github.com/floooh/sokol-zig/blob/e176d2b6bc79b5b13b7fbf7822aec958ba1c2792/build.zig#L231-L237

floooh commented 7 months ago

Phew, FYI I just fixed build.zig for the latest build system changes in the nightly version (in a new branch zig-0.12.0):

https://github.com/floooh/sokol-zig/tree/zig-0.12.0

I started this branch with the sokol-package branch, so that at least there won't be two version (package and non-package) from there on.

This is getting a bit messy with the wasm changes on top, I hope the build system APIs stabilize a bit better soon-ish.

kassane commented 7 months ago

Phew, FYI I just fixed build.zig for the latest build system changes in the nightly version (in a new branch zig-0.12.0):

https://github.com/floooh/sokol-zig/tree/zig-0.12.0

I started this branch with the sokol-package branch, so that at least there won't be two version (package and non-package) from there on.

Amazing!! :+1:

This is getting a bit messy with the wasm changes on top, I hope the build system APIs stabilize a bit better soon-ish.

I also hope! Because right now, I was looking into a new issue in sokol-d for windows-msvc target. Since the latest zig update 0.12.0-dev.2043+6ebeb85ab.

kassane commented 7 months ago

Rebased on branch: package

floooh commented 7 months ago

FYI I think I will spend the next couple of days with a couple of fixes:

I had started a zig-0.12.0 branch in sokol-zig which has been updated to the latest zig build system API changes (I'm not happy with some changes I did, and will use some of your changes which make more sense, like lib.target.isDarwin() => lib.rootModuleTarget().isDarwin() (I hadn't seen this new rootModuleTarget() helper).

I think I will just merge the current master branch back into zig-0.11.0 and treat this as the stable target for people using zig 0.11.x, and instead merge zig-0.12.0 into master (meaning the sokol-zig master branch would be "bleeding edge").

This also means that the sokol-zig master becomes 'package-capable', and I will move the master branches of my example/test projects pacman.zig and kc85.zig to also use the package manager.

I'll keep working on the zig-0.12.0 branch until everything is ready. I'll only do minimal changes to the already existing wasm support and try not to interfere too much with your wip changes :)

PS: while at it, I will also check if some of the package-manager related workarounds in pacman.zig are actually still needed.

floooh commented 7 months ago

PPS: One question that popped into my mind... I wonder if it makes sense to split the examples into a separate build.zig (maybe in a subdirectory). For instance adding Emscripten/WASM build support for the samples adds a lot more stuff to the root build.zig which wouldn't be needed for the sokol package alone.

Thoughts?

kassane commented 7 months ago

I had started a zig-0.12.0 branch in sokol-zig which has been updated to the latest zig build system API changes

Redesigning the new api is pretty tedious, besides looking more verbose. However, the real difficulty is knowing how the zig will handle the wasm target. The use of emcc is minimized when a wasm executable is generated by zig. This is because the zig compiler itself will have to do the linking, exporting, and relocating of functions.

Thoughts?

I wonder if it makes sense to split the examples into a separate build.zig (maybe in a subdirectory). For instance adding Emscripten/WASM build support for the samples adds a lot more stuff to the root build.zig which wouldn't be needed for the sokol package alone.

That could be! However, this reorganization of build module would require a more complete configuration of emscripten.

Something similar here, perhaps:

floooh commented 7 months ago

The use of emcc is minimized when a wasm executable is generated by zig. This is because the zig compiler itself will have to do the linking, exporting, and relocating of functions.

In case of sokol, emcc will need to do the linking, because the sokol C sources use Emscripten-specific magic like embedded Javascript snippets via EM_JS() which need special handling in the linker step. Also the output must not just be a .wasm file, but also a .html and .js file.

For just compiling the sokol static link library, the Zig compiler is sufficient (with the wasm32-emscripten target), but for linking additional stuff needs to happen which is only implemented in emcc.

So one way or another, if we want to have the sokol-zig examples built out of the box (for wasm), we need to be able to callemcc.

To just build the sokol link library from the C sources, only a path to the Emscripten SDK sysroot is needed to provide the Emscripten C headers, but the compilation can be done by the Zig compiler.

floooh commented 7 months ago

kassane marked this pull request as ready for review 1 minute ago

...wait... did you actually get it to work?

floooh commented 7 months ago

This is pretty awesome tbh, didn't know that this works (but hoped it would):

.{
    .name = "sokol-zig",
    .version = "0.0.0",
    .minimum_zig_version = "0.12.0",
    .dependencies = .{
        .emsdk = .{
            .url = "git+https://github.com/emscripten-core/emsdk#0bbae74935d57ff41739648c12cf90b56668398f",
            .hash = "1220f1340cd871b444021c600661f921f96091ce0815fa43008528f4844cece7e245",
        },
    },
    .paths = .{""},
}

...means I can probably do something similar with the sokol-shc compiler...

floooh commented 7 months ago

woah, what sorcery is this :D

image
kassane commented 7 months ago

woah, what sorcery is this :D

https://github.com/floooh/sokol-zig/blob/ae79928797b0b17671574e13a1a29dd96c1d889a/build.zig#L398-L400

kassane commented 7 months ago

All tests :heavy_check_mark: \o/

floooh commented 7 months ago

This is totally awesome....

The clear-sample works (which means all the Emscripten stuff is working).

image

...for the samples which require shaders the GLES3/WebGL2 shaders seem to be missing (which figures because they are not currently in the generated shader files here: https://github.com/floooh/sokol-zig/tree/master/src/examples/shaders, and the args here:

https://github.com/floooh/sokol-zig/blob/24134d507a9b6d61ea1c438c0ec2ffd19530a93c/build.zig#L248

...need to be extended like this:

glsl330:metal_macos:hlsl4:glsl300es

(maybe also :wgpu but let's not get ahead of ourselves...)

We should also replace that ugly Emscripten default webpage which our own shell.html... but that's also no show stopper for the merge, I can take care of this polishing stuff...

In any case, I will abandon my wip work on pacman.zig and kc85.zig and focus on this MR first.

Good stuff :)

floooh commented 7 months ago

...not today though (but expect some potential review feedback bits and pieces over the next few days).

kassane commented 7 months ago

This is pretty awesome tbh, didn't know that this works (but hoped it would):

.{
    .name = "sokol-zig",
    .version = "0.0.0",
    .minimum_zig_version = "0.12.0",
    .dependencies = .{
        .emsdk = .{
            .url = "git+https://github.com/emscripten-core/emsdk#0bbae74935d57ff41739648c12cf90b56668398f",
            .hash = "1220f1340cd871b444021c600661f921f96091ce0815fa43008528f4844cece7e245",
        },
    },
    .paths = .{""},
}

You can update the package using the command:

# (over)write [zon] with new commit hash pkg
>$ zig fetch --save=emsdk "git+https://github.com/emscripten-core/emsdk#{latest-commit-hash}"

...means I can probably do something similar with the sokol-shc compiler...

Yeah! I was also thinking about this possibility in the sokol-d project.

floooh commented 7 months ago

Hmm strange, I just updated the package branch with the latest changes from master, but the PR diff view still shows differences in sokol_app.h, sokol_gfx.h and sokol_log.h which shouldn't be there anymore...

PS: probably a Github bug: https://github.com/isaacs/github/issues/750

floooh commented 7 months ago

...changing base branches to update the view didn't work though hmm...

floooh commented 7 months ago

I added a couple of code review comments / change requests.

Two things are still mysterious to me:

kassane commented 7 months ago

...changing base branches to update the view didn't work though hmm...

Do you want to rebase to branch 0.12.0? Can I also revert the .c files? Just keep build.zig for changes.

floooh commented 7 months ago

Do you want to rebase to branch 0.12.0?

TL;DR: keep this PR based on the package branch, and I will figure something out :D

That was my plan yesterday, but there will be collisions with the changes I already did there (and those changes don't make sense anymore, I would like to use your branch as the "0.12.0 reference branch").

In general my plan is:

...e.g. the main branch becomes the 'bleeding edge'.

Now I already started a 0.12.0 branch prematurely. It would have been better to merge your changes back into the package branch and then merge the package branch back into main, and only create a 0.12.0 branch when Zig 0.12.0 has been released.

I'll check out your changes later today. After merging back into package I will probably fix the remaining issues in the samples that currently produce a black screen, then update my pacman.zig and kc85.zig projects accordingly and then merge everything back into master/main.

And I think I will also generally switch everything (meaning my example projects) to use the package manager approach, and declare the old way of integrating the bindings as 'deprecated' (but probably still keep it working for a while).

floooh commented 7 months ago

Can I also revert the .c files? Just keep build.zig for changes.

Yes, ideally the PR would only affect the main.yml, build.zig and build.zig.zon file.

As I said above, I'm not sure why Github even still shows changes for the .h files, since I updated the package branch yesterday, but the .h file changes that are still shown look like the PR is against an outdated version of those files...

Maybe rebasing again on top package would fix that?

kassane commented 7 months ago

Maybe rebasing again on top package would fix that?

Done!

floooh commented 7 months ago

Done!

Yay that helped, thanks! Looking through your changes now and if I'm happy I'll merge it (but slight change of plans, I'll try to merge into a new wasm_zig-0.12.0 branch created from package, that way the package branch remains undisturbed (and will be 'deprecated' from then on).

floooh commented 7 months ago

Interesting, the captureStdOut trick doesn't seem to hide the emsdk output (on macOS I mean). But anyway, I'm happy with the PR as is and will merge now, and then start fixing the remaining issues (black screen samples etc...) in a branch wasm_zig-0.12.0

floooh commented 7 months ago

Ok merged! Slight correction: target branch name is https://github.com/floooh/sokol-zig/tree/zig-0.12.0-wasm

...I'll continue working on this until it's ready for merging into master and will keep you posted (I'll probably open a new PR where we can discuss changes if needed).

floooh commented 7 months ago

PS: Many thanks for this awesome PR :)

kassane commented 7 months ago

More fixes