Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasm support and zig-0.12.0 fixes #50

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

kassane
Copy link
Contributor

@kassane kassane commented Jan 4, 2024

Tested

  • OS: Linux (archlinux)
  • Arch: x86_64 (zen3)
  • zig version: 0.12.0-dev.2150+63de8a598

Steps

  • zig get emsdk using zon file.
  • run emsdk (download, extracts files)
  • build libsokol w/ emscripten (os-tag)
  • in examples run emcc and emrun

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

Reference

@floooh
Copy link
Owner

floooh commented Jan 5, 2024

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:

sokol-zig/build.zig

Lines 88 to 114 in 2a0d82e

// build sokol into a static library
pub fn buildLibSokol(b: *Build, options: LibSokolOptions) !*CompileStep {
const is_wasm = options.target.getCpu().arch == .wasm32;
// special case wasm, must compile as wasm32-emscripten, not wasm32-freestanding
var target = options.target;
if (is_wasm) {
target.os_tag = .emscripten;
}
const lib = b.addStaticLibrary(.{
.name = "sokol",
.target = target,
.optimize = options.optimize,
.link_libc = true,
});
if (is_wasm) {
// need to add Emscripten SDK include path
if (b.sysroot == null) {
std.log.err("Must provide Emscripten sysroot via '--sysroot [path/to/emsdk]/upstream/emscripten/cache/sysroot'", .{});
return error.Wasm32SysRootExpected;
}
const include_path = try std.fs.path.join(b.allocator, &.{ b.sysroot.?, "include" });
defer b.allocator.free(include_path);
lib.addIncludePath(.{ .path = include_path });
lib.defineCMacro("__EMSCRIPTEN__", "1");
}

...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:

ziglang/zig#16711

@kassane
Copy link
Contributor Author

kassane commented Jan 5, 2024

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

@kassane
Copy link
Contributor Author

kassane commented Jan 5, 2024

My suggestion

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

from:

sokol-zig/build.zig

Lines 18 to 26 in 2a0d82e

pub const LibSokolOptions = struct {
target: CrossTarget,
optimize: OptimizeMode,
build_root: ?[]const u8 = null,
backend: Backend = .auto,
force_egl: bool = false,
enable_x11: bool = true,
enable_wayland: bool = false,
};

sokol-zig/build.zig

Lines 106 to 109 in 2a0d82e

if (b.sysroot == null) {
std.log.err("Must provide Emscripten sysroot via '--sysroot [path/to/emsdk]/upstream/emscripten/cache/sysroot'", .{});
return error.Wasm32SysRootExpected;
}

to:

sokol-zig/build.zig

Lines 19 to 34 in e176d2b

pub const LibSokolOptions = struct {
optimize: OptimizeMode = .Debug,
backend: Backend = .auto,
force_egl: bool = false,
enable_x11: bool = true,
enable_wayland: bool = false,
sysroot: ?[]const u8 = null,
package: ?*Builder.Dependency = null,
fn packagePath(self: LibSokolOptions, b: *Builder) []const u8 {
if (self.package) |dep|
return dep.path("").getPath(b)
else
return "";
}
};

sokol-zig/build.zig

Lines 231 to 237 in e176d2b

if (target.result.isWasm()) {
options.package = b.dependency("emsdk", .{});
if (b.sysroot) |sysroot|
options.sysroot = sysroot
else
options.sysroot = b.pathJoin(&.{ options.packagePath(b), "upstream", "emscripten", "cache", "sysroot" });
}

@floooh
Copy link
Owner

floooh commented Jan 6, 2024

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
Copy link
Contributor Author

kassane commented Jan 6, 2024

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!! 👍

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 kassane changed the base branch from master to package January 7, 2024 12:56
@kassane
Copy link
Contributor Author

kassane commented Jan 7, 2024

Rebased on branch: package

@floooh
Copy link
Owner

floooh commented Jan 8, 2024

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
Copy link
Owner

floooh commented Jan 8, 2024

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
Copy link
Contributor Author

kassane commented Jan 8, 2024

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
Copy link
Owner

floooh commented Jan 8, 2024

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.

@kassane kassane marked this pull request as ready for review January 10, 2024 18:15
@floooh
Copy link
Owner

floooh commented Jan 10, 2024

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

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

@floooh
Copy link
Owner

floooh commented Jan 10, 2024

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
Copy link
Owner

floooh commented Jan 10, 2024

woah, what sorcery is this :D

image

@kassane
Copy link
Contributor Author

kassane commented Jan 10, 2024

woah, what sorcery is this :D

sokol-zig/build.zig

Lines 398 to 400 in ae79928

// get artifacts from zig-cache, no need zig-out
emcc.addArtifactArg(options.lib_sokol);
emcc.addArtifactArg(example);

@kassane
Copy link
Contributor Author

kassane commented Jan 10, 2024

All tests ✔️ \o/

@floooh
Copy link
Owner

floooh commented Jan 10, 2024

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:

"glsl330:metal_macos:hlsl4",

...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
Copy link
Owner

floooh commented Jan 10, 2024

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

@kassane
Copy link
Contributor Author

kassane commented Jan 10, 2024

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 floooh changed the base branch from package to zig-0.12.0 January 11, 2024 16:10
@floooh floooh changed the base branch from zig-0.12.0 to package January 11, 2024 16:13
@floooh
Copy link
Owner

floooh commented Jan 11, 2024

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: isaacs/github#750

@floooh floooh changed the base branch from package to zig-0.12.0 January 11, 2024 16:21
@floooh floooh changed the base branch from zig-0.12.0 to package January 11, 2024 16:21
@floooh
Copy link
Owner

floooh commented Jan 11, 2024

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

build.zig Outdated Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
build.zig Outdated Show resolved Hide resolved
build.zig Outdated Show resolved Hide resolved
build.zig Outdated Show resolved Hide resolved
build.zig Outdated Show resolved Hide resolved
build.zig Outdated Show resolved Hide resolved
build.zig Outdated Show resolved Hide resolved
build.zig Outdated Show resolved Hide resolved
build.zig Outdated Show resolved Hide resolved
@floooh
Copy link
Owner

floooh commented Jan 11, 2024

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

Two things are still mysterious to me:

@kassane
Copy link
Contributor Author

kassane commented Jan 11, 2024

...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.

@kassane kassane requested a review from floooh January 11, 2024 20:03
@floooh
Copy link
Owner

floooh commented Jan 12, 2024

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:

  • change the 'branch philosophy' of sokol-zig so that the main branch works with the latest Zig dev version, instead of the stable version (since most Zig users currently seem to work on the dev version anyway)
  • ...and instead turn the 0.11.0, 0.10.0 etc... branches into "archival branches" which keep working with that version of Zig, but might not have the latest sokol headers.

...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
Copy link
Owner

floooh commented Jan 12, 2024

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
Copy link
Contributor Author

kassane commented Jan 12, 2024

Maybe rebasing again on top package would fix that?

Done!

@floooh
Copy link
Owner

floooh commented Jan 12, 2024

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
Copy link
Owner

floooh commented Jan 12, 2024

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 floooh changed the base branch from package to zig-0.12.0-wasm January 12, 2024 17:40
@floooh floooh changed the title Wasm support wasm support and zig-0.12.0 fixes Jan 12, 2024
@floooh floooh merged commit 0ad7d85 into floooh:zig-0.12.0-wasm Jan 12, 2024
3 checks passed
@floooh
Copy link
Owner

floooh commented Jan 12, 2024

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
Copy link
Owner

floooh commented Jan 12, 2024

PS: Many thanks for this awesome PR :)

@kassane kassane deleted the wasm_zig-0.12.0 branch January 12, 2024 17:44
@kassane
Copy link
Contributor Author

kassane commented Jan 12, 2024

More fixes

@kassane kassane mentioned this pull request Jan 4, 2024
floooh added a commit that referenced this pull request Jan 17, 2024
Finalize WASM and zig-0.12.0 support (see #50)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants