overrideMaterial for nested scenes

[nullEuro]

It is possible to add a scene to another scene. However, setting an overrideMaterial on a nested scene has no effect. Only the overrideMaterial of the root scene is honored (and changes the material of children of nested scenes also as expected).

I expected (and am suggesting now) that it is possible to set an overrideMaterial on any scene, and that it overrides the overrideMaterial of parent scenes. Though that may be debatable. But the documentation in this regard could be improved in either case.

My use case: I have several Sub-Scenes loaded from different GLTF files in my root scene, and want to highlight specific ones under certain conditions with a special material.

Live Demo

[WestLangley]

I do not recall support for sub-scenes ever being considered in that regard...

In any event, a workaround is to use multiple render passes.

https://jsfiddle.net/d2je03mc/

[makc]

@nullEuro would this not have unexpected effects, e g when people just do someObject.add(gltfLoaderResult.scene) and then try to apply effects that set overrideMaterial on the root scene and the entire gltf is still rendered as is because its overrideMaterial is not set?

[WestLangley]

Yeah. Never liked that GLTFLoader adds a THREE.Scene as a child of THREE.Scene.

/ping @donmccurdy

[donmccurdy]

Looking at other properties on Scene objects, nesting them would be problematic... .background, .environment, .fog... I don't think we can support those properties, or .overrideMaterial, on nested scenes.

@nullEuro I believe you'll need to directly change the .material property on meshes you want to effect.

@WestLangley a THREE.Scene is the most obvious representation of a glTF Scene object. It's difficult to say universally whether a glTF file is a scene or is an object added to a scene. The following is valid:

loader.load( 'foo.glb', ( gltf ) => {
  renderer.render( gltf.scenes[0], gltf.cameras[0] );
} );

I don't mind changing the return values as shown below, though.

// before
{
  scenes: Scene[];
} 

// after
{
  scenes: Object3D[];
} 

But I'm not aware that nested scenes have ever been discussed...

[Mugen87]

Looking at other properties on Scene objects, nesting them would be problematic... .background, .environment, .fog... I don't think we can support those properties, or .overrideMaterial, on nested scenes.

Indeed, these properties assume they are only present in a "root" object.

I believe you'll need to directly change the .material property on meshes you want to effect.

That sounds like a proper workaround to me. Object3D.traverse() might help here.

A THREE.Scene is the most obvious representation of a glTF Scene object. It's difficult to say universally whether a glTF file is a scene or is an object added to a scene.

Hence, using THREE.Scene in this context is absolutely fine from my point of view.

But I'm not aware that nested scenes have ever been discussed...

No, not to my knowledge. I'm also not a fan of this idea. It seems more appropriate to me to handle such specific use cases on application layer.

[donmccurdy]

Ok, let's change the return type of GLTFLoader to {scenes: Object3D[], ...}. If we want to do something to more explicitly prevent Scenes from being nested, library-wide, I'll leave that decision to others.

[nullEuro]

Using multiple render passes is an interesting idea, though not really practical for my case.

Traversing the subscene is exactly what I did before I found overrideMaterial and thought it was a great way to simplify my code, later trying to find out why it did not work.

I did not even consider that nesting scenes is not really an intended feature. I think changing the signature of the GLTFLoader result is a good idea and helps to prevent this false assumption, since loading a model and adding it to an existing scene is probably a common use case. I don't have much experience with three.js though.

Anyway thanks for the great discussion.

[WestLangley]

@donmccurdy Can scene be a Group and scenes be a Group[]?

[donmccurdy]

I'd be fine with that too, although use of Group has created its own unplanned side effects (https://github.com/mrdoob/three.js/issues/15561#issuecomment-529136708). For .renderOrder to be relative to the root Group for that glTF asset doesn't seem too bad...

[WestLangley]

These issues are a but conflated...

I think for the purpose of sorting, .renderOrder should be per renderable object without regard to hierarchy.

I do not think the current sorting algorithm handles sorting groups correctly -- or if it ever will. See #18176.

[donmccurdy]

I'm not sure if you're suggesting we should return Group from GLTFLoader, or that the unintuitive effects mentioned in https://github.com/mrdoob/three.js/pull/18176 mean Object3D would be safer?

