Add vsg::Mask support to vsg::Group

[drywolf]

Is your feature request related to a problem? Please describe.

I am doing some quite extensive manipulation/decoration of scene-graphs that have been loaded via vsgXchange from 3D scene files. One thing that has been missing for me is the ability to use Masking on vsg::Group. For example, for some rendering-effects I am creating duplicates of each vsg::StateGroup in the scene-graph, but those groups should only be included in the rendering, when a particular bit in the vsg::View -> mask is active.

I know there already are:

• vsg::Switch
  • but for this, you always must set N masks, not just a single mask for the entire group (where N is the number of children)
  • this could be especially tricky/error-prone if you start adding children at runtime, while wanting the same mask for all children (you better don't forget to set the mask for a newly added child)
• vsg::StateSwitch
  • uses the same mechanism as vsg::Switch, but for a list of vsg::StateCommands
  • here also you have to set N masks, not just 1

Describe the solution you'd like

I am wondering why vsg::Group doesn't already come with this feature?! I would propose to just let every vsg::Group (incl. derived types) have a vsg::Mask property, then controlling the behavior for more advanced scene-graphs would be way more flexible & easy to get right.

I am already using the following patch to vsg::Group, and it makes many things much easier for me to handle:

diff --git forkSrcPrefix/include/vsg/nodes/Group.h forkDstPrefix/include/vsg/nodes/Group.h
index a39aac2ad1fc66b8d7302fc7a77e522c9dd1a86b..071430d9e6c0d1649a653cf11532291ee0cd3552 100644
--- forkSrcPrefix/include/vsg/nodes/Group.h
+++ forkDstPrefix/include/vsg/nodes/Group.h
@@ -31,9 +31,14 @@ namespace vsg
             for (Iterator itr = begin; itr != end; ++itr) addChild(*itr);
         }

+        Mask mask = MASK_ALL;
+
         template<class N, class V>
         static void t_traverse(N& node, V& visitor)
         {
+            if ((visitor.traversalMask & (visitor.overrideMask | node.mask)) == MASK_OFF)
+                return;
+
             for (auto& child : node.children) child->accept(visitor);
         }

diff --git forkSrcPrefix/src/vsg/nodes/Group.cpp forkDstPrefix/src/vsg/nodes/Group.cpp
index 5a885229f8cebde50c1f79aeb3f29ea845e3ec7d..5d8e1323c44976452e394966f90dcb16457df3ce 100644
--- forkSrcPrefix/src/vsg/nodes/Group.cpp
+++ forkDstPrefix/src/vsg/nodes/Group.cpp
@@ -40,6 +40,7 @@ void Group::read(Input& input)
 {
     Node::read(input);

+    input.read("mask", mask);
     input.readObjects("children", children);
 }

@@ -47,5 +48,6 @@ void Group::write(Output& output) const
 {
     Node::write(output);

+    output.write("mask", mask);
     output.writeObjects("children", children);
 }

Describe alternatives you've considered

... see the mention of vsg::Switch above, but IMO there are two different use-cases

• either you want to control the mask of each child in a graph-node individually ---> use vsg::Switch
• or you want to control all the children in a graph-node via a single uniform mask ---> use vsg::Group (with the shown changes above)

Additional context

I need to apply vsg::Mask on vsg::StateGroups ... that's because I create clones of the original vsg::StateGroups that I find in a scene-graph (loaded via vsgXchange), and then I insert the cloned StateGroup to the same parents as the original vsg::StateGroup, but the cloned StateGroups get a vsg::Mask, so I can turn them on/off at any time.

Thanks

[robertosfield]

I'm sorry but this suggestion breaks the efficiency of the VSG, the VSG very purposefully avoids putting extra members into classes that add extra memory footprint and extra if/else blocks into traversal as it impacts cache coherency and adds branching that is unnecessary for the vast majority of scene graph nodes.

The vsg::Switch exists for the purpose of adding mask support and can be used for just scene graph locations that required it. The performance overhead is only borne by scene graphs that require it and the places that require it, not dumped on all internal nodes of the scene graph.

