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

supports meshopt and basisu decoders #288

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

sdumetz
Copy link
Contributor

@sdumetz sdumetz commented Jul 3, 2024

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
Copy link
Collaborator

gjcope commented Jul 3, 2024

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
Copy link
Contributor Author

sdumetz commented Jul 3, 2024

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
Copy link
Collaborator

gjcope commented Jul 3, 2024

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
Copy link
Collaborator

gjcope commented Jul 19, 2024

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

@sdumetz
Copy link
Contributor Author

sdumetz commented Jul 22, 2024

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
Copy link
Collaborator

gjcope commented Jul 22, 2024

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
Copy link
Collaborator

gjcope commented Jul 22, 2024

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
Copy link
Contributor Author

sdumetz commented Jul 26, 2024

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 gjcope merged commit bce117c into Smithsonian:master Jul 26, 2024
@gjcope
Copy link
Collaborator

gjcope commented Jul 26, 2024

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

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.

2 participants