mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.74k stars 35.38k forks source link

GLTFExporter reorders children in groups. #15561

Closed MykhailoShavlovskyi closed 2 years ago

MykhailoShavlovskyi commented 5 years ago
Description of the problem

When exporting a model via GLTFExporter, it reorders children in groups (it places groups first and than meshes). For example if my object children structure is [mesh, mesh, group, mesh], the exported object structure will be [group, mesh, mesh, mesh].

Model before export (download here) has a group somewhere in hierarchy 2019-01-11 14_38_04-window

After the export group is placed in the beginning of hierarchy 2019-01-11 14_38_25-window

This behavior is unwanted, as I'm assigning materials based on child index. Would it be possible to adjust the GLTFExporter so it keeps the original hierarchy, in the way that importing and exporting the same model would result in the exact same data?

Three.js version
Browser
OS
donmccurdy commented 5 years ago

I think we can probably fix the ordering inconsistency, but just FYI – you can assign names or mesh.userData.materialName = 'foo' and both should be preserved when you load the model back into three.js. That may be easier and more robust than relying on order in the scene tree.

takahirox commented 5 years ago

I guess this issue isn't in the exporter side but maybe in the loader side.

Three.js child object is added here in the loader.

https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/GLTFLoader.js#L3335

It's asynchronously done in Promise then the order is inconsistent. I'll look into more later...

takahirox commented 5 years ago

@MykhailoShavlovskyi I think #15587 fixes the issue. Can you please test in your side, too?

MykhailoShavlovskyi commented 5 years ago

@MykhailoShavlovskyi I think #15587 fixes the issue. Can you please test in your side, too?

Thanks, it did fix my issue. Although after loading exported model, objects of type THREE.Group are now of type THREE.Object3D. It makes no difference for me, but could you please explain why it was done like this?

donmccurdy commented 5 years ago

@MykhailoShavlovskyi the glTF format does not have the same Object3D / Group distinction that three.js does. They're just treated as generic empty nodes in the scene graph.

Popov72 commented 5 years ago

In current version, nodes that have several children meshes are of type THREE.Group but other nodes that have children which are not meshes are of type THREE.Object3D. Shouldn't we always have the same type of node when a node has children?

Mixing THREE.Object3D and THREE.Group for nodes that have children can be a problem when trying to use the .renderOrder property to control the rendering order of objects.

donmccurdy commented 5 years ago

@Popov72 when THREE.Group is used, the meshes are not children in the glTF file — they’re all part of the same “mesh”. Because threejs doesn’t have multiple geometries per “mesh”, they become children of a group in the loader.

How does that affect .renderOrder? Does THREE.Group not have that? I might not understand the difference between the two...

Popov72 commented 5 years ago

@donmccurdy Let's say you have a .gltf scene with one root node (R) and two children. One of these children is a mesh (A) with a single primitive, so ends up being created as a Mesh, whereas the other one (B) has 2 primitives, so is created as a Group (BG) which has 2 meshes as children (one per primitive -B1/B2 ).

Now I would like to render B before A no matter what (B is a skybox in my scene). So I set .renderOrder of A to 2 and .renderOrder of BG/B1/B2 to 1.

But it does not work because A having no Group node in its parent hierarchy gets a .groupOrder equal to 0 when sorting (see the projectObject function in the WebGLRenderer class).

Because .renderOrder of groups are taken into account first when sorting objects (see painterSortStable function), the objects are not sorted correctly.

I could achieve what I want if the root node R was a Group object and not an Object3D, which it is currently and which was my initial comment. If it's a Group, I can set its .renderObject to 2 and gets the correct result (because then A will have its .groupOrder sets to 2 when sorting).

Granted, it's specific to my need, but it made me think that having some nodes being Object3D and not Group when they have children is not really correct in the GLTF loader because as soon as someone tries to use .renderOrder to sort objects, it won't work as expected because inevitably some meshes won't have any Group parent and will end up with a default .groupOrder equal to 0, which will screw up the sorting.

In my case, I just set .isGroup to true on the Object3D root node, but that's not really the way to correct the problem in my opinion.

[EDIT] Another way to correct the problem is to set the .renderOrder of all Group nodes in the hierarchy to 0 - less a hacky way than the previous one. [/EDIT]

mrdoob commented 5 years ago

Because threejs doesn’t have multiple geometries per “mesh”, they become children of a group in the loader.

Should we make it so threejs supports multiple geometries per mesh?

mrdoob commented 5 years ago

But if we did that, how would the multiple materials align to multiple geometies? Say...

var mesh = new THREE.Mesh( [ geo1, geo2 ], [ mat1, material2, mat3 ] );

Maybe we can make a THREE.CompoundMesh that supports multiple geometries but only one material?

donmccurdy commented 5 years ago

Hm, I don't know what the intended difference between Group and Object3D is, to say whether GLTFLoader should be using either of them this way. I understand that the problem you're running into would be frustrating. We could switch everything to Group, or everything to Object3D, but I have no idea which is "right." Maybe anything that has children should be a Group?

Should we make it so threejs supports multiple geometries per mesh?

