Skip to content

Conversation

@hybridherbst
Copy link
Contributor

@hybridherbst hybridherbst commented Jan 24, 2022

Description

Currently, GLTFExporter can only export RGBATextures and basic meshes. For example, loading a GLTF mesh that has KTX2 textures and/or Draco compression will prevent exporting the scene. A similar issue happens when auto-converting to USDZ for AR viewing, where the compressed textures are also breaking conversion.

This draft PR attempts to mitigate that by blitting any textures that are not RGBA into new textures that can then be exported.

It's very much draft because I'd like to learn: both

  • where to do this blitting (so that it could be reused for USDZ conversion)
  • how to do it correctly, creating a full renderer on the fly feels very wrong (even with caching that could be added)

I understand that there's efforts for a new texture type to potentially keep the CPU data for the loaded texture in memory so it might be possible to just "pass it through" for export, however I think that's not always desired (e.g. I might want to export a file without KTX2 and Draco compression even if my scene contains some models that do). Also, I don't think keeping the data around on the CPU just because someone might export it later makes sense; with blitting any type of 2D texture, no matter the source, could be exported.

Current Status

  1. I added a etc1s compressed version of coffeemat.glb, coffeemat.etc1s.glb, that does not use meshopt compression, to only deal with textures for now.

  2. I added this coffemat to the misc_exporter_gltf sample scene, together with a button to export it:
    image

  3. Without this PR, clicking that button will simply abort exporting because the CompressedTexture can't be read. With this PR, the export works without warning, and a glb file is produced that contains textures.

  4. The textures look correct when inspected with https://gltf.report, with the exception of the baseColor texture, that seems to be in a wrong color space:
    image

  5. Exporting the fully compressed coffeemat (meshopt + etc1s) results in an invalid model with gltf.report outputting numerous errors about wrong attribute formats (displaying a black model with correct vertices but broken UVs), so I think this is out of scope for this PR.

  6. I attempted adding mesh export support for the meshopt coffeemat by adding Int8 export support, which leads to a black mesh instead of nothing, but fails at properly exporting various attributes; my understanding is that some of them would need to be converted back to non-meshopt formats (e.g. byte normalized back to float for vertex positions)

TODO / Feedback needed

  1. Creating the full renderer feels wrong, is there a better way to blit any texture into a new, readable one?

  2. The place feels wrong, making a texture readable by blitting it would also be beneficial for other exporters

  3. The baseColor texture ends up having the wrong color space - it does actually already look wrong on the HTML page, so not sure if there's another issue hidden here

  4. The Int8/Byte support needs to be stripped out again and potentially moved into a different PR

Feedback very welcome 🙏

CC @elalish as I think you might be interested in better USDZ autoconversion as well

Related Issues
#20474

@donmccurdy
Copy link
Collaborator

@hybridherbst
Copy link
Contributor Author

hybridherbst commented Feb 2, 2022

Also related

And would make progress on / fix in the sense of supporting import for

@donmccurdy
Copy link
Collaborator

@hybridherbst is this PR something you're still interested in? I'm thinking that a utility in examples/jsm/TextureUtils might be the way to go. Ideally it'd take a WebGLRenderer as an optional argument so we aren't making so many GL contexts, take a THREE.CompressedTexture, and return a THREE.Texture. Then we could decide later whether to integrate it with GLTFExporter or ask users to preprocess.

@hybridherbst
Copy link
Contributor Author

Definitely, yes! I also recently saw that there's a similar implementation in USDZExporter, so maybe all of those can be pulled in one place.

@hybridherbst hybridherbst force-pushed the gltf-export-compressed-data branch 2 times, most recently from 9e885a5 to 45322ce Compare September 6, 2022 20:13
@hybridherbst
Copy link
Contributor Author

hybridherbst commented Sep 6, 2022

