iTwin
Imdl tiles meshopt compression
Imodel-native-internal: https://github.com/iTwin/imodel-native-internal/pull/576
![azure-pipelines[bot] avatar](https://avatars.githubusercontent.com/in/9426?v=4 "Published by a pub.dev verified publisher"){kind=small}
@pmconne about consuming meshoptimizer module in parse IMDL workers... MeshoptCompression.decodeMeshoptBuffer is asynchronous, but there is currently no async logic in place in the parser, I could make all parent functions async until I reach the code, but I am not quite sure if that's the right call. I also considered not loading the module dynamically (to avoid await), but I guess that would cancel your effort to avoid loading it only if not needed... any suggestions?
@pmconne about consuming meshoptimizer module in parse IMDL workers... MeshoptCompression.decodeMeshoptBuffer is asynchronous, but there is currently no async logic in place in the parser, I could make all parent functions async until I reach the code, but I am not quite sure if that's the right call. I also considered not loading the module dynamically (to avoid await), but I guess that would cancel your effort to avoid loading it only if not needed... any suggestions?
It's dynamically loaded because we currently only use it for glTF that contains meshopt-encoded data (very rare). If we're going to start meshopt-encoding every iMdl tile, there's no reason to dynamically load it because we'll always need it.
It currently does the decoding on a worker, which is of course async. That is useful for glTF / reality models, whose decoders run on the main thread. You could make the iMdl parser use the synchronous meshopt decoder instead.
@pmconne about consuming meshoptimizer module in parse IMDL workers... MeshoptCompression.decodeMeshoptBuffer is asynchronous, but there is currently no async logic in place in the parser, I could make all parent functions async until I reach the code, but I am not quite sure if that's the right call. I also considered not loading the module dynamically (to avoid await), but I guess that would cancel your effort to avoid loading it only if not needed... any suggestions?
It's dynamically loaded because we currently only use it for glTF that contains meshopt-encoded data (very rare). If we're going to start meshopt-encoding every iMdl tile, there's no reason to dynamically load it because we'll always need it.
It currently does the decoding on a worker, which is of course async. That is useful for glTF / reality models, whose decoders run on the main thread. You could make the iMdl parser use the synchronous meshopt decoder instead.
If we want to execute the meshopt decoder in IMDL parser workers, we will have to inject the WASM decode code into the worker as follows: link, which is tricky since the WASM code is hidden in a module function scope, so I can't access it even by casting the decoder to any. The other alternative is to spawn a meshopt decoder worker from the parser worker and call it asynchronously, which still requires changing all parsing functions to async ... what do you think ?
If we want to execute the meshopt decoder in IMDL parser workers, we will have to inject the WASM decode code into the worker
You can't call the synchronous functions like decodeGltfBuffer from inside a worker? Why not?
If we want to execute the meshopt decoder in IMDL parser workers, we will have to inject the WASM decode code into the worker
You can't call the synchronous functions like
decodeGltfBufferfrom inside a worker? Why not?
Nevermind, I was creating the decoder outside the worker and then passing it as argument, which explains why the WASM was not initialized in the worker scope, it works fine now :)
[WIP do not review]
The native PR has merged, is this now awaiting review?
Sorry wrong link, I updated the description
@pmconne I changed this PR a bit since you last approved it, so I reset your approve so you get a chance to have a second look before merging.
@pmconne what about copying the meshopt module code in itwinjs (mostly sandalone wasm bytecode under MIT license), and change the code to load wasm on demand rather than at module load, this would avoid us having to investigate vitests, since only two tests will load compressed tiles at the moment and thus it will not be enough to cause the wasm memory leak error ?
The decoder file is standalone, and this is all the code we need to make it work in itwinjs: https://github.com/zeux/meshoptimizer/blob/master/js/meshopt_decoder.js
what about copying the meshopt module code in itwinjs
Don't love copying code from dependencies but no reason not to perform the experiment and see if it would resolve the error.