diff --git a/src/definition/DefinitionNode.cpp b/src/definition/DefinitionNode.cpp index 9819b08..2b6855c 100644 --- a/src/definition/DefinitionNode.cpp +++ b/src/definition/DefinitionNode.cpp @@ -9,6 +9,20 @@ #include #include +// Diff for abstract nodes +class DefinitionNodeDiff : public DefinitionDiff { + public: + DefinitionNodeDiff(const DefinitionNode *node) : DefinitionDiff(node) { + } + + DefinitionNodeDiff(const DefinitionDiff *diff) : DefinitionDiff(diff) { + } + + virtual DefinitionDiff *newReversed() const override { + return new DefinitionNodeDiff(this); + } +}; + DefinitionNode::DefinitionNode(DefinitionNode *parent, const string &name, const string &type_name) : parent(parent), type_name(type_name), name(name) { if (parent) { @@ -115,7 +129,9 @@ bool DefinitionNode::applyDiff(const DefinitionDiff *diff, bool) { } } -void DefinitionNode::generateInitDiffs(vector *) const { +void DefinitionNode::generateInitDiffs(vector *diffs) const { + diffs->push_back(new DefinitionNodeDiff(this)); + // TODO add children diffs in cascade ? } void DefinitionNode::addWatcher(DefinitionWatcher *watcher, bool init_diff) { @@ -125,7 +141,7 @@ void DefinitionNode::addWatcher(DefinitionWatcher *watcher, bool init_diff) { generateInitDiffs(&diffs); for (auto diff : diffs) { - watcher->nodeChanged(this, diff); + watcher->nodeChanged(this, diff, this); delete diff; } } diff --git a/src/definition/DefinitionWatcher.cpp b/src/definition/DefinitionWatcher.cpp index c94c233..37f9379 100644 --- a/src/definition/DefinitionWatcher.cpp +++ b/src/definition/DefinitionWatcher.cpp @@ -9,9 +9,10 @@ DefinitionWatcher::DefinitionWatcher() { } DefinitionWatcher::~DefinitionWatcher() { + // FIXME watcher is not removed from the diff manager ! } -void DefinitionWatcher::nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) { +void DefinitionWatcher::nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *) { string type_name = node->getTypeName(); if (type_name == "int") { diff --git a/src/definition/DefinitionWatcher.h b/src/definition/DefinitionWatcher.h index 784c2f9..bbb32ca 100644 --- a/src/definition/DefinitionWatcher.h +++ b/src/definition/DefinitionWatcher.h @@ -20,8 +20,10 @@ class DEFINITIONSHARED_EXPORT DefinitionWatcher { /** * Abstract method called when a node changed. + * + * *parent* is the node that is watched (useful if *node* is a sub-node). */ - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff); + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent); protected: /** diff --git a/src/definition/DiffManager.cpp b/src/definition/DiffManager.cpp index 8bfc857..a88af0b 100644 --- a/src/definition/DiffManager.cpp +++ b/src/definition/DiffManager.cpp @@ -39,11 +39,10 @@ void DiffManager::addDiff(DefinitionNode *node, const DefinitionDiff *diff) { diffs.push_back(diff); // TODO Delayed commit (with merge of consecutive diffs) - node->applyDiff(diff); - for (auto watcher : watchers[node]) { - watcher->nodeChanged(node, diff); - } + Logs::debug("Definition") << "Node changed : " << node->getPath() << endl; + node->applyDiff(diff); + publishToWatchers(node, diff); } void DiffManager::undo() { @@ -55,12 +54,12 @@ void DiffManager::undo() { if (node) { undone++; - node->applyDiff(diff, true); + unique_ptr reversed(diff->newReversed()); - for (auto watcher : watchers[node]) { - unique_ptr reversed(diff->newReversed()); - watcher->nodeChanged(node, reversed.get()); - } + Logs::debug("Definition") << "Node undo : " << node->getPath() << endl; + // TODO use reversed ? + node->applyDiff(diff, true); + publishToWatchers(node, reversed.get()); } else { Logs::error("Definition") << "Can't find node to undo diff : " << diff->getPath() << endl; } @@ -76,13 +75,22 @@ void DiffManager::redo() { if (node) { undone--; + Logs::debug("Definition") << "Node redo : " << node->getPath() << endl; node->applyDiff(diff); - - for (auto watcher : watchers[node]) { - watcher->nodeChanged(node, diff); - } + publishToWatchers(node, diff); } else { Logs::error("Definition") << "Can't find node to redo diff : " << diff->getPath() << endl; } } } + +void DiffManager::publishToWatchers(const DefinitionNode *node, const DefinitionDiff *diff) { + // TODO Parent node signaling should be aggregated (to not receive many nodeChanged calls) + const DefinitionNode *cnode = node; + do { + for (auto watcher : watchers[cnode]) { + watcher->nodeChanged(node, diff, cnode); + } + cnode = cnode->getParent(); + } while (cnode); +} diff --git a/src/definition/DiffManager.h b/src/definition/DiffManager.h index d1acbf9..b64c6f7 100644 --- a/src/definition/DiffManager.h +++ b/src/definition/DiffManager.h @@ -53,6 +53,15 @@ class DEFINITIONSHARED_EXPORT DiffManager { */ void redo(); +protected: + /** + * Publish a diff to the registered watchers. + * + * The diff will be published to the watcher of the node, and to the watchers of all + * parents, up to the definition tree root. + */ + void publishToWatchers(const DefinitionNode *node, const DefinitionDiff *diff); + private: DefinitionNode *tree; int undone; diff --git a/src/definition/WaterDefinition.cpp b/src/definition/WaterDefinition.cpp index 5ba9723..ac6eb6c 100644 --- a/src/definition/WaterDefinition.cpp +++ b/src/definition/WaterDefinition.cpp @@ -109,7 +109,7 @@ void WaterDefinition::validate() { foam_material->validate(); } -void WaterDefinition::nodeChanged(const DefinitionNode *node, const DefinitionDiff *) { +void WaterDefinition::nodeChanged(const DefinitionNode *node, const DefinitionDiff *, const DefinitionNode *) { if (node == model) { switch (model->getValue()) { case 1: diff --git a/src/definition/WaterDefinition.h b/src/definition/WaterDefinition.h index edfec38..62514f8 100644 --- a/src/definition/WaterDefinition.h +++ b/src/definition/WaterDefinition.h @@ -36,7 +36,7 @@ class DEFINITIONSHARED_EXPORT WaterDefinition : public DefinitionNode, public De return depth_color; } - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff); + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; typedef enum { WATER_PRESET_LAKE, WATER_PRESET_SEA } WaterPreset; void applyPreset(WaterPreset preset, RandomGenerator &random = RandomGeneratorDefault); diff --git a/src/interface/modeler/AtmosphereModeler.cpp b/src/interface/modeler/AtmosphereModeler.cpp index 4412dd9..6dd4a2e 100644 --- a/src/interface/modeler/AtmosphereModeler.cpp +++ b/src/interface/modeler/AtmosphereModeler.cpp @@ -18,7 +18,7 @@ AtmosphereModeler::AtmosphereModeler(MainModelerWindow *ui) : BaseModelerTool(ui ui->setQmlProperty("atmosphere_daytime", "value", ui->getScenery()->getAtmosphere()->getDaytime()); } -void AtmosphereModeler::nodeChanged(const DefinitionNode *node, const DefinitionDiff *) { +void AtmosphereModeler::nodeChanged(const DefinitionNode *node, const DefinitionDiff *, const DefinitionNode *) { if (node->getPath().find("/atmosphere/sun/") == 0) { getWindow()->getCamera()->startSunTool(1000); } diff --git a/src/interface/modeler/AtmosphereModeler.h b/src/interface/modeler/AtmosphereModeler.h index 7527876..659aaf3 100644 --- a/src/interface/modeler/AtmosphereModeler.h +++ b/src/interface/modeler/AtmosphereModeler.h @@ -12,7 +12,7 @@ class AtmosphereModeler : protected BaseModelerTool { public: AtmosphereModeler(MainModelerWindow *ui); - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; }; } } diff --git a/src/interface/modeler/FloatPropertyBind.cpp b/src/interface/modeler/FloatPropertyBind.cpp index 2b411f2..8805a69 100644 --- a/src/interface/modeler/FloatPropertyBind.cpp +++ b/src/interface/modeler/FloatPropertyBind.cpp @@ -20,7 +20,7 @@ FloatPropertyBind::FloatPropertyBind(MainModelerWindow *window, const string &ob } } -void FloatPropertyBind::nodeChanged(const DefinitionNode *, const DefinitionDiff *) { +void FloatPropertyBind::nodeChanged(const DefinitionNode *, const DefinitionDiff *, const DefinitionNode *) { if (item) { item->setProperty(property.c_str(), node->getValue()); } diff --git a/src/interface/modeler/FloatPropertyBind.h b/src/interface/modeler/FloatPropertyBind.h index e1e1f23..86d5cc0 100644 --- a/src/interface/modeler/FloatPropertyBind.h +++ b/src/interface/modeler/FloatPropertyBind.h @@ -20,7 +20,7 @@ class FloatPropertyBind : public QObject, public DefinitionWatcher { FloatPropertyBind(MainModelerWindow *window, const string &object_name, const string &property_name, FloatNode *node); - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; private slots: void propertyChanged(); diff --git a/src/interface/modeler/IntPropertyBind.cpp b/src/interface/modeler/IntPropertyBind.cpp index b7b2cb4..c8d95bb 100644 --- a/src/interface/modeler/IntPropertyBind.cpp +++ b/src/interface/modeler/IntPropertyBind.cpp @@ -18,7 +18,7 @@ IntPropertyBind::IntPropertyBind(MainModelerWindow *window, const string &object } } -void IntPropertyBind::nodeChanged(const DefinitionNode *, const DefinitionDiff *) { +void IntPropertyBind::nodeChanged(const DefinitionNode *, const DefinitionDiff *, const DefinitionNode *) { if (item) { item->setProperty(property.c_str(), node->getValue()); } diff --git a/src/interface/modeler/IntPropertyBind.h b/src/interface/modeler/IntPropertyBind.h index 3f89c5e..8588e6b 100644 --- a/src/interface/modeler/IntPropertyBind.h +++ b/src/interface/modeler/IntPropertyBind.h @@ -19,7 +19,7 @@ class IntPropertyBind : public QObject, public DefinitionWatcher { public: IntPropertyBind(MainModelerWindow *window, const string &object_name, const string &property_name, IntNode *node); - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; private slots: void propertyChanged(); diff --git a/src/interface/modeler/ModelerCameras.cpp b/src/interface/modeler/ModelerCameras.cpp index 25c8078..c80935c 100644 --- a/src/interface/modeler/ModelerCameras.cpp +++ b/src/interface/modeler/ModelerCameras.cpp @@ -89,7 +89,7 @@ void ModelerCameras::timerEvent(QTimerEvent *) { } } -void ModelerCameras::nodeChanged(const DefinitionNode *node, const DefinitionDiff *) { +void ModelerCameras::nodeChanged(const DefinitionNode *node, const DefinitionDiff *, const DefinitionNode *) { if (node->getPath().find("/atmosphere/sun/") == 0 and tool_mode == TOOL_SUN) { tool->setTarget(parent->getRenderer()->getAtmosphereRenderer()->getSunLocation()); } diff --git a/src/interface/modeler/ModelerCameras.h b/src/interface/modeler/ModelerCameras.h index 3ac78b1..65f49e3 100644 --- a/src/interface/modeler/ModelerCameras.h +++ b/src/interface/modeler/ModelerCameras.h @@ -50,7 +50,7 @@ class ModelerCameras : public QObject, public DefinitionWatcher { protected: virtual void timerEvent(QTimerEvent *event) override; - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; /** * Validate current camera, pushing it to rendered scenery if needed. diff --git a/src/render/opengl/OpenGLSkybox.cpp b/src/render/opengl/OpenGLSkybox.cpp index a17a822..f55bf28 100644 --- a/src/render/opengl/OpenGLSkybox.cpp +++ b/src/render/opengl/OpenGLSkybox.cpp @@ -78,7 +78,7 @@ void OpenGLSkybox::render() { program->draw(vertices); } -void OpenGLSkybox::nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) { +void OpenGLSkybox::nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) { OpenGLSharedState *state = renderer->getSharedState(); AtmosphereDefinition *newdef = renderer->getScenery()->getAtmosphere(); @@ -92,7 +92,7 @@ void OpenGLSkybox::nodeChanged(const DefinitionNode *node, const DefinitionDiff state->set("dayTime", newdef->getDaytime()); } - DefinitionWatcher::nodeChanged(node, diff); + DefinitionWatcher::nodeChanged(node, diff, parent); } void OpenGLSkybox::floatNodeChanged(const string &path, double new_value, double) { diff --git a/src/render/opengl/OpenGLSkybox.h b/src/render/opengl/OpenGLSkybox.h index 8e7afe0..2714175 100644 --- a/src/render/opengl/OpenGLSkybox.h +++ b/src/render/opengl/OpenGLSkybox.h @@ -18,7 +18,7 @@ class OPENGLSHARED_EXPORT OpenGLSkybox : public OpenGLPart, public DefinitionWat virtual void update() override; virtual void render() override; - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; virtual void floatNodeChanged(const string &path, double new_value, double old_value) override; private: diff --git a/src/render/opengl/OpenGLTerrain.cpp b/src/render/opengl/OpenGLTerrain.cpp index 7c2da82..cd9abfe 100644 --- a/src/render/opengl/OpenGLTerrain.cpp +++ b/src/render/opengl/OpenGLTerrain.cpp @@ -165,7 +165,7 @@ void OpenGLTerrain::performChunksMaintenance() { } } -void OpenGLTerrain::nodeChanged(const DefinitionNode *node, const DefinitionDiff *) { +void OpenGLTerrain::nodeChanged(const DefinitionNode *node, const DefinitionDiff *, const DefinitionNode *) { if (node->getPath() == "/terrain/water_height") { resetTextures(); } else if (node->getPath().find("/atmosphere") == 0) { diff --git a/src/render/opengl/OpenGLTerrain.h b/src/render/opengl/OpenGLTerrain.h index 04ce9ab..d324e8f 100644 --- a/src/render/opengl/OpenGLTerrain.h +++ b/src/render/opengl/OpenGLTerrain.h @@ -35,7 +35,7 @@ class OPENGLSHARED_EXPORT OpenGLTerrain : public OpenGLPart, public DefinitionWa void performChunksMaintenance(); - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *) override; private: OpenGLShaderProgram *program; diff --git a/src/render/opengl/OpenGLVegetation.cpp b/src/render/opengl/OpenGLVegetation.cpp index e59d50f..f5b1b46 100644 --- a/src/render/opengl/OpenGLVegetation.cpp +++ b/src/render/opengl/OpenGLVegetation.cpp @@ -88,7 +88,7 @@ void OpenGLVegetation::render() { } } -void OpenGLVegetation::nodeChanged(const DefinitionNode *node, const DefinitionDiff *) { +void OpenGLVegetation::nodeChanged(const DefinitionNode *node, const DefinitionDiff *, const DefinitionNode *) { if (node->getPath() == "/vegetation") { updateLayers(); } diff --git a/src/render/opengl/OpenGLVegetation.h b/src/render/opengl/OpenGLVegetation.h index 5aa3002..b71d60a 100644 --- a/src/render/opengl/OpenGLVegetation.h +++ b/src/render/opengl/OpenGLVegetation.h @@ -32,7 +32,7 @@ class OPENGLSHARED_EXPORT OpenGLVegetation : public OpenGLPart, public Definitio virtual void update() override; virtual void render() override; - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; /** * Get the currently rendered scenery. diff --git a/src/render/opengl/OpenGLWater.cpp b/src/render/opengl/OpenGLWater.cpp index da87443..6419f0d 100644 --- a/src/render/opengl/OpenGLWater.cpp +++ b/src/render/opengl/OpenGLWater.cpp @@ -66,7 +66,7 @@ void OpenGLWater::render() { } } -void OpenGLWater::nodeChanged(const DefinitionNode *node, const DefinitionDiff *) { +void OpenGLWater::nodeChanged(const DefinitionNode *node, const DefinitionDiff *, const DefinitionNode *) { OpenGLSharedState *state = renderer->getSharedState(); if (node->getPath() == path_height) { diff --git a/src/render/opengl/OpenGLWater.h b/src/render/opengl/OpenGLWater.h index 6c9a1bf..57bb3f4 100644 --- a/src/render/opengl/OpenGLWater.h +++ b/src/render/opengl/OpenGLWater.h @@ -18,7 +18,7 @@ class OPENGLSHARED_EXPORT OpenGLWater : public OpenGLPart, public DefinitionWatc virtual void update() override; virtual void render() override; - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; /** * Enable or disable the water surface rendering. diff --git a/src/render/software/MoonRenderer.cpp b/src/render/software/MoonRenderer.cpp index 21e8c1e..aa66188 100644 --- a/src/render/software/MoonRenderer.cpp +++ b/src/render/software/MoonRenderer.cpp @@ -18,17 +18,13 @@ class MoonRenderer::pimpl { MoonRenderer::MoonRenderer(CelestialBodyDefinition *moon_node) : impl(new pimpl()) { startWatching(moon_node->getRoot(), moon_node->getPath()); - - // FIXME should not be needed because the above watcher should watch the whole node - // and call nodeChanged - moon_node->copy(&impl->definition); } MoonRenderer::~MoonRenderer() { } -void MoonRenderer::nodeChanged(const DefinitionNode *node, const DefinitionDiff *) { - if (auto moon_node = static_cast(node)) { +void MoonRenderer::nodeChanged(const DefinitionNode *, const DefinitionDiff *, const DefinitionNode *parent) { + if (auto moon_node = static_cast(parent)) { moon_node->copy(&impl->definition); } } diff --git a/src/render/software/MoonRenderer.h b/src/render/software/MoonRenderer.h index 3bcbf99..3cd0aac 100644 --- a/src/render/software/MoonRenderer.h +++ b/src/render/software/MoonRenderer.h @@ -19,7 +19,7 @@ class SOFTWARESHARED_EXPORT MoonRenderer : public DefinitionWatcher, public Ligh MoonRenderer(CelestialBodyDefinition *moon_node); virtual ~MoonRenderer(); - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override; + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override; virtual bool getLightsAt(vector &result, const Vector3 &location) const override; /** diff --git a/src/tests/DefinitionNode_Test.cpp b/src/tests/DefinitionNode_Test.cpp index f970d24..30d6c3a 100644 --- a/src/tests/DefinitionNode_Test.cpp +++ b/src/tests/DefinitionNode_Test.cpp @@ -26,7 +26,7 @@ TEST(DefinitionNode, getPath) { } class FakeWatcher : public DefinitionWatcher { - virtual void nodeChanged(const DefinitionNode *, const DefinitionDiff *) override { + virtual void nodeChanged(const DefinitionNode *, const DefinitionDiff *, const DefinitionNode *) override { } }; diff --git a/src/tests/DiffManager_Test.cpp b/src/tests/DiffManager_Test.cpp index 4fd135a..ffb80dc 100644 --- a/src/tests/DiffManager_Test.cpp +++ b/src/tests/DiffManager_Test.cpp @@ -1,5 +1,6 @@ #include "BaseTestCase.h" +#include "TestToolDefinition.h" #include "DiffManager.h" #include "DefinitionNode.h" #include "DefinitionWatcher.h" @@ -118,7 +119,7 @@ class TestWatcher : public DefinitionWatcher { calls = 0; } - virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff) override { + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *) override { EXPECT_EQ(expected_node, node); ASSERT_EQ("float", diff->getTypeName()); const FloatDiff *float_diff = (const FloatDiff *)diff; @@ -154,3 +155,28 @@ TEST(DiffManager, addWatcherWithInitDiffs) { node.addWatcher(&watcher, true); EXPECT_EQ(1, watcher.calls); } + +TEST(DiffManager, publishToWatcher) { + DefinitionNode root(NULL, "root"); + FloatNode node(&root, "node", 1.3); + RecordingDefinitionWatcher watcher; + + EXPECT_EQ(0u, watcher.changes.size()); + + root.addWatcher(&watcher, true); + ASSERT_EQ(1u, watcher.changes.size()); + EXPECT_EQ(&root, watcher.changes[0].node); + EXPECT_EQ(&root, watcher.changes[0].parent); + + node.addWatcher(&watcher, true); + ASSERT_EQ(2u, watcher.changes.size()); + EXPECT_EQ(&node, watcher.changes[1].node); + EXPECT_EQ(&node, watcher.changes[1].parent); + + node.setValue(2.3); + ASSERT_EQ(4u, watcher.changes.size()); + EXPECT_EQ(&node, watcher.changes[2].node); + EXPECT_EQ(&node, watcher.changes[2].parent); + EXPECT_EQ(&node, watcher.changes[3].node); + EXPECT_EQ(&root, watcher.changes[3].parent); +} diff --git a/src/tests/TestToolDefinition.h b/src/tests/TestToolDefinition.h new file mode 100644 index 0000000..eac73f0 --- /dev/null +++ b/src/tests/TestToolDefinition.h @@ -0,0 +1,35 @@ +#ifndef TESTTOOLDEFINITION_H +#define TESTTOOLDEFINITION_H + +#include "DefinitionWatcher.h" + +#include + +namespace { +/** + * Definition watcher that registers all calls to nodeChanged. + */ +class RecordingDefinitionWatcher : public DefinitionWatcher { + public: + RecordingDefinitionWatcher() { + } + + virtual void nodeChanged(const DefinitionNode *node, const DefinitionDiff *diff, const DefinitionNode *parent) override { + RecordedChange change; + change.node = node; + change.diff = diff; // FIXME Referenced diff may get deleted by the diff manager + change.parent = parent; + changes.push_back(change); + } + + typedef struct { + const DefinitionNode *node; + const DefinitionDiff *diff; + const DefinitionNode *parent; + } RecordedChange; + vector changes; +}; +} + + +#endif // TESTTOOLDEFINITION_H