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

Load gl.js from WASM #97

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

tversteeg
Copy link
Contributor

@tversteeg tversteeg commented May 16, 2020

This PR includes the gl.js source in the WASM binary.

How it works

  1. In your index.html you load loader.js instead of gl.js.
  2. The compiled Rust WASM exports the load_gl_js function which uses an imported load_js function.
  3. The loader compiles the WASM file and creates an empty function for all requested imports and an implemented load_js function.
  4. The loader instantiates the WASM and calls load_gl_js on it, this will load all the functions from gl.js.
  5. The loader overwrites its imports with the ones from gl.js.
  6. Miniquad is started!

There are few upsides and a few downsides to this approach:

Pros

  • The gl.js version is always correct since it's embedded, only the versions of loader.js might differ but since that doesn't contain any runtime logic code that shouldn't happen too often.
  • Possibly no extra files in the future, loader.js might be small enough to include directly in index.html, which will result in just the WASM file and index.html.
  • GitHub wont think that your repository is a JavaScript one instead of a Rust one. 😁

Cons

  • gl.js cannot be changed without recompiling.
  • Debugging might be harder since an eval is used to load gl.js, removing file information.

Possible improvements

  • gl.js could be minified as a build step before being included.

@nokola
Copy link
Contributor

nokola commented May 16, 2020

I like this the ability to ship gl.js inline in the WASM - a very good improvement towards making easier to distribute versions and be confident things will "just work"!

Adding few more:

Pros

  1. External contributions from GitHub forks don't have to rely on copying custom gl.js around and modifying index.html to test their changes locally
  2. Feels better to know there's no need to load gl.js from a third-party website or remember to version it manually for different WASMs (adding to existing pro above)

Cons

Not sure if any of these matter, they are all likely insignificant:

  1. `gl.js' will start loading after the WASM loads, not in parallel
  2. browser cannot cache the contents of gl.js and/or cache a precompiled version of gl.js across different web site open-s. It will have to parse gl.js every time when loading different wasm-s
  3. gl.js will be included in every WASM, which is not a big deal (only 6.8k)

My intention is just to add some thoughts, I think the feature is very good as-is. Initially I thought we may need a way to turn it off, but after rethinking it I think we don't need to add an off switch (e.g. through features) because there's no compelling reason for it.

@not-fl3
Copy link
Owner

not-fl3 commented May 16, 2020

Follow up from todays Discord discussion in addition to @nokola's comment:

Problems with current implementation:

  • gl.js requires manual hosting and updating
  • no version check and weird errors when gl.js and miniquad version mismatch
  • gl.js is not actually gl.js, but wasm loader + gl + some helpers, wich is confusing and hard to contribute into

Opinions and requirements on desired rust/js interop:

  • JS modification during development should be simple without rust recompilation each time
  • It should be possible to host wasm and js separately in production environment(?)
  • It is nice to have reproducable "build artifacts" for each build
  • Would be nice to automatically take the JS code from depdencies crates, see quad-snd as an example of a crate with custom JS code
  • Writing project specific JS code should be simple and straightforward
  • The errors of mismatched gl.j/dependencie JS code versions should be clear and explicit
  • It is fine to use unminified JS for development
  • gl.js should be renamed for sure!

Proposed solutions:

  • embed .js into .wasm and extract it back to JS with special loader.js script
  • add optional step for user build.rs script, that would emit JS file into build directory alongside with .wasm, maybe also copy .wasm + .js + some default .html to /target/miniquad-artifacts for easier development
  • Just add version checks to all the JS files and still require manual copying files

@nokola
Copy link
Contributor

nokola commented May 16, 2020 via email

@tversteeg
Copy link
Contributor Author

I had another idea that might solve the problems: what if we make it a cargo disabled-by-default feature to have a separate gl.js?

Pros:

  • New users will not have the hassle of copying the gl.js file
  • Development on gl.js can still easily be done with the feature enabled
  • "Power users" have the option to tweak all JS as they wish

Cons:

  • More cargo features might be seen as a negative
  • Need to maintain 2 loaders

@nokola
Copy link
Contributor

nokola commented May 18, 2020

Feature might work, however, isn't it possible to also copy the gl.js to keep this Pro even without embedding?

New users will not have the hassle of copying the gl.js file

@nokola
Copy link
Contributor

nokola commented May 18, 2020

I like the "keep it simple" we have applied so far, perhaps just copying gl.js for the user during build and somehow making build/run WASM "just work in 1-step" would be best. Disclaimer: I haven't look at how easy it is to do, maybe through build.rs?

@tversteeg
Copy link
Contributor Author

I like the "keep it simple" we have applied so far, perhaps just copying gl.js for the user during build and somehow making build/run WASM "just work in 1-step" would be best. Disclaimer: I haven't look at how easy it is to do, maybe through build.rs?

As far as I know it's not possible to copy stuff outside the libraries' directories in Rust.

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.

3 participants