Merge pull request Revolutionary-Games#23 from Revolutionary-Games/sh…

…ared_data_has_changes

SharedData::hasChanges()

src/engine/shared_data.h

@@ -108,6 +108,14 @@ namespace thrive {
  * obj.m_sharedProperties.touch();
  * \endcode
  *
+ * @subsection touch() and untouch()
+ *
+ * When the writing thread has modified the data, it should call 
+ * SharedData::touch() to mark the buffer as changed. The reading thread can
+ * call SharedData::hasChanges() to query whether the current stable buffer
+ * has any changes. Once it has processed these changes, it should call
+ * SharedData::untouch() to mark them as such.
+ *
  * @section shared_data_lua SharedData in Lua
  *
  * How you can access shared data from Lua depends on whether the script
@@ -139,8 +147,6 @@ namespace thrive {
  * 
  */
 
-using DataVersion = unsigned int;
-
 /**
 * @brief Enumeration naming the three state buffers
 */
@@ -533,6 +539,18 @@ class SharedData : public detail::SharedDataBase<Writer, Reader> {
     */
     SharedData& operator= (const SharedData&) = delete;
 
+    /**
+    * @brief Whether the stable buffer is outdated
+    *
+    * @return 
+    *   \c true if there have been changes to the current stable buffer since 
+    *   the last call to untouch(), \c false otherwise
+    */
+    bool
+    hasChanges() const {
+        return m_lastUntouch < this->getLastBufferChange(StateBuffer::Stable);
+    }
+
     /**
     * @brief Returns the latest data buffer
     */
@@ -550,21 +568,26 @@ class SharedData : public detail::SharedDataBase<Writer, Reader> {
     }
 
     /**
-    * @brief Returns the last frame the stable buffer was changed
+    * @brief Marks the working copy as changed
+    *
+    * Only the writing thread should call this
     */
-    DataVersion
-    stableVersion() const {
+    void
+    touch() {
         State& state = State::instance();
-        short bufferIndex = state.getBufferIndex(StateBuffer::Stable);
-        return m_bufferVersions[bufferIndex];
+        // +1 because we are currently rendering the *next* frame
+        m_lastTouch = state.getBufferFrame(StateBuffer::WorkingCopy) + 1;
+        this->setLastBufferChange(StateBuffer::WorkingCopy, m_lastTouch);
     }
 
     /**
-    * @brief Marks the working copy as changed
+    * @brief Resets the hasChanges() flag
+    *
+    * Only the reading thread should call this.
     */
     void
-    touch() {
-        m_touchedVersion += 1;
+    untouch() {
+        m_lastUntouch = this->getLastBufferChange(StateBuffer::Stable);
     }
 
     /**
@@ -577,13 +600,12 @@ class SharedData : public detail::SharedDataBase<Writer, Reader> {
     updateBuffer(
         short bufferIndex
     ) override {
-        if (m_bufferVersions[bufferIndex] < m_touchedVersion) {
+        if (m_lastBufferChanges[bufferIndex] < m_lastTouch) {
             m_buffers[bufferIndex] = this->latest();
-            m_bufferVersions[bufferIndex] = m_touchedVersion;
+            m_lastBufferChanges[bufferIndex] = m_lastTouch;
         }
     }
 
-
     /**
     * @brief Returns the working copy data buffer
     */
@@ -612,11 +634,33 @@ class SharedData : public detail::SharedDataBase<Writer, Reader> {
         return m_buffers[bufferIndex];
     }
 
-    DataVersion m_touchedVersion = 0;
+    FrameIndex
+    getLastBufferChange(
+        StateBuffer buffer
+    ) const {
+        State& state = State::instance();
+        short bufferIndex = state.getBufferIndex(buffer);
+        return m_lastBufferChanges[bufferIndex];
+    }
+
+    void
+    setLastBufferChange(
+        StateBuffer buffer,
+        FrameIndex lastChange
+    ) {
+        State& state = State::instance();
+        short bufferIndex = state.getBufferIndex(buffer);
+        m_lastBufferChanges[bufferIndex] = lastChange;
+    }
 
     std::array<Data, 3> m_buffers;
 
-    std::array<DataVersion, 3> m_bufferVersions = {{0, 0, 0}};
+    std::array<FrameIndex, 3> m_lastBufferChanges = {{0, 0, 0}};
+
+    FrameIndex m_lastTouch = 0;
+
+    FrameIndex m_lastUntouch = 0;
+
 };
 
 

src/engine/tests/shared_data.cpp

@@ -123,6 +123,78 @@ TEST(SharedData, UpdateWorkingCopy) {
 }
 
 
+TEST(SharedData, Untouch) {
+    State& state = State::instance();
+    state.reset();
+    RenderData<int> data(1);
+    // Touch
+    state.lockWorkingCopy();
+    data.touch();
+    state.releaseWorkingCopy();
+    // Lock stable, should have changes
+    state.lockStable();
+    EXPECT_TRUE(data.hasChanges());
+    state.releaseStable();
+    // Relock stable, should still have changes
+    state.lockStable();
+    EXPECT_TRUE(data.hasChanges());
+    // Untouch, no changes anymore
+    data.untouch();
+    EXPECT_FALSE(data.hasChanges());
+    state.releaseStable();
+    // Relocking, still no changes
+    state.lockStable();
+    EXPECT_FALSE(data.hasChanges());
+    state.releaseStable();
+    state.reset();
+}
+
+
+TEST(SharedData, TouchDuringUntouch) {
+    State& state = State::instance();
+    state.reset();
+    RenderData<int> data(1);
+    // Touch
+    state.lockWorkingCopy();
+    data.touch();
+    state.releaseWorkingCopy();
+    // Untouch (no release)
+    state.lockStable();
+    data.untouch();
+    EXPECT_FALSE(data.hasChanges());
+    // Touch again
+    state.lockWorkingCopy();
+    data.touch();
+    state.releaseWorkingCopy();
+    // Should still have no changes
+    EXPECT_FALSE(data.hasChanges());
+    state.releaseStable();
+    state.reset();
+}
+
+
+TEST(SharedData, UntouchDuringTouch) {
+    State& state = State::instance();
+    state.reset();
+    RenderData<int> data(1);
+    // Touch (no release)
+    state.lockWorkingCopy();
+    data.touch();
+    // Untouch
+    state.lockStable();
+    EXPECT_FALSE(data.hasChanges());
+    data.untouch();
+    state.releaseStable();
+    // Release working copy
+    state.releaseWorkingCopy();
+    // Now it should have changes
+    state.lockStable();
+    EXPECT_TRUE(data.hasChanges());
+    state.releaseStable();
+    state.reset();
+}
+
+
 TEST(SharedData, DataTransfer) {
     State& state = State::instance();
     state.reset();

src/ogre/light_system.cpp

@@ -147,6 +147,9 @@ OgreLightSystem::update(int) {
     }
     for (const auto& value : m_impl->m_entities) {
         OgreLightComponent* lightComponent = std::get<0>(value.second);
+        if (not lightComponent->m_properties.hasChanges()) {
+            continue;
+        }
         Ogre::Light* light = lightComponent->m_light;
         const auto& properties = lightComponent->m_properties.stable();
         light->setType(properties.type);
@@ -164,6 +167,7 @@ OgreLightSystem::update(int) {
             properties.spotlightFalloff
         );
         light->setSpotlightNearClipDistance(properties.spotlightNearClipDistance);
+        lightComponent->m_properties.untouch();
     }
     for (EntityId entityId : m_impl->m_entities.removedEntities()) {
         Ogre::Light* light = m_impl->m_lights[entityId];

src/ogre/scene_node_system.cpp

@@ -185,17 +185,21 @@ OgreUpdateSceneNodeSystem::shutdown() {
 void
 OgreUpdateSceneNodeSystem::update(int) {
     for (const auto& entry : m_impl->m_entities) {
-        OgreSceneNodeComponent* sceneNodeComponent = std::get<0>(entry.second);
         TransformComponent* transformComponent = std::get<1>(entry.second);
-        sceneNodeComponent->m_sceneNode->setOrientation(
-            transformComponent->m_properties.stable().orientation
-        );
-        sceneNodeComponent->m_sceneNode->setPosition(
-            transformComponent->m_properties.stable().position
-        );
-        sceneNodeComponent->m_sceneNode->setScale(
-            transformComponent->m_properties.stable().scale
-        );
+        if (transformComponent->m_properties.hasChanges()) {
+            Ogre::SceneNode* sceneNode = std::get<0>(entry.second)->m_sceneNode;
+            const auto& transformProperties = transformComponent->m_properties.stable();
+            sceneNode->setOrientation(
+                transformProperties.orientation
+            );
+            sceneNode->setPosition(
+                transformProperties.position
+            );
+            sceneNode->setScale(
+                transformProperties.scale
+            );
+            transformComponent->m_properties.untouch();
+        }
     }
 }
 

src/ogre/sky_system.cpp

@@ -118,7 +118,7 @@ void
 SkySystem::update(int) {
     for (auto& value : m_impl->m_skyEntities) {
         SkyPlaneComponent* plane = std::get<0>(value.second);
-        if (plane) {
+        if (plane and plane->m_properties.hasChanges()) {
             const SkyPlaneComponent::Properties& properties = plane->m_properties.stable();
             m_impl->m_sceneManager->setSkyPlane(
                 properties.enabled,
@@ -132,6 +132,7 @@ SkySystem::update(int) {
                 properties.ysegments,
                 properties.groupName
             );
+            plane->m_properties.untouch();
         }
     }
 }