There is also a StateSwitch class that can be used for switching between different StateCommands, so can it can assigned to a StateGroup or just placed into the scene graph:

https://github.com/vsg-dev/VulkanSceneGraph/blob/master/include/vsg/state/StateSwitch.h

[drywolf]

I can understand/accept that 👍 (although might only begin to make a noticeable difference once you start having ten-thousands of vsg::Groups in the graph)

In my fork I already have this code, I will keep an eye on it how it will impact performance. Otherwise I will just have to accept the overhead of inserting an additional vsg::Switch into the hierarchy (this should still be easy enough to change later, if necessary)

[robertosfield]

There shouldn't be a need to modify the VSG itself as you can always subclass from vsg::Group and override the accept() method to implement your own mask handling.

However, vsg::Switch is pretty efficient, grafting in a mask into vsg::Group will likely loose more performance than gain because you are forcing extra memory usage and checks on all group nodes that you really only want in very specific places in the scene graph.

FYI, I start working on shadow support, it's only an the design stage but I expect the to need to tackle the issue of state specialization and masking based on whether the traversal of for shadow map generation or a 3d view that will utilize the shadow maps. I don't know how it'll all shake out yet but there may be things that change in the core VSG and the vsgXchnage::assimp loader to make it all work efficiently and more seamlessly for end users than can be achieved right now.

[drywolf]

There shouldn't be a need to modify the VSG itself as you can always subclass from vsg::Group and override the accept() method to implement your own mask handling.

Yeah, I also already thought about this approach, but my situation is that I need to change the mask of vsg::StateGroups, and those vsg::StateGroups are not even created by me, but are created by vsgXchange while loading the scene.

So even if I sub-classed vsg::StateGroup and override its accept() method, I would still have to pluck the original vsg::StateGroup out of the scene-graph, copy over all properties from the StateGroup to my derived version of the class, and then put the custom StateGroup class instance into the exact same spot in the scene-graph where I removed the original StateGroup.

So while technically possible, this is far from a code-path that is easy to reason about nor easy to maintain long-term. (for now I will stick with my forked version of vsg::Group and see how things evolve)

... grafting in a mask into vsg::Group will likely loose more performance than gain ...

I don't use a vsg::Group mask because I hope to get better performance, I use it because it is the way to have the most readable & maintainable form of the code for the usecase that I'm trying to achieve. In this case, I am more than happy to lose a few nanoseconds per frame, but have code that people other than me actually are able to read & maintain 😅

FYI, I start working on shadow support, it's only an the design stage but I expect the to need to tackle the issue of state specialization and masking based on whether the traversal of for shadow map generation or a 3d view that will utilize the shadow maps. I don't know how it'll all shake out yet but there may be things that change in the core VSG and the vsgXchnage::assimp loader to make it all work efficiently and more seamlessly for end users than can be achieved right now.

I'm looking forward to this. The more different rendering-scenarios / rendering-effects that are thrown at the current VSG API, the more we can see where changes / additions are still needed. Great work 👍

[robertosfield]

The tradeoff you describe is the slippery slope that the OpenSceneGraph fell down and why I was able to get the VulkanSceneGraphs traversals 10 X faster. We aren't talking a few nanoseconds, this design/implementation decisions all add up and can make a massive difference.

If you are working with a custom version of the vsg::Group in your own VSG version then all your serialization will be incompatible and your upgrade path will be impacted.

What you think is "convenient" currently is an approach I'd strongly recommend against, it will cause far more problems down the line.

[drywolf]

Thanks for the additional explanations ❤️ I very much appreciate you sharing your insights/experience!

If you are working with a custom version of the vsg::Group in your own VSG version then all your serialization will be incompatible and your upgrade path will be impacted.

That is a good point and makes me reconsider the choice I made ... Also because vsg::Group is used quite a lot throughout the VSG code-base, and also as a base-class for several other VSG classes. You are right, the consequences of this modification are probably not to be under-estimated.

I will try and refactor my code to not rely on vsg::Group mask 👍