diff --git a/src/engine/shared_data.h b/src/engine/shared_data.h index cbd81720757..983cc01d93a 100644 --- a/src/engine/shared_data.h +++ b/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 { */ 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 { } /** - * @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 { 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 { 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 m_buffers; - std::array m_bufferVersions = {{0, 0, 0}}; + std::array m_lastBufferChanges = {{0, 0, 0}}; + + FrameIndex m_lastTouch = 0; + + FrameIndex m_lastUntouch = 0; + }; diff --git a/src/engine/tests/shared_data.cpp b/src/engine/tests/shared_data.cpp index 4edb4e3d90b..e0793773c20 100644 --- a/src/engine/tests/shared_data.cpp +++ b/src/engine/tests/shared_data.cpp @@ -123,6 +123,78 @@ TEST(SharedData, UpdateWorkingCopy) { } +TEST(SharedData, Untouch) { + State& state = State::instance(); + state.reset(); + RenderData 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 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 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(); diff --git a/src/ogre/light_system.cpp b/src/ogre/light_system.cpp index 07d7b74f591..e69e1096f8b 100644 --- a/src/ogre/light_system.cpp +++ b/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]; diff --git a/src/ogre/scene_node_system.cpp b/src/ogre/scene_node_system.cpp index 9c557304797..a5caad22b88 100644 --- a/src/ogre/scene_node_system.cpp +++ b/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(); + } } } diff --git a/src/ogre/sky_system.cpp b/src/ogre/sky_system.cpp index d7a4110c045..d7713b43a95 100644 --- a/src/ogre/sky_system.cpp +++ b/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(); } } }