From 8472f8c6d006fc5c66795dfdf4fdfa847ed4d21e Mon Sep 17 00:00:00 2001
From: James Turner <zakalawe@mac.com>
Date: Thu, 26 Jan 2017 23:14:48 +0000
Subject: [PATCH] Fix positional ordering of remote-canvas elements.

The mirror protocol now sends the position for internal as well as
leaf nodes, and the group uses this data to sort when no explicit
Z-indices exist. This gets the extra-500 much closer to working!
---
 .../http/MirrorPropertyTreeWebsocket.cxx      | 11 ++++-----
 utils/fgqcanvas/elementdatamodel.cpp          |  6 +++++
 utils/fgqcanvas/fgcanvasgroup.cpp             | 24 +++++++++++++------
 utils/fgqcanvas/localprop.cpp                 |  9 +++++++
 utils/fgqcanvas/localprop.h                   | 13 ++++++++++
 utils/fgqcanvas/temporarywidget.cpp           |  4 ++--
 6 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/src/Network/http/MirrorPropertyTreeWebsocket.cxx b/src/Network/http/MirrorPropertyTreeWebsocket.cxx
index 1f652cbbd..e97159a05 100644
--- a/src/Network/http/MirrorPropertyTreeWebsocket.cxx
+++ b/src/Network/http/MirrorPropertyTreeWebsocket.cxx
@@ -165,7 +165,7 @@ using std::string;
 
         virtual void childAdded(SGPropertyNode* parent, SGPropertyNode* child) override
         {
-            // this works becuase customer operator== overload on RecentlyRemovedNode
+            // this works becuase custom operator== overload on RecentlyRemovedNode
             // ignores the ID value.
             RecentlyRemovedNode m(child, 0);
             auto rrIt = std::find(recentlyRemoved.begin(), recentlyRemoved.end(), m);
@@ -201,9 +201,7 @@ using std::string;
 
         void registerSubtree(SGPropertyNode* node)
         {
-            if (node->getType() != simgear::props::NONE) {
-                valueChanged(node);
-            }
+            valueChanged(node);
 
             // and recurse
             int child = 0;
@@ -248,8 +246,9 @@ using std::string;
                     cJSON_AddItemToObject(newPropData, "index", cJSON_CreateNumber(prop->getIndex()));
                     cJSON_AddItemToObject(newPropData, "position", cJSON_CreateNumber(prop->getPosition()));
                     cJSON_AddItemToObject(newPropData, "id", cJSON_CreateNumber(idForProperty(prop)));
-                    cJSON_AddItemToObject(newPropData, "value", JSON::valueToJson(prop));
-
+                    if (prop->getType() != simgear::props::NONE) {
+                        cJSON_AddItemToObject(newPropData, "value", JSON::valueToJson(prop));
+                    }
                     cJSON_AddItemToArray(newNodesArray, newPropData);
                 }
 
diff --git a/utils/fgqcanvas/elementdatamodel.cpp b/utils/fgqcanvas/elementdatamodel.cpp
index 4627fbe6e..e1f79aa38 100644
--- a/utils/fgqcanvas/elementdatamodel.cpp
+++ b/utils/fgqcanvas/elementdatamodel.cpp
@@ -40,6 +40,10 @@ QVariant ElementDataModel::data(const QModelIndex &index, int role) const
             return key;
         }
 
+        if (key == "position") {
+            return m_element->property()->position();
+        }
+
         return m_element->property()->value(key.constData(), QVariant());
     }
 
@@ -67,4 +71,6 @@ void ElementDataModel::computeKeys()
             m_keys.append(b);
         }
     }
+
+    m_keys.append("position");
 }
diff --git a/utils/fgqcanvas/fgcanvasgroup.cpp b/utils/fgqcanvas/fgcanvasgroup.cpp
index a5950b30d..a453a9247 100644
--- a/utils/fgqcanvas/fgcanvasgroup.cpp
+++ b/utils/fgqcanvas/fgcanvasgroup.cpp
@@ -9,6 +9,20 @@
 #include "fgqcanvasmap.h"
 #include "fgqcanvasimage.h"
 
