Skip to content

Commit

Permalink
Remove code that clears non-procedural nodes from islands that have n…
Browse files Browse the repository at this point in the history
…o procedural node connecting to them as it has been found to have race conditions. Will be revisted later in the island-worker-refactor branch.
  • Loading branch information
xissburg committed Jan 27, 2022
1 parent 8e683a8 commit a52502f
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 98 deletions.
2 changes: 0 additions & 2 deletions include/edyn/parallel/island_coordinator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ class island_coordinator final {
void refresh_dirty_entities();
bool should_split_island(entt::entity source_island_entity);
void sync();
void clear_dangling_non_procedural_nodes();

public:
island_coordinator(island_coordinator const&) = delete;
Expand Down Expand Up @@ -110,7 +109,6 @@ class island_coordinator final {
std::vector<entt::entity> m_new_graph_nodes;
std::vector<entt::entity> m_new_graph_edges;
std::vector<entt::entity> m_islands_to_split;
entt::sparse_set m_possibly_dangling_np_nodes;

bool m_importing_delta {false};
double m_timestamp;
Expand Down
3 changes: 0 additions & 3 deletions include/edyn/parallel/island_worker.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ class island_worker final {
bool should_split();
void sync();
void sync_dirty();
void clear_dangling_non_procedural_nodes();
void update();

public:
Expand Down Expand Up @@ -126,8 +125,6 @@ class island_worker final {

std::vector<entt::entity> m_new_polyhedron_shapes;
std::vector<entt::entity> m_new_compound_shapes;
entt::sparse_set m_possibly_dangling_np_nodes;
bool m_clearing_dangling_np_nodes {false};

std::atomic<int> m_reschedule_counter {0};

Expand Down
48 changes: 0 additions & 48 deletions src/edyn/parallel/island_coordinator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,6 @@ void island_coordinator::on_destroy_graph_node(entt::registry &registry, entt::e
void island_coordinator::on_destroy_graph_edge(entt::registry &registry, entt::entity entity) {
auto &edge = registry.get<graph_edge>(entity);
auto &graph = registry.ctx<entity_graph>();

auto nodes = graph.edge_node_entities(edge.edge_index);

for (auto node : std::array{nodes.first, nodes.second}) {
if (!registry.any_of<procedural_tag>(node) && !m_possibly_dangling_np_nodes.contains(node)) {
m_possibly_dangling_np_nodes.emplace(node);
}
}

graph.remove_edge(edge.edge_index);
}

Expand Down Expand Up @@ -780,44 +771,6 @@ void island_coordinator::sync() {
}
}

void island_coordinator::clear_dangling_non_procedural_nodes() {
// Remove non-procedural entities from islands that have no procedural
// entities connecting to them.
auto &graph = m_registry->ctx<entity_graph>();

for (auto entity : m_possibly_dangling_np_nodes) {
if (!m_registry->valid(entity)) {
continue;
}

auto &resident = m_registry->get<multi_island_resident>(entity);
auto node_index = m_registry->get<graph_node>(entity).node_index;

for (auto island_entity : resident.island_entities) {
auto &island = m_registry->get<edyn::island>(island_entity);
bool has_procedural_neighbor = false;

for (auto node_entity : island.nodes) {
if (m_registry->any_of<procedural_tag>(node_entity) &&
graph.has_adjacency(node_index, m_registry->get<graph_node>(node_entity).node_index)) {
has_procedural_neighbor = true;
break;
}
}

if (!has_procedural_neighbor) {
island.nodes.remove(entity);
resident.island_entities.remove(island_entity);

auto &ctx = m_island_ctx_map.at(island_entity);
ctx->m_entity_map.erase_loc(entity);
}
}
}

m_possibly_dangling_np_nodes.clear();
}

void island_coordinator::update() {
m_timestamp = performance_time();

Expand All @@ -828,7 +781,6 @@ void island_coordinator::update() {
init_new_nodes_and_edges();
refresh_dirty_entities();
sync();
clear_dangling_non_procedural_nodes();
split_islands();
}

Expand Down
47 changes: 2 additions & 45 deletions src/edyn/parallel/island_worker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,7 @@ void island_worker::on_destroy_graph_node(entt::registry &registry, entt::entity
graph.remove_node(node.node_index);

if (!m_importing_delta &&
!m_splitting.load(std::memory_order_relaxed) &&
!m_clearing_dangling_np_nodes) {
!m_splitting.load(std::memory_order_relaxed)) {
m_delta_builder->destroyed(entity);
}

Expand All @@ -183,23 +182,14 @@ void island_worker::on_destroy_graph_node(entt::registry &registry, entt::entity

void island_worker::on_destroy_graph_edge(entt::registry &registry, entt::entity entity) {
auto &graph = registry.ctx<entity_graph>();

auto &edge = registry.get<graph_edge>(entity);
auto nodes = graph.edge_node_entities(edge.edge_index);

for (auto node : std::array{nodes.first, nodes.second}) {
if (!registry.any_of<procedural_tag>(node) && !m_possibly_dangling_np_nodes.contains(node)) {
m_possibly_dangling_np_nodes.emplace(node);
}
}

if (!m_destroying_node) {
graph.remove_edge(edge.edge_index);
}

if (!m_importing_delta &&
!m_splitting.load(std::memory_order_relaxed) &&
!m_clearing_dangling_np_nodes) {
!m_splitting.load(std::memory_order_relaxed)) {
m_delta_builder->destroyed(entity);
}

Expand Down Expand Up @@ -377,38 +367,6 @@ void island_worker::sync_dirty() {
m_registry.clear<dirty>();
}

void island_worker::clear_dangling_non_procedural_nodes() {
m_clearing_dangling_np_nodes = true;

auto &graph = m_registry.ctx<entity_graph>();
auto node_view = m_registry.view<graph_node>();
auto proc_view = m_registry.view<procedural_tag>();

for (auto entity : m_possibly_dangling_np_nodes) {
if (!m_registry.valid(entity)) {
continue;
}

auto node_index = node_view.get<graph_node>(entity).node_index;
bool has_procedural_neighbor = false;

for (auto node_entity : m_registry.view<graph_node>()) {
if (proc_view.contains(node_entity) &&
graph.has_adjacency(node_index, node_view.get<graph_node>(node_entity).node_index)) {
has_procedural_neighbor = true;
break;
}
}

if (!has_procedural_neighbor) {
m_registry.destroy(entity);
}
}

m_possibly_dangling_np_nodes.clear();
m_clearing_dangling_np_nodes = false;
}

void island_worker::update() {
switch (m_state) {
case state::init:
Expand Down Expand Up @@ -617,7 +575,6 @@ void island_worker::finish_step() {
(*settings.external_system_post_step)(m_registry);
}

clear_dangling_non_procedural_nodes();
sync();

m_state = state::step;
Expand Down

0 comments on commit a52502f

Please sign in to comment.