renderer: Make fetching from render entity more reliable.

Author: heinezen

libopenage/curve/keyframe_container.h

@@ -607,18 +607,18 @@ template <typename T>
 typename KeyframeContainer<T>::elem_ptr
 KeyframeContainer<T>::sync(const KeyframeContainer<T> &other,
                            const time::time_t &start) {
-	// Delete elements after start time
+	// Delete elements from this container after start time
 	elem_ptr at = this->last_before(start, this->container.size());
 	at = this->erase_after(at);
 
-	auto at_other = 1; // always skip the first element (because it's the default value)
+	// Find the last element before the start time in the other container
+	elem_ptr at_other = other.last_before(start, other.size());
+	++at_other; // move one element forward so that at_other.time() >= start
 
 	// Copy all elements from other with time >= start
-	for (size_t i = at_other; i < other.size(); i++) {
-		if (other.get(i).time() >= start) {
-			at = this->insert_after(other.get(i), at);
-		}
-	}
+	this->container.insert(this->container.end(),
+	                       other.container.begin() + at_other,
+	                       other.container.end());
 
 	return this->container.size();
 }
@@ -634,13 +634,14 @@ KeyframeContainer<T>::sync(const KeyframeContainer<O> &other,
 	elem_ptr at = this->last_before(start, this->container.size());
 	at = this->erase_after(at);
 
-	auto at_other = 1; // always skip the first element (because it's the default value)
+	// Find the last element before the start time in the other container
+	elem_ptr at_other = other.last_before(start, other.size());
+	++at_other; // move one element forward so that at_other.time() >= start
 
 	// Copy all elements from other with time >= start
 	for (size_t i = at_other; i < other.size(); i++) {
-		if (other.get(i).time() >= start) {
-			at = this->insert_after(keyframe_t(other.get(i).time(), converter(other.get(i).val())), at);
-		}
+		auto &elem = other.get(i);
+		this->container.emplace_back(elem.time(), converter(elem.val()));
 	}
 
 	return this->container.size();

libopenage/gamestate/system/idle.cpp

@@ -1,4 +1,4 @@
-// Copyright 2023-2023 the openage authors. See copying.md for legal info.
+// Copyright 2023-2024 the openage authors. See copying.md for legal info.
 
 #include "idle.h"
 

libopenage/renderer/stages/hud/object.cpp

@@ -36,20 +36,17 @@ void HudDragObject::fetch_updates(const time::time_t &time) {
 		return;
 	}
 
-	// Get data from render entity
-	this->drag_start = this->render_entity->get_drag_start();
-
 	// Thread-safe access to curves needs a lock on the render entity's mutex
 	auto read_lock = this->render_entity->get_read_lock();
 
-	this->drag_pos.sync(this->render_entity->get_drag_pos() /* , this->last_update */);
+	// Get data from render entity
+	this->drag_start = this->render_entity->get_drag_start();
 
-	// Unlock the render entity mutex
-	read_lock.unlock();
+	this->drag_pos.sync(this->render_entity->get_drag_pos() /* , this->last_update */);
 
 	// Set self to changed so that world renderer can update the renderable
 	this->changed = true;
-	this->render_entity->clear_changed_flag();
+	this->render_entity->fetch_done();
 	this->last_update = time;
 }
 

libopenage/renderer/stages/hud/render_entity.cpp

@@ -20,18 +20,15 @@ void DragRenderEntity::update(const coord::input drag_pos,
 	this->drag_pos.set_insert(time, drag_pos);
 
 	this->last_update = time;
+	this->fetch_time = time;
 	this->changed = true;
 }
 
 const curve::Continuous<coord::input> &DragRenderEntity::get_drag_pos() {
-	std::shared_lock lock{this->mutex};
-
 	return this->drag_pos;
 }
 
 const coord::input DragRenderEntity::get_drag_start() {
-	std::shared_lock lock{this->mutex};
-
 	return this->drag_start;
 }
 