I probably wouldn't make an API change like that just based on glTF – glTF puts hardly any requirements on the relationship between primitives in a single mesh. Some primitives could be points, others could be triangles, and still be part of a single mesh. So treating primitives as children of a single Group has been much easier to reason about.

Somewhat related discussion: https://github.com/KhronosGroup/glTF/issues/1278

mrdoob commented 5 years ago

I don't know what the intended difference between Group and Object3D is

Group is "syntactic sugar" but it helps WebGLRenderer know what to expect and how to handle it:

https://github.com/mrdoob/three.js/blob/dev/src/renderers/WebGLRenderer.js#L1253-L1256

If it is a Object3D, WebGLRenderer has to do (and fail) all the checks:

https://github.com/mrdoob/three.js/blob/dev/src/renderers/WebGLRenderer.js#L1253-L1349

donmccurdy commented 5 years ago

Ok! It sounds like GLTFLoader should either:

(a) always use Group instead of Object3D, treat Object3D as abstract. (b) use Group instead of Object3D if the node has >0 children

I think I prefer (a), if anyone would like to open a pull request to make this change. :)

WestLangley commented 5 years ago

I don't know what the intended difference between Group and Object3D is

A Group is not, itself, renderable. That is how I think of it, anyway...


I think the following is true:

  1. If there are no Groups in the scene graph, all .renderOrder settings are absolute.
  2. If there is a Group in the scene graph, then the .renderOrder settings of decedents of the group are relative to the group only.

Who know what happens if the scene graph contains groups, some of which have child groups, and the user tries to control render order...

donmccurdy commented 5 years ago

Given a single model from a library like Sketchfab, it's more likely than not you'll get a scene hierarchy like this:

Screen Shot 2019-09-15 at 6 49 11 PM

Each Object3D above is simply a "node" in glTF syntax or perhaps an "empty" in Blender's; they are not intended by the artist to affect render order. The hierarchy may be deeply nested for many reasons: group animation, wrappers used by exporters/importers to apply a +Y=Up transform, or the artist simply found it easier to work with objects in these groups.

Who know what happens if the scene graph contains groups, some of which have child groups, and the user tries to control render order...

GLTFLoader currently will not nest Groups within Groups — a Group is always the parent of >1 Meshes. Not all Meshes will have a Group as a parent, and there are reasons for it, but that's the source of @Popov72's issue.

Sounds like my last suggestion, replacing all Object3Ds with Groups, would not solve @Popov72's issue, then? And the most direct solution would be to never create Groups in the loader? We can do that, but FBXLoader, ColladaLoader, and OBJLoader all create Groups pretty freely, and I don't think they're doing that for renderOrder-related reasons... 😕

The documentation for THREE.Group did not lead me to expect these side effects, and now that I know I feel like a mesh.groupOrder property might be clearer and that Group should simply mean "not some other subclass of Object3D". 😇

mrdoob commented 5 years ago

I don't understand how renderOrder is relevant in this issue.

As far as I understand the issue here is that GLTFLoader creates extra nodes (Group or Object3D) in the scene graph to try to accommodate the fact that in GLTF a node can have multiple mesh/line/points inside and when we export it back to GLTF then the scene graph is bloated.

Popov72 commented 5 years ago

(sorry for the very long post)

To clear things up, I think we should discuss about the Group class in threejs: what is its purpose?

