Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions examples/jsm/loaders/DRACOLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import {

const _taskCache = new WeakMap();

const WASM_BIN_URL = new URL( '../libs/draco/draco_decoder.wasm', import.meta.url ).toString();
const WASM_JS_URL = new URL( '../libs/draco/draco_wasm_wrapper.js', import.meta.url ).toString();
const JS_URL = new URL( '../libs/draco/draco_decoder.js', import.meta.url ).toString();

Comment on lines +16 to +19
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@donmccurdy - I'm realizing that there are two draco loader decoders in this project: libs/draco/* and libs/draco/gltf/*.

What's the difference between these two? Is the glTF version the same as the basic "draco" one but with some unnecessary code stripped out? Does it make sense to adjust all the examples to remove the "setDecoderPath"? It's a bit unfortunate that we have to have two decoders when the gltf path is likely the most commonly used one. I'm wondering what to do here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See:

If that's still correct, then the "draco" decoder is a superset of the "draco/gltf" decoder, and "draco/gltf" may lack support for decoding point clouds. The glTF KHR_draco_mesh_compression extension doesn't include point cloud compression.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it - so it's purely a file size issue and the non-glTF version seems like the right default.

I'm wondering what we should do to enable users to specify using the "gltf" variant, then? Looking some issue history it looks like #24946 or #31446 could be viable. Generally I don't think it's great that these classes require and examples demonstrate the use of functions like setDecoderPath that don't work in any bundlers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still correct – the gltf variant is smaller and supports only the subset of Draco that is allowed in glTF, whereas the other one supports all of the Draco spec (typically .drc files).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts on how we should cleanly enable using the gltf-draco-decoders in a bundler-friendly way? Looking around a bit more I'm actually not sure if bundlers support the same kind of URL* syntax with bare modules. Eg:

loader.setDecoderFiles(
  new URL( 'three/addons/libs/draco/decoder.js' ).toString(),
  new URL( 'three/addons/libs/draco/decoder.wasm' ).toString(),
);

So we're back to either copying the files using CDNs if users want to use the gltf files (which they should). A simple option could just be a flag that toggles the default URLs:

const dracoLoader = new DRACOLoader();
dracoLoader.useGLTFVariant = true;

// loader will now load the gltf versions rather than the full draco versions

Open to any other thoughts, though.

/**
* A loader for the Draco format.
*
Expand Down Expand Up @@ -358,14 +362,31 @@ class DRACOLoader extends Loader {
const useJS = typeof WebAssembly !== 'object' || this.decoderConfig.type === 'js';
const librariesPending = [];

if ( useJS ) {
if ( this.decoderPath === '' ) {

if ( useJS ) {

librariesPending.push( this._loadLibrary( 'draco_decoder.js', 'text' ) );
librariesPending.push( this._loadLibrary( JS_URL, 'text' ) );

} else {

librariesPending.push( this._loadLibrary( WASM_JS_URL, 'text' ) );
librariesPending.push( this._loadLibrary( WASM_BIN_URL, 'arraybuffer' ) );

}

} else {

librariesPending.push( this._loadLibrary( 'draco_wasm_wrapper.js', 'text' ) );
librariesPending.push( this._loadLibrary( 'draco_decoder.wasm', 'arraybuffer' ) );
if ( useJS ) {

librariesPending.push( this._loadLibrary( 'draco_decoder.js', 'text' ) );

} else {

librariesPending.push( this._loadLibrary( 'draco_wasm_wrapper.js', 'text' ) );
librariesPending.push( this._loadLibrary( 'draco_decoder.wasm', 'arraybuffer' ) );

}

}

Expand Down
2 changes: 0 additions & 2 deletions examples/webgl_tonemapping.html
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@
.setPath( 'textures/equirectangular/' );

const dracoLoader = new DRACOLoader();
dracoLoader.setDecoderPath( 'jsm/libs/draco/gltf/' );

const gltfLoader = new GLTFLoader();
gltfLoader.setDRACOLoader( dracoLoader );
gltfLoader.setPath( 'models/gltf/' );
Expand Down
Loading