search

Smithsonian / dpo-voyager

DPO Voyager - 3D Explorer and Tool Suite
Apache License 2.0
168 stars 30 forks source link

supports meshopt and basisu decoders #288

Closed sdumetz closed 4 months ago

sdumetz commented 4 months ago

This PR adds meshopt and basisu decoders support.

Meshopt is fully bundled from the THREE.js examples

Basisu is provided by the KTX2Loaderwith an additional assembly file, added to the asset folder.

Also added a shortcut to set draco and basisu decoders paths when setting a system asset path. I couldn't find a good CDN for the basisu assembly file so the support is a bit different from draco's default.

It adds some glue code to provide access to the renderer from ModelReader, but this is required to use WebGLRenderer.initTexture and WebGLRenderer.compileAsync, both of which are very important to reduce "blocking time" when loading models.

This is part of a larger effort to provide real time dynamic LOD management that might take a few months to make and for which a POC is available here (very much work-in-progress): https://github.com/Holusion/dpo-voyager/tree/seamless_load_proto

gjcope commented 4 months ago

This is great, thanks! Excited to see the LOD support come together. That will be a nice feature. We've been planning KTX support for awhile (https://github.com/Smithsonian/dpo-voyager/tree/dev-ktx) but hasn't been prioritized due to not being a part of our content pipeline yet.

sdumetz commented 4 months ago

From my tests, it is a pain to encode (I use gltf-transform), the lack of support in blender (not even in progress) makes editing models a lot harder (especially since those are lossy encodings so we can't easily go back and forth).

However on the Voyager Side, support has been flawless and we have seen consistent loading time improvements with both methods and greatly improved memory usage and with GPU upload times with ktx2/basisu textures.

Any reason why this didn't get deployed?

gjcope commented 4 months ago

The reason was just because we hadn't included the support in our content pipeline for the reasons you mentioned and a couple others. But agree, in our proof-of-concept testing we saw a dramatic difference with memory usage. No real reason not to move forward now though.

gjcope commented 4 months ago

model-viewer is using this gstatic endpoint for the basisu files: https://www.gstatic.com/basis-universal/versioned/2021-04-15-ba1c3e4

sdumetz commented 4 months ago

I'd rather not depend on google at all for keeping anything not search-related working long-term. Using the repo's local assets dir is fine for me but if you do really need to have a cloud-based fallback, I couldn't find anything that would be more reliable so this one will have to do.

gjcope commented 4 months ago

It's been reliable for >3 years and they have a product relying on it so it feels pretty safe. I also don't see a downside to having graceful fallbacks. It makes the deployment more lightweight and can also pretty easily be pulled out if something happens to it.

gjcope commented 4 months ago

Looked at this a little more and I think we should have more consistency between the handling of the draco and basis libs.

How do you feel about: https://github.com/Smithsonian/dpo-voyager/tree/dev-meshopt-ktx

This doesn't use gstatic endpoints for draco or basis, but points to the CDN we are already using to deliver assets. I don't particularly like the conflation of bundled assets and libs, but it seems cleaner.

I understand your hesitancy to default to a CDN, but one of our goals is for end users to be able to include the component in their code without a dependency on hosting it themselves which has been a limitation in the past.

It can also easily be configured for local use (which we discuss in the docs: https://smithsonian.github.io/dpo-voyager/explorer/offline/) by just setting the resourcesRoot to './'. We could potentially even add some code to check the CDN and default to the local root if unavailable...

sdumetz commented 4 months ago

To be clear I'm not opposed to default to a CDN, I'm just wary of Google breaking things carelessly. They have a track record. Especially since the route is versioned, it might (or might not) return a 404 if they decide to upgrade the libraries for their model-viewer. We probably wouldn't get warning before this happens.

In the end I don't plan on using the default often, we pretty much always use the resourceRoot so I don't want to weigh in too much on this.

With that said, I like your proposition on https://github.com/Smithsonian/dpo-voyager/tree/dev-meshopt-ktx : it's more consistent than the original patch, easy to update and to replace if needed.

gjcope commented 4 months ago

Understood. This has been merged via dev-meshopt-ktx and v0.43.0 release.