Looking at source code, the class Group is exactly the same as Object3D, it adds nothing new (https://github.com/mrdoob/three.js/blob/dev/src/objects/Group.js).

The only purpose of it (as I can see it), is to alter the way objects are sorted: see https://github.com/mrdoob/three.js/blob/dev/src/renderers/webgl/WebGLRenderLists.js#L5 https://github.com/mrdoob/three.js/blob/dev/src/renderers/WebGLRenderer.js#L1262

Objects in a scene are first sorted by .renderGroup (meaning, the .renderOrder of the closest parent Group of an object, if any - else the object gets a default .renderGroup value of 0), then for objects with the same .renderGroup values they are sorted by their .renderOrder value (and then by others criteria if those .renderOrder are equal too, see source code).

It adds a new level of grouping/sorting objects together, which can be handy in some circumstances I guess (but to be honest I don't see when given the current implementation - see end of this post). And as @WestLangley said it here (#15484 end of discussion):

Also, you have modified the sorting behavior for everyone to accommodate an edge case. Based on my current understanding, I think adding SortingGroup would have been a better option.

Group is there only to alter the way objects are sorted: so, SortingGroupwould be a better name?

Also, I think creating a (some) new node(s) in the scene graph just for this is a bit overkill, as it means, for eg, it can have a matrix transformation (as Group==Object3D), which feels awkward for sorting purpose. Also, for users of ThreeJS it can be a hard feature to use. Imagine someone loading an external scene, and wanting to alter the sorting order (using groups) of some objects afterwards. He must create Group instances, set them correctly (fill their children property) and inject them into the scene graph. Wouldn't it be easier to just be able to change a property of the objects to achieve this? For me, sorting should be a transversal property of the scene graph, not a constitutive part of it.

So maybe a .sortingGroup property (or .groupOrder) in addition to .renderOrder in the Object3D class would be a better choice? This property would store either a number or a pointer to a SortingGroup instance (that would not inherit from Object3D) and that would simply hold the .renderOrder value to use for the group.

Note that not creating nodes for sorting would obviously correct my problem, as there would be no Group objects in the scene graph.

To avoid too many breaking changes, Group could remain but without the sorting meaning attached to it. So we would have Group === Object3D and would be used for syntactic sugar only.

To conclude, I said above that I don't see the interest of the current implementation of sorting with groups because the .groupOrder an object gets is the .renderOrder of the closest parent Group: what if groups are nested inside groups inside groups? Why use the closest parent and not another one? Imagine I have a Group G1 with .renderOrder=4 and inside it (child) another Group G2 with .renderOrder=2 and inside this group some meshes. Those meshes will get a groupOrder of 2, but surely if I created a group with .renderOrder=4 that's because I wanted its children to have a groupOrder of 4, and so be displayed after groupOrder of 0/1/2/3? So maybe the groupOrder should be that of the first parent Group of the group hierarchy leading to a displayable object? Or maybe it should be the max() of all the groupOrder of the parent hierarchy? As you can see, there are many questions that arise because of the current implementation (at least those are questions I'm asking to myself). With the proposed .sortingGroup property, you get rid of those questions because you can only have a single .groupOrder property for an object, the one pointed to by the property.

=> chances are that you won't be able to achieve a proper sorting order without serious working if you have nested groups in a moderate sized scene and you use real group sorting (you can effectively disable group sorting in a scene with Group objects by setting all those group objects .renderOrder property to 0).

As @WestLangley (again!) said above:

Who know what happens if the scene graph contains groups, some of which have child groups, and the user tries to control render order...

mrdoob commented 5 years ago

@Popov72 Please, do not hijack threads. Feel free to open a new Issue for that discussion.

Popov72 commented 5 years ago

I thought it was related and could help finding a solution, sorry for that.

DVLP commented 5 years ago

When exporting as GLTF and importing it again it breaks every mesh with multiple materials into multiple meshes with one material each and puts them in a group. Additionally each of these primitives still holds entire set of attributes from the original large mesh and accesses them from an index. Maybe this fix will work also for other grouping problems. In GLTFLoader in loadMesh function at the bottom replace:

This:

var group = new THREE.Group();

for ( var i = 0, il = meshes.length; i < il; i ++ ) {

    group.add( meshes[ i ] );

}
return group;

With this:


const mergedGeo = new THREE.BufferGeometry();

// each primitive holds full list of attributes from the original multimaterial mesh
// therefore no need to merge attributes we just take them from the first primitive
mergedGeo.attributes = meshes[0].geometry.attributes;

let indexPos = 0;

for ( i = 0, il = meshes.length; i < il; i ++ ) {

    mergedGeo.groups.push({
        start: indexPos,
        count: meshes[i].geometry.index.count,
        materialIndex: i,
    });

    indexPos += meshes[i].geometry.index.count;

}

const mergedIndex = new Uint32Array(indexPos);

indexPos = 0;

for ( i = 0, il = meshes.length; i < il; i ++ ) {

    mergedIndex.set(meshes[i].geometry.index.array, indexPos);
    indexPos += meshes[i].geometry.index.count;

}

mergedGeo.setIndex(new THREE.BufferAttribute(mergedIndex, 1));

return new THREE.Mesh(mergedGeo, originalMaterials);

This will rebuild groups and merge indices. Thankfully no need to mess with other attributes.

Popov72 commented 5 years ago

It's by design that meshes are not merged, it has been discussed in a number of issues. See for eg #11944, #13707, #15889

DVLP commented 5 years ago

@Popov72 Thanks for these links. If this is by design then entire three.js approach might change with getting rid of multi materials(?). If not then probably flattening should be available behind a flag in GLTF loader. Anyway for my purposes I'll stick to flattening by default to not modify objects structure imported from other formats and to avoid unnecessary matrix multiplications.

donmccurdy commented 5 years ago

If this is by design then entire three.js approach might change with getting rid of multi materials(?).

Exporting to any format and then re-importing the model may change the model's structure somewhat. No third-party format has exactly the same representation of things that three.js does; only scene.toJSON() is exactly 1:1. Solutions like the one you show above are good for re-creating the original structure, when you already know what it was, but we can't make those assumptions in GLTFLoader.

If not then probably flattening should be available behind a flag in GLTF loader.

We removed it because it was complex to maintain and caused bugs; we're not considering re-introducing it at this time. There are some utilities (like BufferGeometryUtils.mergeBufferGeometries()) that can be helpful for users who want to flatten a scene themselves, although it is meant for a slightly different case than your code above.

Anyway for my purposes I'll stick to flattening by default...

👍


@DVLP Note that this is different from the original topic here; if you'd like to discuss further it would be better to open a new issue.

donmccurdy commented 2 years ago

I think this is the same issue as https://github.com/mrdoob/three.js/issues/18774, let's merge them.