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

Add .WASM support for Jai bindgen #1128

Merged
merged 2 commits into from
Oct 23, 2024
Merged

Conversation

colinbellino
Copy link
Contributor

@colinbellino colinbellino commented Oct 16, 2024

I'm putting together a JAI + WASM example, so i thought i'd update the bindgen for Jai while i'm at it ;)

The corresponding commit in sokol-jai: colinbellino/sokol-jai@4a679fd

@colinbellino
Copy link
Contributor Author

colinbellino commented Oct 16, 2024

I'm not entirely sure why the pipeline is failing in test-d, will investigate 🤔
Edit: Looks like the second time the test passed, maybe a flaky check?

@colinbellino colinbellino marked this pull request as ready for review October 16, 2024 15:55
@floooh
Copy link
Owner

floooh commented Oct 18, 2024

Apologies for not responding yet. I'll try to set aside a day over the weekend to look into PRs.

@floooh
Copy link
Owner

floooh commented Oct 18, 2024

PS looks like that failed CI run was some unrelated networking hickup:

/Users/runner/work/sokol/sokolbuild.zig.zon:27:20: error: unable to discover remote git server capabilities: UnknownHostName

@floooh floooh merged commit c256380 into floooh:master Oct 23, 2024
32 checks passed
@floooh
Copy link
Owner

floooh commented Oct 23, 2024

Sorry, should have merged that PR sooner, since it's a trivial update. I really need to catch up a bit on PRs :)

Btw something unrelated:

I'm now at a point where I'd need to contribute to https://github.com/colinbellino/sokol-jai (because there's a API breaking change in the pipeline which requires updating the shaders and samples.

It will look similar to this: kassane/sokol-d#31

I wonder if it makes sense if you give me a couple of rights for the https://github.com/colinbellino/sokol-jai repo, just enough to create branches and PRs. That will make it a bit simpler to create PRs without having to maintain my own fork (I would still do regular PRs for you to review and not commit directly into main, but at least I wouldn't need to maintain my own fork).

Also, I still haven't tried yet to get into the Jai Beta, that would also make a couple of things easier :D

I will probably need to do a "blind" PR at first which might not actually build and need some fixups (e.g. I might need your help for that).

@colinbellino
Copy link
Contributor Author

colinbellino commented Oct 24, 2024

Thanks for merging this ;)

I can probably do the update to colinbellino/sokol-jai, is the incoming change documented somewhere? (just came back from vacations, haven't really dug into this yet).
I've added you as collaborator on the project just in case.

(For the beta if you want to get in at some point, do let me know in private. I'm sure we can figure something out with Jon.)

@floooh
Copy link
Owner

floooh commented Oct 24, 2024

Thanks for adding me. I'll create a draft PR with more info, but in general the things that need to be fixed in the sample code:

  • the code-generated vertex attribute and bind slot constants have changed to make a bit more sense:
    • vertex attr slot constants now contain the shader program name instead of the shader snippet name (e.g. ATTR_vs_pos becomes ATTR_cube_pos
    • resource bind slot names now contains the type (e.g. SLOT_tex becomes IMG_tex (sampler slot constants become SMP_*, storage buffers SBUF_* and uniform block slots UB_*
  • in the sg_bindings struct, the shader stages are gone (e.g. bind.fs.images[SLOT_tex] = img; becomes bind.images[IMG_tex] = img;
  • in the sg_apply_uniforms() call, the shader stage arg is gone (e.g. sg_apply_bindings(SG_SHADERSTAGE_VS, SLOT_vs_params, ...) becomes sg_apply_bindings(UB_vs_params, ...)

...but as I said, I'll try to apply those changes blindly, and the time to merge this stuff is also still a bit out (I'm working checkbox by checkbox though this checklist: #1111)

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