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

Direct .basisu image support #106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daverin
Copy link
Contributor

@daverin daverin commented Oct 5, 2020

Support for direct (non-KTX container) .basis format.

Uses atteneder/KtxUnity (needs to be included in manifest.json since unity does not resolve git dependencies.

My team needed this for a proof-of-concept glTF pipeline in Unity. Without it, loading glTF/glb files with large textures creates enormous stalls on the main thread and causes excessive RAM usage due to png/jpg expansion.

glb/gltf files with web encoded images (png/jpg) can be converted to ones with embedded basis images using one of the following tools:

@daverin daverin changed the title Feature/basisu Direct .basisu image support Oct 5, 2020
@DerrickBarra
Copy link

DerrickBarra commented Nov 11, 2020

Thanks for your submission @daverin, our team is testing out your pull request with our own model and ran into an issue with KtxUnity failing to load the ktx2 textures from a glb. (returns a null texture reference)

Would you mind sharing a model your team can confirm works? I'm double-checking this with @atteneder from KtxUnity as well, but I saw your pull request and figured you might have a confirmed working .glb with ktx2 textures.

@daverin
Copy link
Contributor Author

daverin commented Nov 12, 2020

Hi @DerrickBarra

I couldn't get KTX2 to work. I received a reply from @atteneder on an issue from KtxUnity confirming that it's resolved but I haven't had a chance to test it yet.

For now, my team is using .basis textures directly and we're happy.

here is a sample project. Checkout SampleSceneGltf. The sample assets have their basis images compressed with different flags. There is also a sample with basis and draco.

@DerrickBarra
Copy link

DerrickBarra commented Nov 12, 2020

@daverin Thanks for the links! I just tested loading .glb files with .basis textures from your repo (hosted on our AWS), with the same result (returned texture is null).

Avocado - GLB & Basis
Gladiator - GLB & Basis

We're using KtxUnity 0.9.1 (latest release). Interesting that the older version you are using is working (the newest version has a TextureOrientation value returned as well as the final texture). I suppose it's possible that these files were made with an outdated version of the basis specification, but there's also a chance that something new in KtxUnity broke support.

If it's not too much of a bother, do you think you could see if the latest KtxUnity works with the files on your machine? Debugging what plugin or file has issues is pretty hard, so to hear that someone else has it working is exciting. If it breaks for you against the newest version of KtxUnity then I'll let @atteneder know.

@DerrickBarra
Copy link

DerrickBarra commented Nov 12, 2020

@daverin So I found out why KtxUnity 0.9.1 doesn't work with the code from this branch. For some reason the use of TextureBase.LoadBytesRoutine() causes the resulting texture to be null.

If I change the code to use TextureBase.LoadFromBytes(), the textures load as expected (.ktx2 and .basis work).

Basically I made a script called CoroutineHolder in the GLTFUtility plugin folder, and I spawn that or look it up so I can pass it in before I call TextureBase.LoadFromBytes()

Also btw the code in this branch could be updated to support .ktx2, since that also works in the latest version of KtxUnity.

@daverin
Copy link
Contributor Author

daverin commented Nov 12, 2020

Hi @DerrickBarra

I can relate. Trying to pinpoint where these errors are located is pretty hard. I've encountered this issue before. On our GitLab fork of the repo, I managed to solve it.

The error you're seeing is the result of this enumerator not being run. It seems co-routines can't be directly yielded in the ImportTask if it is run asynchronously.

I created two potential fixes for it. I'm not knowledgable enough in co-routines to understand which one is better:

I also updated the sample project and my fork Beamm-Incorporated/KtxUnity to use the latest release of KtxUnity.

@DerrickBarra
Copy link

@daverin Thanks! I let @atteneder know about this bug. Glad to see there are some possible fixes.

@atteneder
Copy link

Hi folks,

First of all thanks for putting KtxUnity to good use!

@daverin both your solutions seem workaroundish.

Make sure you understand how to synchronously call nested coroutines. I recommend this article.

That's why I always keep a monobehaviour reference around to call StartCoroutine on.

The long-term solution for KtxUnity is to replace Coroutines by async methods (since that should be DOTS compatible), but there's no timeline for it yet.

@daverin
Copy link
Contributor Author

daverin commented Nov 13, 2020

Thanks @atteneder 🙌🏾

@DerrickBarra
Copy link

@daverin
Check out our forked version of GLTFUtility.

It has a dependency on KtxUnity and StaticCoroutine (along with the 'KTX' scripting define symbol). But I confirmed that it works with .KTX2 and .Basis texture loading withing unzipped .gltf archives and .glb files. It also properly flips the textures via the material UV settings.

The use of StaticCoroutine could be removed if we required that the user passes in a monobehaviour when they call the initial loading logic. Our team uses StaticCoroutine for other stuff, but that wouldn't be a hard change to make if you care to.

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.

3 participants