[WestLangley]

I was suggesting :

1. GLTFLoader return Group,
2. renderOrder should not consider hierarchy.

[Mugen87]

renderOrder should not consider hierarchy.

I do not vote to revert #15484 since I know projects which successfully use it for sprite rendering. Please study the history of this PR. If this logic is removed, a replacement is necessary. E.g. a new class like SortingGroup.

[WestLangley]

I do not vote to revert #15484

But #15484 does not work correctly. See if you can help @gkjohnson in #18176. If you can figure out how to make it work, that would be great.

Otherwise, I recommend we do what I suggested above:

renderOrder should not consider hierarchy.

[Mugen87]

But #15484 does not work correctly.

15484 does what it is supposed to do. Without the PR, it's not possible to render the following scene correctly.

With group order: https://jsfiddle.net/0oz785eh/1/ Without: https://jsfiddle.net/0oz785eh/2/

The behavior might need an enhancement for other use cases but that does not mean the current implementation is "wrong".

[WestLangley]

15484 does what it is supposed to do.

I the arguments in #18176 make a compelling case that #15484 does not do what the docs say it does.

[Mugen87]

Then we need maybe adjust the docs. Sorry, I had not time so far to study #18176 closer.

[gkjohnson]

@WestLangley

In any event, a workaround is to use multiple render passes.

It's only kind of a workaround, unfortunately. Rendering multiple scenes in sequence as you've shown means you have keep lights in sync which can be a pain. I have a suggestion here which would make it easier to share lighting state across multiple scene renders by passing a lights array into the render function and make cases like this more simple.

@donmccurdy

I'd be fine with that too, although use of Group has created its own unplanned side effects (#15561 (comment)). For .renderOrder to be relative to the root Group for that glTF asset doesn't seem too bad...

I'm not sure if you're suggesting we should return Group from GLTFLoader, or that the unintuitive effects mentioned in #18176 mean Object3D would be safer?

In my opinion this should be a reason to rethink / remove Group.renderOrder. As it is now users are in a position where they have to decide whether they should use Group, Scene, or Object3D just to avoid the undesireable and moderately controllable render order behavior imposed by Group -- suddenly using Group vs Object3D is a meaningful distinction that can result in some pretty brittle and hard to understand code.

Just my personal experience but I struggled for awhile to understand exactly what the renderOrder is meant to do and only really understood it after digging into the source and getting to WebGLRenderList. I still find it a bit hard to describe how it works.

@Mugen87

I do not vote to revert #15484 since I know projects which successfully use it for sprite rendering. Please study the history of this PR. If this logic is removed, a replacement is necessary. E.g. a new class like SortingGroup.

I've taken a look at the issue that spawned the renderOrder solution (#14415) and I'm maybe more confused. In the examples and fiddles provided it's unclear why creating two meshes and putting them in a Group with a unique render order is somehow more desirable than the multi material Mesh with a unique render order? If renderOrder dictating that the whole subhierarchy render together is somehow prohibitive then maybe another opt-in solution like a RenderGroup class that sorts and renders all deep Mesh and RenderGroup instances together would work. At the moment I'm not even sure what renderOrder is offering for the original use case. And if it did then it breaks as soon as any of those sprite elements are put into a subgroup.

[DusanBosnjakKodiak]

Something like this:

const stack = []
const traverse = o =>{
  if( o instanceof Scene){ 
    stack.unshift(o)
  }
  if(o.material){
    o.material = stack[0]
  }
  o.children.forEach(traverse)
  if( o instanceof Scene){
    stack.shift()
  }
}

I think the issue is, what if you want to do 10 of these things. You group them into one traversal. You still end up with at least one more because of scene.updateMatrixWorld(). But the problem of applying nested overrides is fairly trivial.

I think that there is no difference between Object3D and Scene. Scene.whatever might as well be Object3D.userData.whatever. Ie, any Object3D could be told to have a background, and the topmost ones would be honored.

[mrdoob]

I don't think we should support nested Scenes.

GLTFLoader should indeed return Group instead of Scene but I understand #15484 is problematic. I'd be open to revert it. Lets continue that discussion in #18176.