Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix thread-safe access for render entities #1692

Merged
merged 7 commits into from
Oct 31, 2024
Prev Previous commit
Next Next commit
refactor: Shorten render entity class name in Terrain render stage.
  • Loading branch information
heinezen committed Oct 15, 2024
commit 3f73385ba0bdbef0fbecc11a1f3cc2aca77452ac
2 changes: 1 addition & 1 deletion libopenage/gamestate/terrain_chunk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ TerrainChunk::TerrainChunk(const util::Vector2s size,
}
}

void TerrainChunk::set_render_entity(const std::shared_ptr<renderer::terrain::TerrainRenderEntity> &entity) {
void TerrainChunk::set_render_entity(const std::shared_ptr<renderer::terrain::RenderEntity> &entity) {
this->render_entity = entity;
}

Expand Down
4 changes: 2 additions & 2 deletions libopenage/gamestate/terrain_chunk.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class TerrainChunk {
*
* @param entity New render entity.
*/
void set_render_entity(const std::shared_ptr<renderer::terrain::TerrainRenderEntity> &entity);
void set_render_entity(const std::shared_ptr<renderer::terrain::RenderEntity> &entity);

/**
* Get the size of this terrain chunk.
Expand Down Expand Up @@ -78,7 +78,7 @@ class TerrainChunk {
/**
* Render entity for pushing updates to the renderer. Can be \p nullptr.
*/
std::shared_ptr<renderer::terrain::TerrainRenderEntity> render_entity;
std::shared_ptr<renderer::terrain::RenderEntity> render_entity;
};

} // namespace openage::gamestate
2 changes: 1 addition & 1 deletion libopenage/renderer/demo/demo_3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ void renderer_demo_3(const util::Path &path) {

// Fill a 10x10 terrain grid with height values
auto terrain_size = util::Vector2s{10, 10};
std::vector<std::pair<terrain::TerrainRenderEntity::terrain_elevation_t, std::string>> tiles{};
std::vector<std::pair<terrain::RenderEntity::terrain_elevation_t, std::string>> tiles{};
tiles.reserve(terrain_size[0] * terrain_size[1]);
for (size_t i = 0; i < terrain_size[0] * terrain_size[1]; ++i) {
tiles.emplace_back(0.0f, "./textures/test_terrain.terrain");
Expand Down
2 changes: 1 addition & 1 deletion libopenage/renderer/demo/stresstest_0.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ void renderer_stresstest_0(const util::Path &path) {

// Fill a 10x10 terrain grid with height values
auto terrain_size = util::Vector2s{10, 10};
std::vector<std::pair<terrain::TerrainRenderEntity::terrain_elevation_t, std::string>> tiles{};
std::vector<std::pair<terrain::RenderEntity::terrain_elevation_t, std::string>> tiles{};
tiles.reserve(terrain_size[0] * terrain_size[1]);
for (size_t i = 0; i < terrain_size[0] * terrain_size[1]; ++i) {
tiles.emplace_back(0.0f, "./textures/test_terrain.terrain");
Expand Down
2 changes: 1 addition & 1 deletion libopenage/renderer/demo/stresstest_1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ void renderer_stresstest_1(const util::Path &path) {

// Fill a 10x10 terrain grid with height values
auto terrain_size = util::Vector2s{10, 10};
std::vector<std::pair<terrain::TerrainRenderEntity::terrain_elevation_t, std::string>> tiles{};
std::vector<std::pair<terrain::RenderEntity::terrain_elevation_t, std::string>> tiles{};
tiles.reserve(terrain_size[0] * terrain_size[1]);
for (size_t i = 0; i < terrain_size[0] * terrain_size[1]; ++i) {
tiles.emplace_back(0.0f, "./textures/test_terrain.terrain");
Expand Down
8 changes: 4 additions & 4 deletions libopenage/renderer/render_factory.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022-2023 the openage authors. See copying.md for legal info.
// Copyright 2022-2024 the openage authors. See copying.md for legal info.

#include "render_factory.h"

Expand All @@ -15,9 +15,9 @@ RenderFactory::RenderFactory(const std::shared_ptr<terrain::TerrainRenderStage>
world_renderer{world_renderer} {
}

std::shared_ptr<terrain::TerrainRenderEntity> RenderFactory::add_terrain_render_entity(const util::Vector2s chunk_size,
const coord::tile_delta chunk_offset) {
auto entity = std::make_shared<terrain::TerrainRenderEntity>();
std::shared_ptr<terrain::RenderEntity> RenderFactory::add_terrain_render_entity(const util::Vector2s chunk_size,
const coord::tile_delta chunk_offset) {
auto entity = std::make_shared<terrain::RenderEntity>();
this->terrain_renderer->add_render_entity(entity, chunk_size, chunk_offset.to_phys2().to_scene2());

return entity;
Expand Down
6 changes: 3 additions & 3 deletions libopenage/renderer/render_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
namespace openage::renderer {
namespace terrain {
class TerrainRenderStage;
class TerrainRenderEntity;
class RenderEntity;
} // namespace terrain

namespace world {
Expand Down Expand Up @@ -47,8 +47,8 @@ class RenderFactory {
*
* @return Render entity for pushing terrain updates.
*/
std::shared_ptr<terrain::TerrainRenderEntity> add_terrain_render_entity(const util::Vector2s chunk_size,
const coord::tile_delta chunk_offset);
std::shared_ptr<terrain::RenderEntity> add_terrain_render_entity(const util::Vector2s chunk_size,
const coord::tile_delta chunk_offset);

/**
* Create a new world render entity and register it at the world renderer.
Expand Down
2 changes: 1 addition & 1 deletion libopenage/renderer/stages/terrain/chunk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ TerrainChunk::TerrainChunk(const std::shared_ptr<renderer::resources::AssetManag
offset{offset},
asset_manager{asset_manager} {}

void TerrainChunk::set_render_entity(const std::shared_ptr<TerrainRenderEntity> &entity) {
void TerrainChunk::set_render_entity(const std::shared_ptr<RenderEntity> &entity) {
this->render_entity = entity;
}

Expand Down
6 changes: 3 additions & 3 deletions libopenage/renderer/stages/terrain/chunk.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AssetManager;

namespace terrain {
class TerrainRenderMesh;
class TerrainRenderEntity;
class RenderEntity;

/**
* Stores the state of a terrain chunk in the terrain render stage.
Expand All @@ -44,7 +44,7 @@ class TerrainChunk {
* @param size Size of the chunk in tiles.
* @param offset Offset of the chunk from origin in tiles.
*/
void set_render_entity(const std::shared_ptr<TerrainRenderEntity> &entity);
void set_render_entity(const std::shared_ptr<RenderEntity> &entity);

/**
* Fetch updates from the render entity.
Expand Down Expand Up @@ -114,7 +114,7 @@ class TerrainChunk {
* Source for ingame terrain coordinates. These coordinates are translated into
* our render vertex mesh when \p update() is called.
*/
std::shared_ptr<TerrainRenderEntity> render_entity;
std::shared_ptr<RenderEntity> render_entity;
};


Expand Down
4 changes: 2 additions & 2 deletions libopenage/renderer/stages/terrain/model.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022-2023 the openage authors. See copying.md for legal info.
// Copyright 2022-2024 the openage authors. See copying.md for legal info.

#include "model.h"

Expand Down Expand Up @@ -26,7 +26,7 @@ TerrainRenderModel::TerrainRenderModel(const std::shared_ptr<renderer::resources
asset_manager{asset_manager} {
}

void TerrainRenderModel::add_chunk(const std::shared_ptr<TerrainRenderEntity> &entity,
void TerrainRenderModel::add_chunk(const std::shared_ptr<RenderEntity> &entity,
const util::Vector2s size,
const coord::scene2_delta offset) {
auto chunk = std::make_shared<TerrainChunk>(this->asset_manager, size, offset);
Expand Down
4 changes: 2 additions & 2 deletions libopenage/renderer/stages/terrain/model.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class AssetManager;
}

namespace terrain {
class TerrainRenderEntity;
class RenderEntity;
class TerrainRenderMesh;
class TerrainChunk;

Expand All @@ -46,7 +46,7 @@ class TerrainRenderModel {
* @param chunk_size Size of the chunk in tiles.
* @param chunk_offset Offset of the chunk from origin in tiles.
*/
void add_chunk(const std::shared_ptr<TerrainRenderEntity> &entity,
void add_chunk(const std::shared_ptr<RenderEntity> &entity,
const util::Vector2s chunk_size,
const coord::scene2_delta chunk_offset);

Expand Down
28 changes: 14 additions & 14 deletions libopenage/renderer/stages/terrain/render_entity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

namespace openage::renderer::terrain {

TerrainRenderEntity::TerrainRenderEntity() :
RenderEntity{},
RenderEntity::RenderEntity() :
renderer::RenderEntity{},
size{0, 0},
tiles{},
terrain_paths{},
Expand All @@ -19,11 +19,11 @@ TerrainRenderEntity::TerrainRenderEntity() :
{
}

void TerrainRenderEntity::update_tile(const util::Vector2s size,
const coord::tile &pos,
const terrain_elevation_t elevation,
const std::string terrain_path,
const time::time_t time) {
void RenderEntity::update_tile(const util::Vector2s size,
const coord::tile &pos,
const terrain_elevation_t elevation,
const std::string terrain_path,
const time::time_t time) {
std::unique_lock lock{this->mutex};

if (this->vertices.empty()) {
Expand Down Expand Up @@ -51,9 +51,9 @@ void TerrainRenderEntity::update_tile(const util::Vector2s size,
this->changed = true;
}

void TerrainRenderEntity::update(const util::Vector2s size,
const tiles_t tiles,
const time::time_t time) {
void RenderEntity::update(const util::Vector2s size,
const tiles_t tiles,
const time::time_t time) {
std::unique_lock lock{this->mutex};

// increase by 1 in every dimension because tiles
Expand Down Expand Up @@ -107,25 +107,25 @@ void TerrainRenderEntity::update(const util::Vector2s size,
this->changed = true;
}

const std::vector<coord::scene3> &TerrainRenderEntity::get_vertices() {
const std::vector<coord::scene3> &RenderEntity::get_vertices() {
std::shared_lock lock{this->mutex};

return this->vertices;
}

const TerrainRenderEntity::tiles_t &TerrainRenderEntity::get_tiles() {
const RenderEntity::tiles_t &RenderEntity::get_tiles() {
std::shared_lock lock{this->mutex};

return this->tiles;
}

const std::unordered_set<std::string> &TerrainRenderEntity::get_terrain_paths() {
const std::unordered_set<std::string> &RenderEntity::get_terrain_paths() {
std::shared_lock lock{this->mutex};

return this->terrain_paths;
}

const util::Vector2s &TerrainRenderEntity::get_size() {
const util::Vector2s &RenderEntity::get_size() {
std::shared_lock lock{this->mutex};

return this->size;
Expand Down
6 changes: 3 additions & 3 deletions libopenage/renderer/stages/terrain/render_entity.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ namespace openage::renderer::terrain {
/**
* Render entity for pushing updates to the Terrain renderer.
*/
class TerrainRenderEntity final : public renderer::RenderEntity {
class RenderEntity final : public renderer::RenderEntity {
public:
TerrainRenderEntity();
~TerrainRenderEntity() = default;
RenderEntity();
~RenderEntity() = default;

using terrain_elevation_t = util::FixedPoint<uint64_t, 16>;
using tiles_t = std::vector<std::pair<terrain_elevation_t, std::string>>;
Expand Down
2 changes: 1 addition & 1 deletion libopenage/renderer/stages/terrain/render_stage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ std::shared_ptr<renderer::RenderPass> TerrainRenderStage::get_render_pass() {
return this->render_pass;
}

void TerrainRenderStage::add_render_entity(const std::shared_ptr<TerrainRenderEntity> entity,
void TerrainRenderStage::add_render_entity(const std::shared_ptr<RenderEntity> entity,
const util::Vector2s chunk_size,
const coord::scene2_delta chunk_offset) {
std::unique_lock lock{this->mutex};
Expand Down
6 changes: 3 additions & 3 deletions libopenage/renderer/stages/terrain/render_stage.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class AssetManager;
}

namespace terrain {
class TerrainRenderEntity;
class RenderEntity;
class TerrainRenderMesh;
class TerrainRenderModel;

Expand Down Expand Up @@ -73,7 +73,7 @@ class TerrainRenderStage {
*
* @param render_entity New render entity.
*/
void add_render_entity(const std::shared_ptr<TerrainRenderEntity> entity,
void add_render_entity(const std::shared_ptr<RenderEntity> entity,
const util::Vector2s chunk_size,
const coord::scene2_delta chunk_offset);

Expand Down Expand Up @@ -119,7 +119,7 @@ class TerrainRenderStage {
/**
* Engine interface for updating terrain draw information.
*/
std::shared_ptr<TerrainRenderEntity> render_entity;
std::shared_ptr<RenderEntity> render_entity;

/**
* 3D model of the terrain.
Expand Down