vsg-dev/vsgXchange

vsgconv ignores empty transforms

Closed this issue · 3 comments

I guess this is currently expected behavior but probably it is possible to extend vsgconv to put empty transforms to vsgt/vsgb files, because this can simplify different transform manipulations from code later.

To clarify the issue there is a sample file exported to gltf from blender. It has one geometry (Cube) and two attached to it empty transforms (Transform_1 and Transform_2). When converted to vsgt there is no mentions about empty transforms, though in program it could be useful to have them named thus could easily be found and used to attach something to Cube by adding children to these transforms.

image

So, I wonder can vsgconv be extended, maybe with some command line arguments to include empty transforms? Maybe if the idea is fine I can provide some PR.

test.zip

I consider something like this. It doesn't ignore empty and identity transforms, thus in code they can be found and transformed.

--- a/src/assimp/assimp.cpp
+++ b/src/assimp/assimp.cpp
@@ -144,6 +144,7 @@ struct SceneConverter
     LightMap lightMap;

     bool useViewDependentState = true;
+    bool includeEmptyTransforms = true;

     // TODO flatShadedShaderSet?
     vsg::ref_ptr<vsg::ShaderSet> pbrShaderSet;
@@ -822,9 +823,19 @@ vsg::ref_ptr<vsg::Node> SceneConverter::visit(const aiNode* node, int depth)
         }
     }

-    if (children.empty()) return {};
+    if (children.empty()) {
+        if (includeEmptyTransforms) {
+            aiMatrix4x4 m = node->mTransformation;
+            m.Transpose();
+            auto transform = vsg::MatrixTransform::create(vsg::dmat4(vsg::mat4((float*)&m)));
+            if (!name.empty()) transform->setValue("name", name);
+            return transform;
+        }
+        return {};
+    }

-    if (node->mTransformation.IsIdentity())
+    if (!includeEmptyTransforms && node->mTransformation.IsIdentity())
     {
         if (children.size() == 1 && name.empty()) return children[0];

I see this is a wish list item, and can see value in being able to do this, though it's something that is probably not appropriate for default behaviour, so I'd suggest it be an vsg::Option passed in like the other options. I would suggest that this is more general than just empty transform nodes, as user might create different types of nodes as placeholders, so perhaps a honour_empty_nodes option - this name is just an off the top of the head suggestion, there may be a better name.

Later I'm planning to add scene graph optimization that would find and remove nodes such as empty nodes, and transforms that are identity, combining parent/child transform sequences, through to flattening transforms. These optimizes would be appropriate to be toggled on/off using a similar scheme to honour_empty_nodes.

I've created PR that preserves old behaviour by default and uses command line parameter to include empty nodes.