Three.js: GLTFLoader: Some glTF models cannot be cloned

Created on 21 Jun 2017  ·  20Comments  ·  Source: mrdoob/three.js

Description of the problem

GLTF2 models that are clones do not displayed correctly. This is for a number of reasons

  • the onBeforeRender callback is not copied
  • GLTFShader is not cloned across
  • SkinnedModel skeleton is reinitialised to 0 bones and the source skinnedmodel skeleton is not cloned

http://jsfiddle.net/mn768h35/

Three.js version
  • [X ] Dev
  • [X] r85
  • [ ] ...
Browser
  • [x] All of them
  • [ ] Chrome
  • [ ] Firefox
  • [ ] Internet Explorer
OS
  • [x] All of them
  • [ ] Windows
  • [ ] macOS
  • [ ] Linux
  • [ ] Android
  • [ ] iOS
Hardware Requirements (graphics card, VR Device, ...)
Bug Loaders

Most helpful comment

@cdata @mrdoob nice, thanks! 👍

Quick status update on this issue, then. The original problem (custom shaders in the model) no longer exists in glTF2. But I'm aware of three cases in which cloning a glTF model might still fail:

  1. [ ] Skinned mesh (workaround possible via @cdata's script)
  2. [ ] Spec/gloss materials (relies on ShaderMaterial; can't clone the refresh callback. workaround possible by converting to metal/rough)
  3. [ ] Animations referencing unnamed nodes (we fall back on UUID, workaround possible by adding names to nodes).

NOTE: I don't think the problem cloning skinned meshes is unique to glTF... it is just because the bones are often not children of the mesh in glTF examples? So that could also be worked around by ensuring the bones are children of the mesh in the glTF asset, I believe, but certainly it would be nice for this to "just work".

All 20 comments

SkinnedModel skeleton is reinitialised to 0 bones and the source skinnedmodel skeleton is not cloned

Yeah, this is well known issue.

Yeah, this is well known issue.

Is a solution being tracked ?
Is there a proposed solution to avoid having to re-parse the model from the downloaded file

While reparsing is reasonable for one off model download this would be problematic for many downloaded instances both in terms of memory and time.

The problem is currently discussed here #11574. We are trying to find a solution for this.

Possible duplicate of https://github.com/mrdoob/three.js/issues/8869 (but thank you for the extra details and JSFiddle!)

Regarding onBeforeRender and GLTFShader, ~this only applies~ these apply to the KHR_materials_pbrSpecularGlossiness and KHR_technique_webgl extensions respectively. The spec for the latter is not final yet, so no one should be using that.

Models using the default PBR metal/rough and KHR_materials_common (blinn/phong) do not use GLTFShader or onBeforeRender, and can be cloned just fine. @mikearmstrong001 are you able to change your models from a spec/gloss to a metal/rough PBR workflow?

EDIT: or have i misunderstood? I don't see any cloning, or any GLTFShader objects, in the JSFiddle.

EDIT: or have i misunderstood? I don't see any cloning, or any GLTFShader objects, in the JSFiddle.

? look for:

scene.add(gltf.scene.clone());

debugging led to knowing about the GLTFShader. The duck model does work

Possible duplicate of #8869 (but thank you for the extra details and JSFiddle!)

I think so but I am unclear if that is related to the GLTFLoader or GLTF2Loader

Regarding GLTFShader and onBeforeRender, this only applies to the KHR_materials_pbrSpecularGlossiness and KHR_technique_webgl extensions. The spec for the latter is not final yet, so no one should be using that.

GLTFShaders are definitely created and the current design will not facilitate cloning as the details of the implementation are hidden. I did some workarounds to have my own version and may continue but I'd rather not introduce fragile code into the framework.

are you able to change your models from a spec/gloss to a metal/rough PBR workflow?

As this is for a framework built on three.js we can't stipulate the models used.

@mrdoob + others, any thoughts on making cloning a requirement as part of adding new model formats?

Ok, if it helps the example you've included does not have any GLTFShaders, so once the animation issue is fixed you should be able to clone models fine.

gltf.scene.traverse((node) => {
  if (node.isMesh) console.log(node.material.type);
});
VM671:2 MeshStandardMaterial

As this is for a framework built on three.js we can't stipulate the models used.

Do you have a model showing the GLTFShader issue? There should not be any such models, because the KHR_technique_webgl extension has not been ratified for glTF2, and no exporters create it. GLTFShader is effectively dead code in the meantime.

Bear in mind that both spec-gloss and GLSL are optional extensions to the main glTF2 spec, and it's unlikely that three.js will support every possible extension. The only material type in the core specification is PBR metal-rough, or MeshStandardMaterial, and we support cloning that just fine. We also support cloning for common materials (MeshPhongMaterial) extension, although that spec is also not yet finalized.

I'll look into supporting cloning for KHR_materials_pbrSpecularGlossiness, but this is difficult without making changes to the core Mesh class. We'd need an onAfterClone callback or something, because the mesh can't reproduce its own onBeforeRender callback.

EDIT: I guess we could get away with child.onBeforeRender = this.onBeforeRender, if that seems safe? For spec-gloss.

I was testing various meshes with CesiumMan so I think it probably just one of the pbr based examples eg

http://jsfiddle.net/b5r4cca4/

I guess we could get away with child.onBeforeRender = this.onBeforeRender, if that seems safe? For spec-gloss.

I'm not convinced this would work as the uniforms would reference the original hierarchy.

The way I looked into handling it was as a post cloning fixup that worked over the hierarchy mapping source nodes to cloned nodes based. Something similar could be done by specialise the clone on the base scene, I agree the current clone setup is too restrictive because it is difficult to operate on global aspects of the scene in one pass and perhaps by relaxing this it will simplify the skeleton cloning as well.

Addressing comments from @zellski and @jbaicoianu in #11574 —

It sounds like, in addition to the ongoing work on shared Skeleton, GLTF2Loader should avoid referencing UUIDs for skinning. It will be necessary to add names to unnamed bones in the loader, but that's a reasonable trade to support cloning and serialization.

@cdata just hacked this for a project he's working on:
https://gist.github.com/cdata/f2d7a6ccdec071839bc1954c32595e87

@cdata @mrdoob nice, thanks! 👍

Quick status update on this issue, then. The original problem (custom shaders in the model) no longer exists in glTF2. But I'm aware of three cases in which cloning a glTF model might still fail:

  1. [ ] Skinned mesh (workaround possible via @cdata's script)
  2. [ ] Spec/gloss materials (relies on ShaderMaterial; can't clone the refresh callback. workaround possible by converting to metal/rough)
  3. [ ] Animations referencing unnamed nodes (we fall back on UUID, workaround possible by adding names to nodes).

NOTE: I don't think the problem cloning skinned meshes is unique to glTF... it is just because the bones are often not children of the mesh in glTF examples? So that could also be worked around by ensuring the bones are children of the mesh in the glTF asset, I believe, but certainly it would be nice for this to "just work".

For what it's worth, this is how we solved the skinned mesh problem -- the idea being that the user shouldn't need to know a loaded model needs special attention: https://gist.github.com/zellski/be4e9207ab8e70c4e89062d48ce345ba

We have a lightly hacked-up GLTFLoader.js that instantiates these two classes instead of the unmodified ones.

Would be curious how that works out, or if you run into any issues.. it seems like having SkinnedMesh.copy() keep a reference to the source skeleton but then cloning that skeleton at the scene level would be redundant? But it would be nice to arrive at a way of solving this robustly... I tried with https://github.com/mrdoob/three.js/pull/10807 but ☹️

/cc @takahirox

@donmccurdy

picking up from https://github.com/mrdoob/three.js/issues/14009

Note on the clone failure. I think this responsibility is in the wrong place. Instead of extending the StandardMaterial it seems the ShaderMaterial is built mostly from StandardMaterial's template.

https://github.com/mrdoob/three.js/blob/04e86d0e9f7f4dafffc134064c36049a7368deca/examples/js/loaders/GLTFLoader.js#L578

This has a problem then of managing these uniforms, which is handled in the onBeforeRender, by calling this:
https://github.com/mrdoob/three.js/blob/04e86d0e9f7f4dafffc134064c36049a7368deca/examples/js/loaders/GLTFLoader.js#L775

This is abstract enough that it could easily handled by the renderer, as i've demonstrated here:
https://github.com/mrdoob/three.js/pull/13198/files#diff-c0e88b98497597a015ecf238e91ac3a0R2080

I think it would take the burden from the loader. I'm not sure what bothers me there, but it seems that a "loader" persists quite throughout the app, if every render call depends on some logic being there.

If only certain adjustments need to be made on the StandardMaterial, then i think there wouldn't be a need for this:
https://github.com/mrdoob/three.js/blob/04e86d0e9f7f4dafffc134064c36049a7368deca/examples/js/loaders/GLTFLoader.js#L618


                    .replace( 'uniform float roughness;', 'uniform vec3 specular;' )
                    .replace( 'uniform float metalness;', 'uniform float glossiness;' )
                    .replace( '#include <roughnessmap_pars_fragment>', specularMapParsFragmentChunk )
                    .replace( '#include <metalnessmap_pars_fragment>', glossinessMapParsFragmentChunk )
                    .replace( '#include <roughnessmap_fragment>', specularMapFragmentChunk )
                    .replace( '#include <metalnessmap_fragment>', glossinessMapFragmentChunk )
                    .replace( '#include <lights_physical_fragment>', lightPhysicalFragmentChunk );
//done deal :( it's a big string now

material.shaderOverride.chunks = {
  roughnessmap_pars_fragment: specularMapParsFragmentChunk,
  metalnessmap_pars_fragment: glossinessMapParsFragmentChunk ,
  roughnessmap_fragment: specularMapFragmentChunk ,
  metalnessmap_fragment: glossinessMapFragmentChunk ,
  lights_physical_fragment: lightPhysicalFragmentChunk ,
}

The uniforms would further pave the way to refactoring all of this:
https://github.com/mrdoob/three.js/blob/04e86d0e9f7f4dafffc134064c36049a7368deca/examples/js/loaders/GLTFLoader.js#L786

The pattern i think could be much more concise and clean:

material.shaderOverride.uniforms = {
  specular: { value: new THREE.Vector3() },
  glossiness: { value: 0, type: 'float' } //maybe without the type but int/float ambigous 
}

material.glossiness could then be piped directly into the uniform via get/set.

Note care would have to be taken here to follow the pattern of replacing chunks, in order to erase metalness and roughness, since this would just inject the two new ones.

The benefit of all this is that it's configured data that can be carried over, and frees up onBeforeRender

To maybe further clarify:

I submitted the onBeforeRender mostly with the stencil buffer in mind. I wanted a place where i could issue a low level command to webgl, and have it work with the rest of three.js.

Three lacks a high level interface there, but in spirit it's similar to transparent = true and setting various blending modes.

So imagine you wanted to implement some kind of clipping of a gltf model.

as a user i would like to be able to view a gltf model in a web page and see cross sections of it

You might have to copy the nodes, assign some simple materials, or not draw to the buffer. One Mesh will have one onBeforeRender another a different one, and finally the beauty shot with the PBR comes in, it also needs to set up some stencil state, so it needs the call back.

It makes sense to update uniforms in there (i only found out about this a few days ago), but it would be more tied to the effect. Maybe some uMaskIndex rather than something that remains relatively static throughout its lifetime like roughness = asphaltRoughness.

Storing this stuff in a different place would free up onBeforeRender for other things. One could leverage the same system that parses the other 95% of StandardMaterial and piggy back off of that for this type of update refreshUniforms.

I might me misunderstanding the whole problem though.

@donmccurdy

I actually wasn't able to break the cloning even in the case of spec_gloss. However, if it's any worth, I've made a proposal that demonstrates how to remove the refreshUniforms alltogether, while it still works.

@donmccurdy

Here's the demo:
https://github.com/mrdoob/three.js/pull/14031

I'm not sure if it's useful at all as i don't know the difference between the gltf2 loader and this one, but again, if it's any worth it's an example how one could trim some 160 lines of code from that file, and decouple and free up the onBeforeRender from the extended material. The benefit there is, i could for example still implement some kind of cross-sectioning algorithm using the stencil buffer and such, while not caring about the implementation of the shading part of the _(any)_ Material.

onBeforeCompile as you pointed out, would suffer from the same problems _(holy word of the DOM)_. This slight tweak of the renderer could aleviate this problem.

We'd need an onAfterClone callback or something, because the mesh can't reproduce its own onBeforeRender callback.

please do not implement onAfterClone please, pretty please

tl:dr;

remove any mention of onBeforeRender in GLTFLoader. Refactor / remove some code.

I was unable to reproduce the cloning issue. But this PR uses onBeforeCompile instead of onBeforeRender. https://github.com/mrdoob/three.js/pull/14099

I think this issue is now covered by other, non-glTF-specific, issues:

  • onBeforeCompile may break material's .clone() method (#14009)
  • geometry.clone() doesn't copy reference to bones/skinweights/animations etc (#5878)
  • No way of cloning a mesh so it works with a skinning material (#11574)
Was this page helpful?
0 / 5 - 0 ratings

Related issues

jack-jun picture jack-jun  ·  3Comments

Bandit picture Bandit  ·  3Comments

ghost picture ghost  ·  3Comments

Horray picture Horray  ·  3Comments

jlaquinte picture jlaquinte  ·  3Comments