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

Update meshopt_decoder module to latest #24491

Merged
merged 1 commit into from
Aug 16, 2022
Merged

Update meshopt_decoder module to latest #24491

merged 1 commit into from
Aug 16, 2022

Conversation

zeux
Copy link
Contributor

@zeux zeux commented Aug 12, 2022

This brings up meshopt_decoder modules to the latest versions, which
mostly constitutes a few internal Wasm changes that get reflected in the
different Wasm blobs, and support for Web Workers.

Note that with this change, GLTFLoader will start using decodeGltfBufferAsync
path (see #24460) but by default that still uses the same flow (so no change!).
However, this allows the application to call

MeshoptDecoder.useWorkers(4);

... to activate asynchronous geometry decoding. It's expected that the
better default is to not use WebWorkers, but using them can be
beneficial for very large files or very latency-sensitive applications.

Some size/perf stats:

old decoder (0.14 from before this change):

  • .js 21414 bytes
  • .js.gz 5745 bytes

new decoder:

  • .js 24850 bytes (+16%)
  • .js.gz 6442 bytes (+12%, or +700 bytes downloaded)
  • 3-4% faster decoding in Chrome
  • support for Web Workers (which is where most of the size impact comes from!)

new decoder with worker support stripped (not part of this PR!):

  • .js 22715 bytes (+6%)
  • .js.gz 5831 bytes (+1%)
  • 3-4% faster decoding in Chrome

I'm currently expecting that the extra ~700 bytes of download cost is
Not A Big Deal, and that size-conscious applications use webpack which
may or may not be able to strip Web Worker support out if that is not
used (that said I haven't tried).

This brings up meshopt_decoder modules to the latest versions, which
mostly constitutes a few internal Wasm changes that get reflected in the
different Wasm blobs, and support for Web Workers.

Note that with this change, GLTFLoader will start using decodeGltfBufferAsync
path but by default that still uses the same flow (so no change!).
However, this allows the application to call

	MeshoptDecoder.useWorkers(4);

... to activate asynchronous geometry decoding. It's expected that the
better default is to not use WebWorkers, but using them can be
beneficial for very large files or very latency-sensitive applications.

Some size/perf stats:

old decoder (0.14 from before this change):
- .js 21414 bytes
- .js.gz 5745 bytes

new decoder:
- .js 24850 bytes (+16%)
- .js.gz 6442 bytes (+12%)
- 3-4% faster decoding in Chrome
- support for Web Workers (which is where most of the size impact comes from!)

new decoder with worker support stripped (not part of this PR!):
- .js 22715 bytes (+6%)
- .js.gz 5831 bytes (+1%)
- 3-4% faster decoding in Chrome

I'm currently expecting that the extra ~700 bytes of download cost is
Not A Big Deal, and that size-conscious applications use webpack which
may or may not be able to strip Web Worker support out if that is not
used...
@donmccurdy
Copy link
Collaborator

I'm currently expecting that the extra ~700 bytes of download cost is
Not A Big Deal, and that size-conscious applications use webpack which
may or may not be able to strip Web Worker support out...

In general we've found it harder than expected to get dead code eliminated by build tools reliably, and I suspect the detection conditions here are not obvious enough for most build tools.

If (in the future) you are looking for a more reliable way to include optional functionality that won't increase bundle size for clients who don't use it, without publishing multiple npm packages, the use of package.json#exports might be the way to go. For example:

import { MeshoptDecoder } from 'meshoptimizer/worker';

But I definitely agree that 700 bytes for web worker support is reasonable, and personally wouldn't bother with additional modules or workarounds in this case.

@Mugen87 Mugen87 added this to the r144 milestone Aug 15, 2022
@mrdoob
Copy link
Owner

mrdoob commented Aug 16, 2022

Thanks!

@Mugen87 Mugen87 merged commit 3e7149a into mrdoob:dev Aug 16, 2022
abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
This brings up meshopt_decoder modules to the latest versions, which
mostly constitutes a few internal Wasm changes that get reflected in the
different Wasm blobs, and support for Web Workers.

Note that with this change, GLTFLoader will start using decodeGltfBufferAsync
path but by default that still uses the same flow (so no change!).
However, this allows the application to call

	MeshoptDecoder.useWorkers(4);

... to activate asynchronous geometry decoding. It's expected that the
better default is to not use WebWorkers, but using them can be
beneficial for very large files or very latency-sensitive applications.

Some size/perf stats:

old decoder (0.14 from before this change):
- .js 21414 bytes
- .js.gz 5745 bytes

new decoder:
- .js 24850 bytes (+16%)
- .js.gz 6442 bytes (+12%)
- 3-4% faster decoding in Chrome
- support for Web Workers (which is where most of the size impact comes from!)

new decoder with worker support stripped (not part of this PR!):
- .js 22715 bytes (+6%)
- .js.gz 5831 bytes (+1%)
- 3-4% faster decoding in Chrome

I'm currently expecting that the extra ~700 bytes of download cost is
Not A Big Deal, and that size-conscious applications use webpack which
may or may not be able to strip Web Worker support out if that is not
used...
snagy pushed a commit to snagy/three.js-1 that referenced this pull request Sep 21, 2022
This brings up meshopt_decoder modules to the latest versions, which
mostly constitutes a few internal Wasm changes that get reflected in the
different Wasm blobs, and support for Web Workers.

Note that with this change, GLTFLoader will start using decodeGltfBufferAsync
path but by default that still uses the same flow (so no change!).
However, this allows the application to call

	MeshoptDecoder.useWorkers(4);

... to activate asynchronous geometry decoding. It's expected that the
better default is to not use WebWorkers, but using them can be
beneficial for very large files or very latency-sensitive applications.

Some size/perf stats:

old decoder (0.14 from before this change):
- .js 21414 bytes
- .js.gz 5745 bytes

new decoder:
- .js 24850 bytes (+16%)
- .js.gz 6442 bytes (+12%)
- 3-4% faster decoding in Chrome
- support for Web Workers (which is where most of the size impact comes from!)

new decoder with worker support stripped (not part of this PR!):
- .js 22715 bytes (+6%)
- .js.gz 5831 bytes (+1%)
- 3-4% faster decoding in Chrome

I'm currently expecting that the extra ~700 bytes of download cost is
Not A Big Deal, and that size-conscious applications use webpack which
may or may not be able to strip Web Worker support out if that is not
used...
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.

None yet

4 participants