@donmccurdy I rebased on latest dev and got a couple questions so this PR can be completed:

  1. Do you mean creating a new jsm/utils/TextureUtils.js?

  2. Do you have an idea regarding the colorspace issues I mentioned in my original post?
    I saw buildMetalRoughTexture converts from sRGB to Linear but seems the color space issue happens with map.

  3. Regarding this working by default, what do you think about creating a context only once per export and releasing it at the end? – this way it would be nicely encapsulated, nobody can mess up by passing a weird context in, and it doesn't create too many new ones (max. 1 per GLTF or USDZ export). - it could still be optional to pass in a context?
    EDIT: added in that the temp context is reused for the entire export and disposed at the end.

  4. What I also tried for this PR is exporting from draco- and meshopt-compressed meshes - draco is working, meshopt isn't working yet.
    Would be great if you could take a look –
    testing is as easy as opening misc_exporter_gltf.html and changing

    gltfLoader.load( 'coffeemat.etc1s+draco.glb', function ( gltf ) {
    to
    gltfLoader.load( 'coffeemat.etc1s+meshopt.glb', function ( gltf ) {

    and pressing "Export (compressed)" in the UI.
    My suspicion is that the SHORT format isn't implemented correctly yet on my end, there's warnings around that in the console.

  5. Note for future me or someone else looking at this PR, seems buildMetalRoughTexture would also need to use the new path; the order is right now wrong (first metal/roughness is combined, which will fail for compressed data, then the texture is read back if compressed).

@donmccurdy
Copy link
Collaborator

donmccurdy commented Oct 2, 2022

  1. Do you mean creating a new jsm/utils/TextureUtils.js?

Yes, I think so.

  1. Do you have an idea regarding the colorspace issues I mentioned in my original post?

If map.encoding = THREE.sRGBEncoding, then the output of the fragment shader to the readable texture must be encoded (Linear-sRGB to sRGB). I would imagine there's some way to access the GLSL function three.js includes for this...

  1. Regarding this working by default, what do you think about creating a context only once per export and releasing it at the end?

I think a helper located in jsm/utils/TextureUtils.js would operate only on a single texture, without any concept of an export session. Something like:

const texture = TextureUtils.decompress( compressedTexture, renderer );

If it's the user's responsibility to call this before exporting a scene with compressed textures, they'd choose what renderer(s) to provide. If we enable it by default in GLTFExporter (preference: not right now) then we'd limit how many are created.

  1. What I also tried for this PR is exporting from draco- and meshopt-compressed meshes - draco is working, meshopt isn't working yet.

Let's start a new thread for this somewhere? We've never implemented KHR_mesh_quantization in GLTFExporter, which would be required here.

@elalish
Copy link
Contributor

elalish commented Nov 9, 2022

I'm interested in this too! The <model-viewer> editor is exposing this problem: google/model-viewer#3911

@hybridherbst hybridherbst force-pushed the gltf-export-compressed-data branch from 81891e9 to dc59c08 Compare November 24, 2022 22:52
@hybridherbst
Copy link
Contributor Author

hybridherbst commented Nov 24, 2022

Finally got around to continue here;

  • I added a TextureUtils class
  • that has a decompress method
  • creates its own temporary renderer if none is provided
  • respects sRGB vs. Linear encoding

Little Coffeemat looks pretty good now!

Original Compressed Version (etc1s+draco) | Re-Exported Version
Coffeemat-Comparison.zip

image

@hybridherbst hybridherbst marked this pull request as ready for review November 24, 2022 23:04
@hybridherbst hybridherbst changed the title [Draft] GLTF Exporter: export from compressed texture data GLTFExporter: export from compressed texture data Nov 24, 2022
@hybridherbst hybridherbst force-pushed the gltf-export-compressed-data branch from a569d79 to 74223fa Compare March 27, 2023 17:06
@hybridherbst hybridherbst force-pushed the gltf-export-compressed-data branch from 614fe8b to a96ca87 Compare March 27, 2023 18:40
@hybridherbst
Copy link
Contributor Author

I rebased this on latest dev now and simplified the code, some things that this PR did have already been added in the meantime.

@donmccurdy I also accomodated most of your feedback above. I did not find a good alternative for the cache.images.delete line though, plus I think that caching has some broken edgecases - the same image with different flipY settings would result in the same cache hit (and one of them wrong data), but I think that's outside the scope of this PR.

I also tested that it still works for the compressed coffeemat (which is still included in this PR as part of the sample change, but could be removed before merging).

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

Thanks @hybridherbst!

plus I think that caching has some broken edgecases - the same image with different flipY settings would result in the same cache hit (and one of them wrong data), but I think that's outside the scope of this PR.

Agreed, we can think about this later.

added coffemat.etc1s.glb and etc1s+draco, etc1s+meshopt to test compressed formats

only use one temp render context, clean up renderer after writing file, make metalnessMap and roughnessMap readable

refactor: move to TextureUtils class and use that in GLTFExporter, respect sRGB vs. Linear, cache some generated objects

cleanup

simplify modifiedMap access

fix formatting for TextureUtils

dispose of temporary renderer

remove duplicate switch entries
@hybridherbst hybridherbst force-pushed the gltf-export-compressed-data branch from 36b1639 to 3f1307e Compare May 23, 2023 21:33
Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

A couple suggestions but nothing blocking, thanks @hybridherbst!

window.addEventListener( 'resize', onWindowResize );

// ---------------------------------------------------------------------
// Exporting compressed textures and meshes (KTX2 / Draco) (TODO: Meshopt)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Draco compression won't leave any trace in the resulting three.js objects, and Meshopt requires only KHR_mesh_quantization (which is tested for export above). So it might just be KTX2 that is a concern here, perhaps we could use the existing coffeemat.glb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe when I originally started the PR Draco compression did indeed not work for roundtrips since it could still end up as int8 or so, which wasn't supported in the exporter. May not be the case anymore!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried using the existing one, but that actually doesn't export cleanly right now, with an error I don't understand - the readback canvas seems to have the right size but comes out as 0x0:

20230524-023523_chrome

I debugged a bit and am not really sure what's going on there...

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

A couple suggestions but nothing blocking, thanks @hybridherbst!

@hybridherbst
Copy link
Contributor Author

Thanks! I'm honestly not really sure why the existing coffeemat.glb doesn't export right now — would it be possible you take a look at that?

@donmccurdy
Copy link
Collaborator

@hybridherbst not sure why it's only hitting the issue for one version of the model, but this patch should fix it: 4e87041

@hybridherbst
Copy link
Contributor Author

Excellent, that works – I removed the additional model again and now the existing coffeemat.glb is used.

@mrdoob mrdoob added this to the r153 milestone May 26, 2023
@mrdoob mrdoob merged commit 412e471 into mrdoob:dev May 26, 2023
@Methuselah96 Methuselah96 mentioned this pull request Jun 4, 2023
45 tasks
@hybridherbst hybridherbst deleted the gltf-export-compressed-data branch July 6, 2023 12:21
jeffbeene added a commit to jeffbeene/three.js that referenced this pull request Dec 14, 2023
@jeffbeene
Copy link
Contributor

@hybridherbst I implemented your decompress function in USDZExporter - I'm now able to view coffeemat.glb in AR with QuickLook. Thank you for your work on this, hopefully all is good with my PR and this will make it into r160!

#27382

Mugen87 added a commit that referenced this pull request Dec 15, 2023
* USDZExporter: export from compressed texture data #23321

* add screenshot, update files.json

* Update USDZExporter.js

Clean up.

* remove usdz compressed example

* Update USDZExporter.js

---------

Co-authored-by: Michael Herzog <[email protected]>
AdaRoseCannon pushed a commit to AdaRoseCannon/three.js that referenced this pull request Jan 15, 2024
…b#27382)

* USDZExporter: export from compressed texture data mrdoob#23321

* add screenshot, update files.json

* Update USDZExporter.js

Clean up.

* remove usdz compressed example

* Update USDZExporter.js

---------

Co-authored-by: Michael Herzog <[email protected]>
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.

5 participants