Added pimpl on DiffManager

src/definition/DefinitionNode.cpp

@@ -149,7 +149,7 @@ void DefinitionNode::addWatcher(DefinitionWatcher *watcher, bool init_diff) {
     }
 }
 
-int DefinitionNode::getWatcherCount() const {
+unsigned long DefinitionNode::getWatcherCount() const {
     if (root && root->diffs) {
         return root->diffs->getWatcherCount(this);
     } else {
@@ -265,7 +265,8 @@ void DefinitionNode::addDiff(const DefinitionDiff *diff) {
     if (root && root->diffs) {
         root->diffs->addDiff(this, diff);
     } else {
-        // TODO Apply diff ?
+        // No diff manager available, apply it directly and delete it
+        applyDiff(diff);
         delete diff;
     }
 }

src/definition/DefinitionNode.h

@@ -43,7 +43,7 @@ class DEFINITIONSHARED_EXPORT DefinitionNode {
     inline DiffManager *getDiffManager() const {
         return diffs;
     }
-    inline int getChildrenCount() const {
+    inline unsigned long getChildrenCount() const {
         return children.size();
     }
 
@@ -94,7 +94,7 @@ class DEFINITIONSHARED_EXPORT DefinitionNode {
     /**
      * Get the current number of watchers.
      */
-    int getWatcherCount() const;
+    unsigned long getWatcherCount() const;
 
   protected:
     void addChild(DefinitionNode *child);

src/definition/DefinitionWatcher.cpp

@@ -38,3 +38,7 @@ void DefinitionWatcher::startWatching(const DefinitionNode *root, const string &
         Logs::warning("Definition") << "Node not found for watching : " << path << endl;
     }
 }
+
+void DefinitionWatcher::startWatching(const DefinitionNode *node, bool init_diff) {
+    startWatching(node->getRoot(), node->getPath(), init_diff);
+}

src/definition/DefinitionWatcher.h

@@ -31,6 +31,13 @@ class DEFINITIONSHARED_EXPORT DefinitionWatcher {
      */
     void startWatching(const DefinitionNode *root, const string &path, bool init_diff = true);
 
+    /**
+     * Start watching a node.
+     *
+     * Overloaded for convenience.
+     */
+    void startWatching(const DefinitionNode *node, bool init_diff = true);
+
     /**
      * Abstract convenience to receive integer node changes.
      */

src/definition/DiffManager.cpp

@@ -1,42 +1,59 @@
 #include "DiffManager.h"
 
 #include <algorithm>
-#include <memory>
+#include <map>
+#include <vector>
 #include "DefinitionNode.h"
 #include "DefinitionDiff.h"
 #include "DefinitionWatcher.h"
 #include "Logs.h"
 
-DiffManager::DiffManager(DefinitionNode *tree) : tree(tree) {
-    undone = 0;
+class DiffManager::pimpl {
+  public:
+    pimpl(DefinitionNode *tree) : tree(tree), undone(0) {
+    }
+
+    DefinitionNode *tree;
+    unsigned long undone;
+    vector<const DefinitionDiff *> diffs;
+    map<const DefinitionNode *, vector<DefinitionWatcher *>> watchers;
+};
+
+DiffManager::DiffManager(DefinitionNode *tree) : impl(new pimpl(tree)) {
 }
 
 DiffManager::~DiffManager() {
-    for (auto diff : diffs) {
+    // TODO smart pointers
+    for (auto diff : impl->diffs) {
         delete diff;
     }
-    diffs.clear();
+    impl->diffs.clear();
+}
+
+unsigned long DiffManager::getDiffCount(int include_undone) const {
+    return include_undone ? impl->diffs.size() : impl->diffs.size() - impl->undone;
 }
 
 void DiffManager::addWatcher(const DefinitionNode *node, DefinitionWatcher *watcher) {
-    if (find(watchers[node].begin(), watchers[node].end(), watcher) == watchers[node].end()) {
-        watchers[node].push_back(watcher);
+    auto &watchers = impl->watchers[node];
+    if (find(watchers.begin(), watchers.end(), watcher) == watchers.end()) {
+        watchers.push_back(watcher);
     }
 }
 
-int DiffManager::getWatcherCount(const DefinitionNode *node) {
-    return watchers[node].size();
+unsigned long DiffManager::getWatcherCount(const DefinitionNode *node) {
+    return impl->watchers[node].size();
 }
 
 void DiffManager::addDiff(DefinitionNode *node, const DefinitionDiff *diff) {
-    while (undone > 0) {
+    while (impl->undone > 0) {
         // truncate diffs ahead
-        delete diffs.back();
-        diffs.pop_back();
-        undone--;
+        delete impl->diffs.back();
+        impl->diffs.pop_back();
+        impl->undone--;
     }
 
-    diffs.push_back(diff);
+    impl->diffs.push_back(diff);
 
     // TODO Delayed commit (with merge of consecutive diffs)
 
@@ -46,13 +63,13 @@ void DiffManager::addDiff(DefinitionNode *node, const DefinitionDiff *diff) {
 }
 
 void DiffManager::undo() {
-    if (undone < (int)diffs.size()) {
-        const DefinitionDiff *diff = diffs[diffs.size() - undone - 1];
+    if (impl->undone < impl->diffs.size()) {
+        const DefinitionDiff *diff = impl->diffs[impl->diffs.size() - impl->undone - 1];
 
         // Obtain the node by path and reverse apply diff on it
-        DefinitionNode *node = tree->findByPath(diff->getPath());
+        DefinitionNode *node = impl->tree->findByPath(diff->getPath());
         if (node) {
-            undone++;
+            impl->undone++;
 
             unique_ptr<DefinitionDiff> reversed(diff->newReversed());
 
@@ -67,13 +84,13 @@ void DiffManager::undo() {
 }
 
 void DiffManager::redo() {
-    if (undone > 0) {
-        const DefinitionDiff *diff = diffs[diffs.size() - undone];
+    if (impl->undone > 0) {
+        const DefinitionDiff *diff = impl->diffs[impl->diffs.size() - impl->undone];
 
         // Obtain the node by path and re-apply diff on it
-        DefinitionNode *node = tree->findByPath(diff->getPath());
+        DefinitionNode *node = impl->tree->findByPath(diff->getPath());
         if (node) {
-            undone--;
+            impl->undone--;
 
             Logs::debug("Definition") << "Node redo : " << node->getPath() << endl;
             node->applyDiff(diff);
@@ -88,7 +105,7 @@ void DiffManager::publishToWatchers(const DefinitionNode *node, const Definition
     // TODO Parent node signaling should be aggregated (to not receive many nodeChanged calls)
     const DefinitionNode *cnode = node;
     do {
-        for (auto watcher : watchers[cnode]) {
+        for (auto watcher : impl->watchers[cnode]) {
             watcher->nodeChanged(node, diff, cnode);
         }
         cnode = cnode->getParent();

src/definition/DiffManager.h

@@ -3,8 +3,7 @@
 
 #include "definition_global.h"
 
-#include <map>
-#include <vector>
+#include <memory>
 
 namespace paysages {
 namespace definition {
@@ -22,9 +21,7 @@ class DEFINITIONSHARED_EXPORT DiffManager {
     /**
      * Get the total number of diff stored.
      */
-    inline int getDiffCount(int include_undone = true) {
-        return include_undone ? diffs.size() : diffs.size() - undone;
-    }
+    unsigned long getDiffCount(int include_undone = true) const;
 
     /**
      * Add a watcher for a specific node.
@@ -36,7 +33,7 @@ class DEFINITIONSHARED_EXPORT DiffManager {
     /**
      * Get the number of watchers registered for a given node.
      */
-    int getWatcherCount(const DefinitionNode *node);
+    unsigned long getWatcherCount(const DefinitionNode *node);
 
     /**
      * Add a new diff of a node to the change flow.
@@ -63,10 +60,8 @@ class DEFINITIONSHARED_EXPORT DiffManager {
     void publishToWatchers(const DefinitionNode *node, const DefinitionDiff *diff);
 
   private:
-    DefinitionNode *tree;
-    int undone;
-    vector<const DefinitionDiff *> diffs;
-    map<const DefinitionNode *, vector<DefinitionWatcher *>> watchers;
+    class pimpl;
+    unique_ptr<pimpl> impl;
 };
 }
 }

src/definition/VegetationDefinition.cpp

@@ -29,9 +29,9 @@ void VegetationDefinition::applyPreset(VegetationPreset preset, RandomGenerator
 
     clear();
 
-    /*if (preset == VEGETATION_PRESET_TEMPERATE) {
+    if (preset == VEGETATION_PRESET_TEMPERATE) {
         layer.applyPreset(VegetationLayerDefinition::VEGETATION_BASIC_TREES, random);
         layer.setName("Basic tree");
-        addLayer(layer);
-    }*/
+        //addLayer(layer);
+    }
 }

src/render/opengl/OpenGLVegetation.cpp

@@ -118,7 +118,7 @@ void OpenGLVegetation::acquireLayers(vector<OpenGLVegetationLayer *> &layers) {
     layers_lock->release();
 }
 
-void OpenGLVegetation::releaseLayers(const vector<OpenGLVegetationLayer *> &layers) {
+void OpenGLVegetation::releaseLayers(const vector<OpenGLVegetationLayer *> &) {
     // TODO Reference count
 }
 

src/render/opengl/OpenGLVegetationImpostor.cpp

@@ -67,7 +67,8 @@ void OpenGLVegetationImpostor::render(OpenGLShaderProgram *program, const OpenGL
 }
 
 void OpenGLVegetationImpostor::prepareTexture(const VegetationModelDefinition &model, const Scenery &environment,
-                                              bool *interrupt) {
+                                              bool *) {
+    // TODO interrupt
     Scenery scenery;
     environment.getAtmosphere()->copy(scenery.getAtmosphere());
     SoftwareRenderer renderer(&scenery);

src/tests/DefinitionNode_Test.cpp

@@ -25,20 +25,20 @@ TEST(DefinitionNode, getPath) {
     EXPECT_EQ("/branch/leaf", leaf.getPath());
 }
 
-class FakeWatcher : public DefinitionWatcher {
+TEST(DefinitionNode, addWatcher) {
+    class FakeWatcher : public DefinitionWatcher {
         virtual void nodeChanged(const DefinitionNode *, const DefinitionDiff *, const DefinitionNode *) override {
         }
-};
+    };
 
-TEST(DefinitionNode, addWatcher) {
     DefinitionNode root(NULL, "root");
     FakeWatcher watcher;
 
-    EXPECT_EQ(0, root.getWatcherCount());
+    EXPECT_EQ(0u, root.getWatcherCount());
     root.addWatcher(&watcher);
-    EXPECT_EQ(1, root.getWatcherCount());
+    EXPECT_EQ(1u, root.getWatcherCount());
     root.addWatcher(&watcher);
-    EXPECT_EQ(1, root.getWatcherCount());
+    EXPECT_EQ(1u, root.getWatcherCount());
 }
 
 TEST(DefinitionNode, findByPath) {
@@ -65,15 +65,15 @@ TEST(DefinitionNode, attachDetach) {
 
     EXPECT_EQ(root, child1->getParent());
     EXPECT_EQ(root, child2->getParent());
-    EXPECT_EQ(2, root->getChildrenCount());
+    EXPECT_EQ(2u, root->getChildrenCount());
 
     delete child1;
 
-    EXPECT_EQ(1, root->getChildrenCount());
+    EXPECT_EQ(1u, root->getChildrenCount());
 
     delete child2;
 
-    EXPECT_EQ(0, root->getChildrenCount());
+    EXPECT_EQ(0u, root->getChildrenCount());
 
     delete root;
 }

src/tests/Layers_Test.cpp

@@ -127,25 +127,25 @@ TEST(Layers, undoRedo) {
     Layers layers(NULL, "layers", _construc3);
 
     EXPECT_EQ(0, layers.getLayerCount());
-    EXPECT_EQ(0, layers.getDiffManager()->getDiffCount());
+    EXPECT_EQ(0u, layers.getDiffManager()->getDiffCount());
 
     layers.addLayer("testlayer");
 
     ASSERT_EQ(1, layers.getLayerCount());
 
-    TestLayer *layer = (TestLayer *)layers.getLayer(0);
+    auto layer = static_cast<TestLayer *>(layers.getLayer(0));
 
-    EXPECT_EQ(1, layers.getDiffManager()->getDiffCount());
+    EXPECT_EQ(1u, layers.getDiffManager()->getDiffCount());
     checkLayerChild(layers, 0, "val", 5);
 
     layer->val->setValue(8);
 
-    EXPECT_EQ(2, layers.getDiffManager()->getDiffCount());
+    EXPECT_EQ(2u, layers.getDiffManager()->getDiffCount());
     checkLayerChild(layers, 0, "val", 8);
 
     layers.removeLayer(0);
 
-    EXPECT_EQ(3, layers.getDiffManager()->getDiffCount());
+    EXPECT_EQ(3u, layers.getDiffManager()->getDiffCount());
     EXPECT_EQ(0, layers.getLayerCount());
 
     // Start undoing
@@ -156,7 +156,7 @@ TEST(Layers, undoRedo) {
     layers.getDiffManager()->undo();
     EXPECT_EQ(0, layers.getLayerCount());
 
-    EXPECT_EQ(3, layers.getDiffManager()->getDiffCount());
+    EXPECT_EQ(3u, layers.getDiffManager()->getDiffCount());
 
     // Start redoing
     layers.getDiffManager()->redo();
@@ -166,7 +166,7 @@ TEST(Layers, undoRedo) {
     layers.getDiffManager()->redo();
     EXPECT_EQ(0, layers.getLayerCount());
 
-    EXPECT_EQ(3, layers.getDiffManager()->getDiffCount());
+    EXPECT_EQ(3u, layers.getDiffManager()->getDiffCount());
 
     // TODO Test swapping
 }