Change axis object animations to be compatible with <=2020.3 refs Ticket #2706

The changes to the sorting rules caused a breaking change that was incompatible.

The solution to this is to revert to being compatible with <=2020.3 but also to allow modellers to choose the new sorting rules.

I've added a new tag <defaults> which can contain either <axis-animation-vertex-order-xyz/> or <axis-animation-vertex-order-x/>. The defaults will affect all animations in the .xml and also any included models unless the included model also has a <defaults> section.

So for modelers that want to use the new definitive vertex sorting rules adding the following to the main model is sufficient.

    <defaults>
        <axis-animation-vertex-order-xyz/>
    </defaults>

There are also a couple of new tags that can be used in the <axis> section.

1. <order-by-xyz/> - use the new sorting rules
2. <order-by-x/> - use the 2020.3 sorting rules
3. <swap-axis-direction/> - when the animation goes the wrong way this is an elegant way to fix it.

See https://sourceforge.net/p/flightgear/codetickets/2706/

simgear/scene/model/SGReaderWriterXML.cxx

@@ -523,6 +523,10 @@ sgLoad3DModel_internal(const SGPath& path,
     osg::ref_ptr<SGModelData> data = options->getModelData();
     options->setModelData(0);
 
+    // remeber the current value of the vertex order setting 
+    // because an included <model> may change this.
+    bool currentVertexOrderXYZ = options->getVertexOrderXYZ();
+
     osg::ref_ptr<osg::Node> model;
     osg::ref_ptr<osg::Group> group;
     SGPropertyNode_ptr props = new SGPropertyNode;
@@ -551,6 +555,13 @@ sgLoad3DModel_internal(const SGPath& path,
             return std::make_tuple(0, (osg::Node *) NULL);
         }
 
+        if (props->hasChild("defaults")) {
+            SGPropertyNode_ptr defaultsNode = props->getNode("defaults");
+            if (defaultsNode->hasChild("axis-animation-vertex-order-xyz"))
+                options->setVertexOrderXYZ(true);
+            if (defaultsNode->hasChild("axis-animation-vertex-order-x"))
+                options->setVertexOrderXYZ(false);
+        }
         if (props->hasValue("/path")) {
             string modelPathStr = props->getStringValue("/path");
             modelpath = SGModelLib::findDataFile(modelPathStr, NULL, modelDir);
@@ -845,6 +856,9 @@ sgLoad3DModel_internal(const SGPath& path,
 
     animationcount += animation_nodes.size();
 
+    // restore the vertex order in case a submodel changed it.
+    options->setVertexOrderXYZ(currentVertexOrderXYZ);
+
     if (!needTransform && group->getNumChildren() < 2) {
         model = group->getChild(0);
         group->removeChild(model.get());

simgear/scene/model/animation.cxx

@@ -97,7 +97,7 @@ class LineCollector : public osg::NodeVisitor {
     };
 
 public:
-    LineCollector() : osg::NodeVisitor(osg::NodeVisitor::NODE_VISITOR, osg::NodeVisitor::TRAVERSE_ALL_CHILDREN) { }
+    LineCollector(bool orderXYZ, bool swapAxis) : osg::NodeVisitor(osg::NodeVisitor::NODE_VISITOR, osg::NodeVisitor::TRAVERSE_ALL_CHILDREN), _orderXYZ(orderXYZ), _swapAxis(swapAxis){ }
 
     virtual void apply(osg::Geode& geode)
     {
@@ -141,14 +141,37 @@ public:
         SGVec3f tv1(toSG(_matrix.preMult(v1)));
         SGVec3f tv2(toSG(_matrix.preMult(v2)));
 
-        // Get the ends in the right order based on their 
+        if (_orderXYZ)
-        // lowest coordinates in x,y,z
+        {
-        if (compareVec3(v1, v2))
+            // Get the ends in the right order based on their 
-            _lineSegments.push_back(SGLineSegmentf(tv1, tv2));
+            // lowest coordinates in x,y,z
+            // This gives us a definitive vertex order in all cases
+            // whereas previously when X was equal the order would
+            // effectively be determined by the order of the vertices in the
+            // model file.
+            if (compareVec3(v1, v2))
+                addLineSegment(tv1, tv2);
+            else
+                addLineSegment(tv2, tv1);
+        }
         else
-            _lineSegments.push_back(SGLineSegmentf(tv2, tv1));
+        {
+            // 2020.3 and prior: sort only by X
+            if (tv1[0] > tv2[0])
+                addLineSegment(tv1, tv2);
+            else
+                addLineSegment(tv2, tv1);
+        }
+    }
+    
+    // add a line segment handling axis swapping.
+    void addLineSegment(SGVec3f v1, SGVec3f v2)
+    {
+        if (_swapAxis)
+            _lineSegments.push_back(SGLineSegmentf(v2, v1));
+        else
+            _lineSegments.push_back(SGLineSegmentf(v1, v2));
     }
-
     void addBVHElements(osg::Node& node, simgear::BVHLineGeometry::Type type)
     {
         if (_lineSegments.empty())
@@ -180,6 +203,8 @@ public:
 private:
     osg::Matrix _matrix;
     std::vector<SGLineSegmentf> _lineSegments;
+    bool _orderXYZ; /// 2020.3 and prior sorting vertex ordering rules (Only compare X component of vector)
+    bool _swapAxis; // invert the vector direction.
 };
 
 /**
@@ -730,11 +755,12 @@ const SGLineSegment<double>* SGAnimation::setCenterAndAxisFromObject(osg::Node *
     std::string axis_object_name = std::string();
     bool can_warn = true;
 
-    if (_configNode->hasValue("axis/object-name"))
+    const SGPropertyNode* axisNode =_configNode->getNode("axis");
-    {
+
-        axis_object_name = _configNode->getStringValue("axis/object-name");
+    if (axisNode->hasValue("object-name")) {
+        axis_object_name = axisNode->getStringValue("object-name");
     }
-    else if (!_configNode->getNode("axis")) {
+    else if (!axisNode) {
         axis_object_name = _configNode->getStringValue("object-name") + std::string("-axis");
         // for compatibility we will not warn if no axis object can be found when there was nothing 
         // specified - as the axis could just be the default at the origin
@@ -759,13 +785,42 @@ const SGLineSegment<double>* SGAnimation::setCenterAndAxisFromObject(osg::Node *
             osg::Group *object_group = axis_object_name_finder.getGroup();
             if (object_group)
             {
+                /*
+                 * work out which vertex sorting rule to use.
+                 * - specified in the axis node
+                 * - or when not specified use the <defaults> value in the options.
+                 */
+                bool orderXYZ = false; 
+
+                // check to see if this node specifies the vertex sorting rule
+                if (axisNode != nullptr && axisNode->hasChild("order-by-xyz"))
+                    orderXYZ = true;
+                else if (axisNode != nullptr && axisNode->hasChild("order-by-x"))
+                    orderXYZ = false;
+                else {
+                    // no local definition so use the <defaults> specified vertex order.
+                    const SGReaderWriterOptions* wOpts = dynamic_cast<const SGReaderWriterOptions*>(modelData.getOptions());
+                    if (wOpts)
+                        orderXYZ = wOpts->getVertexOrderXYZ();
+                }
+
+                /*
+                 * Check to see if we need to swap the axis direction (i.e. the 
+                 * collected line segment vertices).
+                 * This helps to avoid having to negate the rotations to get the
+                 * required direction.
+                 */
+                bool swapAxis = false;
+                if (axisNode != nullptr && axisNode->hasChild("swap-axis-direction")) {
+                    swapAxis = true;
+                }
                 /*
                  * we have found the object group (for the axis). This should be two vertices
                  * Now process this (with the line collector) to get the vertices.
                  * Once we have that we can then calculate the center and the affected axes.
                  */
                 object_group->setNodeMask(0xffffffff);
-                LineCollector lineCollector;
+                LineCollector lineCollector(orderXYZ, swapAxis);
                 object_group->accept(lineCollector);
                 std::vector<SGLineSegmentf> segs = lineCollector.getLineSegments();
 

simgear/scene/util/SGReaderWriterOptions.hxx

@@ -80,7 +80,8 @@ public:
         _instantiateMaterialEffects(false),
         _autoTooltipsMaster(false),
         _autoTooltipsMasterMax(0),
-        _LoadOriginHint(ORIGIN_MODEL)
+        _LoadOriginHint(ORIGIN_MODEL),
+        _vertexOrderXYZ(false)
     { }
     SGReaderWriterOptions(const SGReaderWriterOptions& options,
                           const osg::CopyOp& copyop = osg::CopyOp::SHALLOW_COPY) : osgDB::Options(options, copyop),
@@ -163,6 +164,11 @@ public:
     void setAutoTooltipsMasterMax(int autoTooltipsMasterMax)
     { _autoTooltipsMasterMax = autoTooltipsMasterMax; }
 
+    // the VertexOrderXYZ defines the sorting rule to use for
+    // axis objects in animations.
+    bool getVertexOrderXYZ() const                  { return _vertexOrderXYZ; }
+    void setVertexOrderXYZ(bool vertexOrderXYZ)     { _vertexOrderXYZ = vertexOrderXYZ; }
+
     static SGReaderWriterOptions* copyOrCreate(const osgDB::Options* options);
     static SGReaderWriterOptions* fromPath(const SGPath& path);
 
@@ -210,6 +216,7 @@ private:
     SGGeod _geod;
     mutable LoadOriginHint _LoadOriginHint;
     ErrorContext _errorContext;
+    bool _vertexOrderXYZ; // used for axis objects in animations
 };
 
 }