-
Notifications
You must be signed in to change notification settings - Fork 865
Use Emscripten fetch in networkfilemanager #4627
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
base: master
Are you sure you want to change the base?
Conversation
|
fyi @willcohen |
|
BTW, the github action is creating an artifact with the js and wasm files. |
rouault
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool!
src/networkfilemanager.cpp
Outdated
| out_error_string[0] = '\0'; | ||
| } | ||
|
|
||
| // cast 64b to 32b. Hopefully we dont download more than 2GB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that comment accurate ? I believe 64-bit WASM builds are possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetch->numBytes is 64 bits, so it is possible. However size_t is 32 bits (surprise!)
I have the impression that the function calling with size_to_read was not expecting the 32 bits limit, does it?
What should I do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point was that on a 64-bit WASM build size_t is going to be 64 bit, hence the comment about 64->32bit cast wouldn't be accurate. That's a minor point.
That cast isn't much of a concern. We shouldn't request more than 4 GB at once...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, pretty recent: https://webassembly.org/news/2025-09-17-wasm-3.0/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just removed the comment.
scripts/ci/emscripten/Dockerfile
Outdated
| # Define standard locations for building and installing | ||
| ENV BUILD_DIR="/build" | ||
| ENV INSTALL_DIR="/usr/local/wasm" | ||
| ENV PROJ_VERSION="9.7.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unneeded in this repository as we use the git checkout
| ENV PROJ_VERSION="9.7.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea was that the Dockerfile and build_wasm.sh could be used for the "normal users" when needed. In that case it would be useful just to set the release or branch. (I am thinking on making configurable the EXPORTED_FUNCTIONS parameter via a file, something already available in emcc. In a later PR probably)
I agree that having a default version is bad, I will remove it in any case.
Maybe having the scripts somewhere else in the code-tree makes it more accessible.
Said that, I don't have really a strong opinion. The "user" can always clone the code themselves. I can remove those options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add some comment in the files that the version pin isn't used by PROJ CI ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed from the Dockerfile. In the script PROJ_TAG has to be explicitly set, or populate the proj_src folder with your local copy (usually mounting a volume).
The code (or tag) to compile has to be explicitly provided.
11318a5 to
e76ffd8
Compare
|
I am positively thrilled to see this moving forward. I’m currently working on putting proj-wasm to work on some projects to reproject transpiled-to-WASM JTS objects and or allow them to work with CRSes beyond just EPSG in the browser and once I get that a little integrated I will be very excited to be able to seamlessly add grids in too. Long way of saying I may not be able to use this downstream for a few weeks, but I’m working up to it. |
e76ffd8 to
b36295b
Compare
The main purpose of this PR is an implementation in the networkfilemanager that substitutes cURL when running in wasm. For that purpose emscripten_fetch is used, as suggested in emscripten-core/emscripten#3270 (comment)
The added implementation in
networkfilemanager.cppis a copy of the cURL implementation, trying to mimic it as much as possible. Emscripten does convert it into whatever code in wasm + js, using the fetch in the browser.To enable this feature, PROJ must be compiled with the option
ENABLE_EMSCRIPTEN_FETCH, that by default is OFF. It works only inemscripten.As a needed tool for my tests I implemented a Docker image and a build script to produce a wasm compilation, and test it. It has been very useful, as it showed up some problems due to the synchronous fetch used in PROJ. Already having the build infrastructure, I added it as a github action. Anybody can take that to produce a wasm of PROJ.
emscripten documentation says clearly that to get synchronous fetch the
-pthreadflag is needed. It also needs that the javascript calls are done in a Web Worker, not in the main thread. I was not able to make--proxy-to-workerwork, as there is nomainto use it. The build script is then using the-pthreadflag for all the dependencies that have to be compiled:zlib,libtiffandlibsqlite3.If a developer doesn't need the fetch functionality, it is much easier not linking with libtiff (and zlib).
If anybody with a better empscripten knowledge has a better solution, please let me know.
A basic example is running in https://jjimenezshaw.github.io/wasm-proj/example.html
For the usage in the browser, an special configuration for Cross-origin isolation is needed. As that is not included in GitHub Pages, I am using https://github.com/gzuidhof/coi-serviceworker in the example. Probably you need it too.
docs/source/*.rstfor new API