libopenage/renderer/stages/hud/render_entity.h

@@ -39,18 +39,13 @@ class DragRenderEntity final : public renderer::RenderEntity {
 	/**
 	 * Get the position of the dragged corner.
 	 *
-	 * Accessing the drag position curve REQUIRES a read lock on the render entity
-	 * (using \p get_read_lock()) to ensure thread safety.
-	 *
 	 * @return Coordinates of the dragged corner.
 	 */
 	const curve::Continuous<coord::input> &get_drag_pos();
 
 	/**
 	 * Get the position of the start corner.
 	 *
-	 * Accessing the drag start is thread-safe.
-	 *
 	 * @return Coordinates of the start corner.
 	 */
 	const coord::input get_drag_start();

libopenage/renderer/stages/render_entity.cpp

@@ -2,20 +2,17 @@
 
 #include "render_entity.h"
 
-#include <mutex>
-
 
 namespace openage::renderer {
 
 RenderEntity::RenderEntity() :
 	changed{false},
-	last_update{time::time_t::zero()} {
+	last_update{time::TIME_ZERO},
+	fetch_time{time::TIME_MAX} {
 }
 
-time::time_t RenderEntity::get_update_time() {
-	std::shared_lock lock{this->mutex};
-
-	return this->last_update;
+time::time_t RenderEntity::get_fetch_time() {
+	return this->fetch_time;
 }
 
 bool RenderEntity::is_changed() {
@@ -24,14 +21,13 @@ bool RenderEntity::is_changed() {
 	return this->changed;
 }
 
-void RenderEntity::clear_changed_flag() {
-	std::unique_lock lock{this->mutex};
-
+void RenderEntity::fetch_done() {
 	this->changed = false;
+	this->fetch_time = time::TIME_MAX;
 }
 
-std::shared_lock<std::shared_mutex> RenderEntity::get_read_lock() {
-	return std::shared_lock{this->mutex};
+std::unique_lock<std::shared_mutex> RenderEntity::get_read_lock() {
+	return std::unique_lock{this->mutex};
 }
 
 } // namespace openage::renderer

libopenage/renderer/stages/render_entity.h

@@ -15,32 +15,47 @@ namespace openage::renderer {
 /**
  * Interface for render entities that allow pushing updates from game simulation
  * to renderer.
+ *
+ * Accessing the render entity from the renderer thread REQUIRES a
+ * read lock on the render entity (using \p get_read_lock()) to ensure
+ * thread safety.
  */
 class RenderEntity {
 public:
 	~RenderEntity() = default;
 
 	/**
-	 * Get the time of the last update.
+	 * Get the earliest time for which updates are available.
+	 *
+	 * Render objects should synchronize their state with the render entity
+	 * from this time onwards.
 	 *
 	 * Accessing the update time is thread-safe.
 	 *
 	 * @return Time of last update.
 	 */
-	time::time_t get_update_time();
+	time::time_t get_fetch_time();
 
 	/**
 	 * Check whether the render entity has received new updates from the
 	 * gamestate.
 	 *
+	 * Accessing the change flag is thread-safe.
+	 *
 	 * @return true if updates have been received, else false.
 	 */
 	bool is_changed();
 
 	/**
-	 * Clear the update flag by setting it to false.
+	 * Indicate to this entity that its updates have been processed and transfered to the
+	 * render object.
+	 *
+	 * - Clear the update flag by setting it to false.
+	 * - Sets the fetch time to \p time::MAX_TIME.
+	 *
+	 * Accessing this method is thread-safe.
 	 */
-	void clear_changed_flag();
+	void fetch_done();
 
 	/**
 	 * Get a shared lock for thread-safe reading from the render entity.
@@ -49,7 +64,7 @@ class RenderEntity {
 	 *
 	 * @return Lock for the render entity.
 	 */
-	std::shared_lock<std::shared_mutex> get_read_lock();
+	std::unique_lock<std::shared_mutex> get_read_lock();
 
 protected:
 	/**
@@ -71,10 +86,17 @@ class RenderEntity {
 	bool changed;
 
 	/**
-	 * Time of the last update call.
+	 * Time of the last update.
 	 */
 	time::time_t last_update;
 
+	/**
+	 * Earliest time for which updates have been received.
+	 *
+	 * \p time::TIME_MAX indicates that no updates are available.
+	 */
+	time::time_t fetch_time;
+
 	/**
 	 * Mutex for protecting threaded access.
 	 */

libopenage/renderer/stages/terrain/chunk.cpp

@@ -32,6 +32,9 @@ void TerrainChunk::fetch_updates(const time::time_t & /* time */) {
 		return;
 	}
 
+	// Thread-safe access to data needs a lock on the render entity's mutex
+	auto read_lock = this->render_entity->get_read_lock();
+
 	// Get the terrain data from the render entity
 	auto terrain_size = this->render_entity->get_size();
 	auto terrain_paths = this->render_entity->get_terrain_paths();
@@ -54,7 +57,7 @@ void TerrainChunk::fetch_updates(const time::time_t & /* time */) {
 	// this->meshes.push_back(new_mesh);
 
 	// Indicate to the render entity that its updates have been processed.
-	this->render_entity->clear_changed_flag();
+	this->render_entity->fetch_done();
 }
 
 void TerrainChunk::update_uniforms(const time::time_t &time) {

libopenage/renderer/stages/terrain/render_entity.cpp

@@ -42,6 +42,7 @@ void RenderEntity::update_tile(const util::Vector2s size,
 
 	// update the last update time
 	this->last_update = time;
+	this->fetch_time = time;
 
 	// update the terrain paths
 	this->terrain_paths.insert(terrain_path);
@@ -94,7 +95,7 @@ void RenderEntity::update(const util::Vector2s size,
 	this->tiles = tiles;
 
 	// update the last update time
-	this->last_update = time;
+	this->fetch_time = time;
 
 	// update the terrain paths
 	this->terrain_paths.clear();
@@ -106,26 +107,18 @@ void RenderEntity::update(const util::Vector2s size,
 }
 
 const std::vector<coord::scene3> RenderEntity::get_vertices() {
-	std::shared_lock lock{this->mutex};
-
 	return this->vertices;
 }
 
 const RenderEntity::tiles_t RenderEntity::get_tiles() {
-	std::shared_lock lock{this->mutex};
-
 	return this->tiles;
 }
 
 const std::unordered_set<std::string> RenderEntity::get_terrain_paths() {
-	std::shared_lock lock{this->mutex};
-
 	return this->terrain_paths;
 }
 
 const util::Vector2s RenderEntity::get_size() {
-	std::shared_lock lock{this->mutex};
-
 	return this->size;
 }
 

libopenage/renderer/stages/terrain/render_entity.h

@@ -61,36 +61,28 @@ class RenderEntity final : public renderer::RenderEntity {
 	/**
 	 * Get the vertices of the terrain.
 	 *
-	 * Accessing the terrain vertices is thread-safe.
-	 *
 	 * @return Vector of vertex coordinates.
 	 */
 	const std::vector<coord::scene3> get_vertices();
 
 	/**
 	 * Get the tiles of the terrain.
 	 *
-	 * Accessing the terrain tiles is thread-safe.
-	 *
 	 * @return Terrain tiles.
 	 */
 	const tiles_t get_tiles();
 
 	/**
 	 * Get the terrain paths used in the terrain.
 	 *
-	 * Accessing the terrain paths is thread-safe.
-	 *
 	 * @return Terrain paths.
 	 */
 	const std::unordered_set<std::string> get_terrain_paths();
 
 	/**
 	 * Get the number of vertices on each side of the terrain.
 	 *
-	 * Accessing the vertices size is thread-safe.
-	 *
-	 * @return Vector with width as first element and height as second element.
+	 * @return Number of vertices on each side (width x height).
 	 */
 	const util::Vector2s get_size();