+class ChildOrderingFunction
+{
+public:
+    bool operator()(const FGCanvasElement* a, FGCanvasElement* b)
+    {
+        if (a->zIndex() == b->zIndex()) {
+            // use prop node positions in the parent
+            return a->property()->position() < b->property()->position();
+        }
+
+        return a->zIndex() < b->zIndex();
+    }
+};
+
 FGCanvasGroup::FGCanvasGroup(FGCanvasGroup* pr, LocalProp* prop) :
     FGCanvasElement(pr, prop)
 {
@@ -54,8 +68,7 @@ unsigned int FGCanvasGroup::indexOfChild(const FGCanvasElement *e) const
 void FGCanvasGroup::doPaint(FGCanvasPaintContext *context) const
 {
     if (_zIndicesDirty) {
-        std::sort(_children.begin(), _children.end(), [](const FGCanvasElement* a, FGCanvasElement* b)
-                 { return a->zIndex() < b->zIndex(); });
+        std::sort(_children.begin(), _children.end(), ChildOrderingFunction());
         _zIndicesDirty = false;
     }
 
@@ -82,23 +95,18 @@ bool FGCanvasGroup::onChildAdded(LocalProp *prop)
     if (nm == "group") {
         _children.push_back(new FGCanvasGroup(this, prop));
         newChildCount++;
-        return true;
     } else if (nm == "path") {
         _children.push_back(new FGCanvasPath(this, prop));
         newChildCount++;
-        return true;
     } else if (nm == "text") {
         _children.push_back(new FGCanvasText(this, prop));
         newChildCount++;
-        return true;
     } else if (nm == "image") {
         _children.push_back(new FGQCanvasImage(this, prop));
         newChildCount++;
-        return true;
     } else if (nm == "map") {
         _children.push_back(new FGQCanvasMap(this, prop));
         newChildCount++;
-        return true;
     } else if (nm == "symbol-type") {
         connect(prop, &LocalProp::valueChanged, this, &FGCanvasGroup::markCachedSymbolDirty);
         return true;
@@ -112,7 +120,9 @@ bool FGCanvasGroup::onChildAdded(LocalProp *prop)
     }
 
     if (newChildCount > 0) {
+        markChildZIndicesDirty();
         emit childAdded();
+        return true;
     }
 
     qDebug() << "saw unknown group child" << prop->name();
diff --git a/utils/fgqcanvas/localprop.cpp b/utils/fgqcanvas/localprop.cpp
index 88c5a40ff..014c3a58a 100644
--- a/utils/fgqcanvas/localprop.cpp
+++ b/utils/fgqcanvas/localprop.cpp
@@ -53,6 +53,10 @@ LocalProp::LocalProp(LocalProp *pr, const NameIndexTuple& ni) :
 {
 }
 
+LocalProp::~LocalProp()
+{
+}
+
 void LocalProp::processChange(QJsonValue json)
 {
     QVariant newValue = json.toVariant();
@@ -132,6 +136,11 @@ unsigned int LocalProp::index() const
     return _id.index;
 }
 
+void LocalProp::setPosition(unsigned int pos)
+{
+    _position = pos;
+}
+
 LocalProp *LocalProp::parent() const
 {
     return const_cast<LocalProp*>(_parent);
diff --git a/utils/fgqcanvas/localprop.h b/utils/fgqcanvas/localprop.h
index 0951c370a..aa61d9134 100644
--- a/utils/fgqcanvas/localprop.h
+++ b/utils/fgqcanvas/localprop.h
@@ -57,6 +57,8 @@ class LocalProp : public QObject
 public:
     LocalProp(LocalProp* parent, const NameIndexTuple& ni);
 
+    virtual ~LocalProp();
+
     void processChange(QJsonValue newValue);
 
     const NameIndexTuple& id() const;
@@ -79,6 +81,16 @@ public:
 
     unsigned int index() const;
 
+    /// position in the main FG propery tree. Normally
+    /// irrelevant but unfortunately necessary for correct
+    /// z-ordering of Canvas elements
+    unsigned int position() const
+    {
+        return _position;
+    }
+
+    void setPosition(unsigned int pos);
+
     LocalProp* parent() const;
 
     std::vector<LocalProp*> children() const
@@ -108,6 +120,7 @@ private:
     const LocalProp* _parent;
     std::vector<LocalProp*> _children;
     QVariant _value;
+    unsigned int _position = 0;
 };
 
 #endif // LOCALPROP_H
diff --git a/utils/fgqcanvas/temporarywidget.cpp b/utils/fgqcanvas/temporarywidget.cpp
index 62e504ba4..c951b4f1f 100644
--- a/utils/fgqcanvas/temporarywidget.cpp
+++ b/utils/fgqcanvas/temporarywidget.cpp
@@ -105,7 +105,7 @@ void TemporaryWidget::onTextMessageReceived(QString message)
 
             QByteArray localPath = nodePath.mid(rootPropertyPath.size() + 1);
             LocalProp* newNode = propertyFromPath(localPath);
-
+            newNode->setPosition(newProp.value("position").toInt());
             // store in the global dict
             unsigned int propId = newProp.value("id").toInt();
             if (idPropertyDict.contains(propId)) {
@@ -119,7 +119,7 @@ void TemporaryWidget::onTextMessageReceived(QString message)
         }
 
         // process removes
-        QJsonArray removed = json.object().value("remvoed").toArray();
+        QJsonArray removed = json.object().value("removed").toArray();
         Q_FOREACH (QJsonValue v, removed) {
             unsigned int propId = v.toInt();
             if (idPropertyDict.contains(propId)) {