From 70c3d8596b36b411268b4e743146b4073999e7e2 Mon Sep 17 00:00:00 2001
From: "Zheng, Lei" <realthunder.dev@gmail.com>
Date: Wed, 21 Feb 2018 18:13:03 +0800
Subject: [PATCH] Document: 2-pass recompute for dependency inversion

Here is a simple example of dependency inversion:

A parent object has some child object, but the child object can only be
updated after the parent object finished update, because it requires
some information from the parent.

A concrete example will be Sketcher::SkechExport, that export partial
geometry element from a SkechObject. For better object hierarchy,
SketchExport is grouped under SketchObject. However, it can only be
updated after SketchObject finished recompute

This patch offers a quick and dirty solution to this dependency
inversion problem. When Document::recompute(objs) is called, the first
pass proceed as normal. After its done, it will check if any object is
still touched, and mark those touched object with
ObjectStatus::Recompute2 flag. It will then launch a second pass of
recompute. Those parent objects that need dependency inversion shall
check its children for this flag, and do not touch them if found. If
there are still objects touched after second pass, an error message
will be printed.
---
 src/App/Document.cpp     | 62 ++++++++++++++++++++++++----------------
 src/App/DocumentObject.h |  3 +-
 2 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/src/App/Document.cpp b/src/App/Document.cpp
index ce9fc6dc6561..d4fd8e0b99d6 100644
--- a/src/App/Document.cpp
+++ b/src/App/Document.cpp
@@ -2491,15 +2491,13 @@ int Document::recompute(const std::vector<App::DocumentObject*> &objs)
 
 #else //ifdef USE_OLD_DAG
 
-int Document::recompute(const std::vector<App::DocumentObject*> &objs)
-{
+int Document::recompute(const std::vector<App::DocumentObject*> &objs) {
+    int objectCount = 0;
+
     if (testStatus(Document::Recomputing)) {
         // this is clearly a bug in the calling instance
         throw Base::RuntimeError("Nested recomputes of a document are not allowed");
     }
-
-    int objectCount = 0;
-
     // The 'SkipRecompute' flag can be (tmp.) set to avoid too many
     // time expensive recomputes
     bool skip = testStatus(Document::SkipRecompute);
@@ -2534,7 +2532,7 @@ int Document::recompute(const std::vector<App::DocumentObject*> &objs)
         cerr << "App::Document::recompute(): cyclic dependency detected" << endl;
         topoSortedObjects = d->partialTopologicalSort(depObjs);
     }
-    for (auto objIt = topoSortedObjects.rbegin(); objIt != topoSortedObjects.rend(); ++objIt){
+    std::reverse(topoSortedObjects.begin(),topoSortedObjects.end());
 #else
     vector<DocumentObject*> topoSortedObjects;
     try {
@@ -2544,28 +2542,44 @@ int Document::recompute(const std::vector<App::DocumentObject*> &objs)
         topoSortedObjects = d->partialTopologicalSort(getDependencyList(objs.empty()?d->objectArray:objs,false));
         std::reverse(topoSortedObjects.begin(),topoSortedObjects.end());
     }
-    for (auto objIt = topoSortedObjects.begin(); objIt != topoSortedObjects.end(); ++objIt){
 #endif
-        // ask the object if it should be recomputed
-        if ((*objIt)->isTouched() || (*objIt)->mustExecute() == 1){
-            objectCount++;
-            if (_recomputeFeature(*objIt)) {
-                // if something happened break execution of recompute
-                return -1;
-            }
-            else{
-                (*objIt)->purgeTouched();
-                // set all dependent object touched to force recompute
-                for (auto inObjIt : (*objIt)->getInList())
-                    inObjIt->touch();
+    size_t idx = 0;
+    // maximum two passes to allow some form of dependency inversion
+    for(int passes=0; passes<2 && idx<topoSortedObjects.size(); ++passes) {
+        FC_LOG("Recompute pass " << passes);
+        for (;idx<topoSortedObjects.size();++idx) {
+            auto obj = topoSortedObjects[idx];
+            // ask the object if it should be recomputed
+            if (obj->isTouched() || obj->mustExecute() == 1) {
+                objectCount++;
+                if (_recomputeFeature(obj)) {
+                    // if something happened break execution of recompute
+                    return -1;
+                }
+                else{
+                    obj->purgeTouched();
+                    // set all dependent object touched to force recompute
+                    for (auto inObjIt : obj->getInList())
+                        inObjIt->touch();
+                }
             }
         }
-    }
-    if(FC_LOG_INSTANCE.isEnabled(FC_LOGLEVEL_LOG)) {
         // check if all objects are recalculated which were thouched 
-        for (auto obj : topoSortedObjects) {
-            if (obj->isTouched())
-                FC_ERR(obj->getNameInDocument() << " still touched after recompute");
+        for (size_t i=0;i<topoSortedObjects.size();++i) {
+            auto obj = topoSortedObjects[i];
+            obj->setStatus(ObjectStatus::Recompute2,false);
+            if(obj->isTouched()) {
+                if(passes>0) 
+                    FC_ERR(obj->getNameInDocument() << " still touched after recompute");
+                else{
+                    FC_LOG(obj->getNameInDocument() << " still touched after recompute");
+                    if(idx>=topoSortedObjects.size()) {
+                        // let's start the next pass on the first touched object
+                        idx = i;
+                    }
+                    obj->setStatus(ObjectStatus::Recompute2,true);
+                }
+            }
         }
     }
 
diff --git a/src/App/DocumentObject.h b/src/App/DocumentObject.h
index e74b17e82264..f8d497522b41 100644
--- a/src/App/DocumentObject.h
+++ b/src/App/DocumentObject.h
@@ -52,7 +52,8 @@ enum ObjectStatus {
     Remove = 5,
     PythonCall = 6,
     Destroy = 7,
-    Expand = 16
+    Recompute2 = 8,
+    Expand = 16,
 };
 
 /** Return object for feature execution