Enable GLTF export with baked-in textures #5
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
| @@ -31,6 +31,7 @@ | |||
| "@types/node": "^22.5.5", | |||
| "@types/react": "^18.3.3", | |||
| "@types/react-dom": "^18.3.0", | |||
| "@types/three": "0.168.0", | |||
There was a problem hiding this comment.
Tried to add it in order to make onDone and onError types below work, but without success.
| onDone: (gltf) => { | ||
| const blob = new Blob([JSON.stringify(gltf)], { | ||
| type: "application/json" | ||
| }); | ||
| const url = URL.createObjectURL(blob); | ||
|
|
||
| const link = document.createElement("a"); | ||
| link.href = url; | ||
| link.download = "scene.gltf"; | ||
| link.click(); | ||
|
|
||
| URL.revokeObjectURL(url); | ||
| }, | ||
| onError: (error) => { |
There was a problem hiding this comment.
Here, the params of onDone and onError are inferred as any. Seems that it's because GLTFExporter type is inferred as any in the first place.
| object.material = new THREE.MeshBasicMaterial({ | ||
| ...object.material, | ||
| map: texture |
There was a problem hiding this comment.
Here I think I'm applying the texture wrong.
I'm a Three.js noob tbh, any clue is welcome :)
|
Hey! Thanks for the PR. After thinking over on this, it seems that the scope of this PR is beyond the scope of the core library. As such, it would be better suited to be an example usage of the library rather than a core feature. Another point for this is that So, I think it would be best to persue this:
If going with 1, please close this PR. I may persue option 1 in my own time too, if so, i will let you know |
No worries! Throwing out the idea but what would you think about creating a I'm fine with doing option 1, but it seems to me that it would be such a thin layer on top of this lib that it could be kind of overkill. Side note: any idea about what I'm doing wrong here? #5 (comment) |
|
Yeah sorry for the initial confusion. I think we can do a mix of approach 1 and 2 where you can replace the current example with this use case. I’d be happy to accept a PR for that for the comment, yep that seems about right, keep in mind, in my email I outlined an approach that I haven’t tried yet so this may or may not work. We’ll need to find out |
|
The reason I’m hesitant to add it to the core of this library is that it’s an inherently hacky approach and would introduce some maintenance overhead if it doesn’t work for some folk or breaks when ThreeJS updates something. Since GLTFExporter is not a part of code ThreeJS, breaking changes are more likely within it if left in userland, we wouldn’t have to scurry for a solution of something like that happens |
No worries again!
Maybe just extend it? I think it's nice to have the current example to showcase a simple use case. Wdyt?
I can tell you for sure: it does not 😂. I ran it before submitting this draft.
I'm going to investigate.
Thanks for the explanations, got it! |
|
Extending it sounds good to me! |
|
Great! On it! |
|
Can't manage do get around this. The Posting this here in case you would have any clue. |
The idea is to leverage GLTFExporter