From 62e86f45f41b39af5852ca5685f2190b17eec785 Mon Sep 17 00:00:00 2001 From: BlackMATov Date: Sun, 10 Feb 2019 07:48:13 +0700 Subject: [PATCH] node is final class without virtual events --- headers/enduro2d/high/node.hpp | 6 +- sources/enduro2d/high/node.cpp | 147 +++------------- untests/sources/untests_high/node.cpp | 243 -------------------------- 3 files changed, 31 insertions(+), 365 deletions(-) diff --git a/headers/enduro2d/high/node.hpp b/headers/enduro2d/high/node.hpp index d1d4755e..62b053e2 100644 --- a/headers/enduro2d/high/node.hpp +++ b/headers/enduro2d/high/node.hpp @@ -88,6 +88,9 @@ namespace e2d bool add_sibling_after( const node_iptr& sibling) noexcept; + bool remove_child( + const node_iptr& child) noexcept; + bool send_backward() noexcept; bool bring_to_back() noexcept; @@ -113,9 +116,6 @@ namespace e2d void for_each_child(F&& f) const; protected: node(world& world); - protected: - virtual void on_change_parent_() noexcept; - virtual void on_change_children_() noexcept; private: enum flag_masks : u32 { fm_dirty_local_matrix = 1u << 0, diff --git a/sources/enduro2d/high/node.cpp b/sources/enduro2d/high/node.cpp index 46d05981..8cec5c4b 100644 --- a/sources/enduro2d/high/node.cpp +++ b/sources/enduro2d/high/node.cpp @@ -7,17 +7,6 @@ #include #include -namespace -{ - using namespace e2d; - - struct iptr_release_disposer { - void operator()(node* n) const noexcept { - intrusive_ptr_release(n); - } - }; -} - namespace e2d { node::node(world& world) @@ -25,12 +14,7 @@ namespace e2d node::~node() noexcept { E2D_ASSERT(!parent_); - while ( !children_.empty() ) { - node_iptr child(&children_.back()); - children_.pop_back_and_dispose(iptr_release_disposer()); - child->parent_ = nullptr; - child->on_change_parent_(); - } + remove_all_children(); } node_iptr node::create(world& world) { @@ -143,30 +127,15 @@ namespace e2d } bool node::remove_from_parent() noexcept { - if ( !parent_ ) { - return false; - } - node_iptr self(this); - parent_->children_.erase_and_dispose( - node_children::iterator_to(*this), - iptr_release_disposer()); - parent_->on_change_children_(); - parent_ = nullptr; - on_change_parent_(); - return true; + return parent_ + ? parent_->remove_child(this) + : false; } std::size_t node::remove_all_children() noexcept { - std::size_t count = 0; + const std::size_t count = children_.size(); while ( !children_.empty() ) { - node_iptr child(&children_.back()); - children_.pop_back_and_dispose(iptr_release_disposer()); - child->parent_ = nullptr; - child->on_change_parent_(); - ++count; - } - if ( count > 0 ) { - on_change_children_(); + children_.back().remove_from_parent(); } return count; } @@ -198,26 +167,11 @@ namespace e2d return true; } - if ( child->parent_ && child->parent_ == this ) { - children_.erase(node_children::iterator_to(*child)); - children_.push_front(*child); - on_change_children_(); - return true; - } - - if ( child->parent_ ) { - child->parent_->children_.erase_and_dispose( - node_children::iterator_to(*child), - iptr_release_disposer()); - child->parent_->on_change_children_(); - child->parent_ = nullptr; - } - intrusive_ptr_add_ref(child.get()); + child->remove_from_parent(); children_.push_front(*child); child->parent_ = this; - child->on_change_parent_(); - on_change_children_(); + child->mark_dirty_world_matrix_(); return true; } @@ -232,26 +186,11 @@ namespace e2d return true; } - if ( child->parent_ && child->parent_ == this ) { - children_.erase(node_children::iterator_to(*child)); - children_.push_back(*child); - on_change_children_(); - return true; - } - - if ( child->parent_ ) { - child->parent_->children_.erase_and_dispose( - node_children::iterator_to(*child), - iptr_release_disposer()); - child->parent_->on_change_children_(); - child->parent_ = nullptr; - } - intrusive_ptr_add_ref(child.get()); + child->remove_from_parent(); children_.push_back(*child); child->parent_ = this; - child->on_change_parent_(); - on_change_children_(); + child->mark_dirty_world_matrix_(); return true; } @@ -267,30 +206,13 @@ namespace e2d return true; } - if ( child->parent_ && child->parent_ == this ) { - children_.erase(node_children::iterator_to(*child)); - children_.insert( - node_children::iterator_to(*before), - *child); - on_change_children_(); - return true; - } - - if ( child->parent_ ) { - child->parent_->children_.erase_and_dispose( - node_children::iterator_to(*child), - iptr_release_disposer()); - child->parent_->on_change_children_(); - child->parent_ = nullptr; - } - intrusive_ptr_add_ref(child.get()); + child->remove_from_parent(); children_.insert( node_children::iterator_to(*before), *child); child->parent_ = this; - child->on_change_parent_(); - on_change_children_(); + child->mark_dirty_world_matrix_(); return true; } @@ -306,30 +228,13 @@ namespace e2d return true; } - if ( child->parent_ && child->parent_ == this ) { - children_.erase(node_children::iterator_to(*child)); - children_.insert( - ++node_children::iterator_to(*after), - *child); - on_change_children_(); - return true; - } - - if ( child->parent_ ) { - child->parent_->children_.erase_and_dispose( - node_children::iterator_to(*child), - iptr_release_disposer()); - child->parent_->on_change_children_(); - child->parent_ = nullptr; - } - intrusive_ptr_add_ref(child.get()); + child->remove_from_parent(); children_.insert( ++node_children::iterator_to(*after), *child); child->parent_ = this; - child->on_change_parent_(); - on_change_children_(); + child->mark_dirty_world_matrix_(); return true; } @@ -345,6 +250,20 @@ namespace e2d : false; } + bool node::remove_child(const node_iptr& child) noexcept { + if ( !child || child->parent_ != this ) { + return false; + } + children_.erase_and_dispose( + node_children::iterator_to(*child), + [](node* n){ + n->parent_ = nullptr; + n->mark_dirty_world_matrix_(); + intrusive_ptr_release(n); + }); + return true; + } + bool node::send_backward() noexcept { node_iptr prev = prev_sibling(); return prev @@ -442,16 +361,6 @@ namespace e2d } } -namespace e2d -{ - void node::on_change_parent_() noexcept { - mark_dirty_world_matrix_(); - } - - void node::on_change_children_() noexcept { - } -} - namespace e2d { void node::mark_dirty_local_matrix_() noexcept { diff --git a/untests/sources/untests_high/node.cpp b/untests/sources/untests_high/node.cpp index b7eaf54f..1f9a5f9a 100644 --- a/untests/sources/untests_high/node.cpp +++ b/untests/sources/untests_high/node.cpp @@ -32,29 +32,13 @@ namespace ~fake_node() noexcept final { ++s_dtor_count; } - - void on_change_parent_() noexcept override { - ++parent_changes; - ++s_parent_changes; - } - - void on_change_children_() noexcept override { - ++children_changes; - ++s_children_changes; - } public: static void reset_counters() noexcept { s_ctor_count = 0; s_dtor_count = 0; - s_parent_changes = 0; - s_children_changes = 0; } - std::size_t parent_changes{0}; - std::size_t children_changes{0}; static std::size_t s_ctor_count; static std::size_t s_dtor_count; - static std::size_t s_parent_changes; - static std::size_t s_children_changes; public: static intrusive_ptr create(world& world) { return intrusive_ptr(new fake_node(world)); @@ -71,8 +55,6 @@ namespace std::size_t fake_node::s_ctor_count{0}; std::size_t fake_node::s_dtor_count{0}; - std::size_t fake_node::s_parent_changes{0}; - std::size_t fake_node::s_children_changes{0}; } TEST_CASE("node") { @@ -217,34 +199,6 @@ TEST_CASE("node") { REQUIRE(p->remove_all_children() == 0); } } - SECTION("auto_remove/remove_all_children/events") { - { - fake_node::reset_counters(); - auto p = fake_node::create(w); - auto n = fake_node::create(w, p); - n->remove_from_parent(); - REQUIRE(p->children_changes == 2); - REQUIRE(fake_node::s_parent_changes == 2); - } - { - fake_node::reset_counters(); - auto p = fake_node::create(w); - auto n = fake_node::create(w, p); - p.reset(); - REQUIRE(fake_node::s_children_changes == 1); - REQUIRE(n->parent_changes == 2); - } - { - fake_node::reset_counters(); - auto p = fake_node::create(w); - auto n1 = fake_node::create(w, p); - auto n2 = fake_node::create(w, p); - REQUIRE(p->remove_all_children() == 2); - REQUIRE(p->remove_all_children() == 0); - REQUIRE(p->children_changes == 3); - REQUIRE(fake_node::s_parent_changes == 4); - } - } SECTION("remove_from_parent") { auto p = node::create(w); auto n1 = node::create(w, p); @@ -267,26 +221,6 @@ TEST_CASE("node") { REQUIRE_FALSE(n2->parent()); REQUIRE(p->child_count() == 0); } - SECTION("remove_from_parent/events") { - fake_node::reset_counters(); - - auto p = fake_node::create(w); - auto n1 = fake_node::create(w, p); - auto n2 = fake_node::create(w, p); - - auto np = fake_node::create(w); - np->remove_from_parent(); - REQUIRE(fake_node::s_parent_changes == 2); - REQUIRE(fake_node::s_children_changes == 2); - - REQUIRE(n1->remove_from_parent()); - REQUIRE(fake_node::s_parent_changes == 3); - REQUIRE(fake_node::s_children_changes == 3); - - REQUIRE(n2->remove_from_parent()); - REQUIRE(fake_node::s_parent_changes == 4); - REQUIRE(fake_node::s_children_changes == 4); - } SECTION("child_count/child_count_recursive") { auto p1 = node::create(w); REQUIRE(p1->child_count() == 0); @@ -345,14 +279,11 @@ TEST_CASE("node") { REQUIRE_FALSE(n3->send_backward()); // n3 n1 n2 REQUIRE(n2->bring_to_back()); // n2 n3 n1 REQUIRE(n2->bring_to_back()); // n2 n3 n1 - - REQUIRE(p->children_changes == 6); } { auto n = fake_node::create(w); REQUIRE_FALSE(n->send_backward()); REQUIRE_FALSE(n->bring_to_back()); - REQUIRE(n->children_changes == 0); } } SECTION("send_forward/bring_to_front") { @@ -395,14 +326,11 @@ TEST_CASE("node") { REQUIRE_FALSE(n1->send_forward()); // n2 n3 n1 REQUIRE(n2->bring_to_front()); // n3 n1 n2 REQUIRE(n2->bring_to_front()); // n3 n1 n2 - - REQUIRE(p->children_changes == 6); } { auto n = fake_node::create(w); REQUIRE_FALSE(n->send_forward()); REQUIRE_FALSE(n->bring_to_back()); - REQUIRE(n->children_changes == 0); } } SECTION("last_child/first_child") { @@ -546,88 +474,6 @@ TEST_CASE("node") { REQUIRE(n3->parent() == p); } } - SECTION("add_child_to_back/add_child_to_front/events") { - { - auto p = fake_node::create(w); - auto n1 = fake_node::create(w); - auto n2 = fake_node::create(w); - auto n3 = fake_node::create(w); - - REQUIRE(p->add_child_to_back(n1)); - REQUIRE(p->add_child_to_back(n2)); - REQUIRE(p->add_child_to_back(n3)); // n3 n2 n1 - REQUIRE_FALSE(p->add_child_to_back(nullptr)); - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 3); - REQUIRE(n1->parent_changes == 1); - REQUIRE(n1->children_changes == 0); - - // add to self parent (change order) - REQUIRE(p->add_child_to_back(n1)); // n1 n3 n2 - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 4); - REQUIRE(n1->parent_changes == 1); - REQUIRE(n1->children_changes == 0); - - // add to self parent (no change order) - REQUIRE(p->add_child_to_back(n1)); // n1 n3 n2 - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 4); - REQUIRE(n1->parent_changes == 1); - REQUIRE(n1->children_changes == 0); - - // add to another parent - auto p2 = node::create(w); - REQUIRE(p2->add_child_to_back(n1)); - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 5); - REQUIRE(n1->parent_changes == 2); - REQUIRE(n1->children_changes == 0); - - REQUIRE(n2->parent_changes == 1); - REQUIRE(n2->children_changes == 0); - } - { - auto p = fake_node::create(w); - auto n1 = fake_node::create(w); - auto n2 = fake_node::create(w); - auto n3 = fake_node::create(w); - - REQUIRE(p->add_child_to_front(n1)); - REQUIRE(p->add_child_to_front(n2)); - REQUIRE(p->add_child_to_front(n3)); // n1 n2 n3 - REQUIRE_FALSE(p->add_child_to_front(nullptr)); - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 3); - REQUIRE(n1->parent_changes == 1); - REQUIRE(n1->children_changes == 0); - - // add to self parent (change order) - REQUIRE(p->add_child_to_front(n1)); // n2 n3 n1 - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 4); - REQUIRE(n1->parent_changes == 1); - REQUIRE(n1->children_changes == 0); - - // add to self parent (no change order) - REQUIRE(p->add_child_to_front(n1)); // n2 n3 n1 - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 4); - REQUIRE(n1->parent_changes == 1); - REQUIRE(n1->children_changes == 0); - - // add to another parent - auto p2 = node::create(w); - REQUIRE(p2->add_child_to_front(n1)); - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 5); - REQUIRE(n1->parent_changes == 2); - REQUIRE(n1->children_changes == 0); - - REQUIRE(n2->parent_changes == 1); - REQUIRE(n2->children_changes == 0); - } - } SECTION("add_child_after/add_child_before") { auto p = node::create(w); auto n1 = node::create(w); @@ -710,78 +556,6 @@ TEST_CASE("node") { REQUIRE(n3->parent() == p); } } - SECTION("add_child_after/add_child_before/events") { - { - auto p = fake_node::create(w); - auto n1 = fake_node::create(w); - auto n2 = fake_node::create(w); - auto n3 = fake_node::create(w); - - REQUIRE(p->add_child(n1)); // n1 - REQUIRE(p->add_child_before(n1, n2)); // n2 n1 - REQUIRE(p->add_child_before(n1, n3)); // n2 n3 n1 - - REQUIRE(p->add_child_before(n1, n1)); // n2 n3 n1 - REQUIRE(p->add_child_before(n3, n2)); // n2 n3 n1 - - REQUIRE_FALSE(p->add_child_before(n1, nullptr)); - REQUIRE_FALSE(p->add_child_before(nullptr, n1)); - REQUIRE_FALSE(p->add_child_before(nullptr, nullptr)); - REQUIRE_FALSE(p->add_child_before(p, n3)); - - REQUIRE(p->add_child_before(n3, n1)); // n2 n1 n3 - REQUIRE(p->add_child_before(n2, n3)); // n3 n2 n1 - - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 5); - - // to another parent - auto p2 = node::create(w); - auto n4 = node::create(w, p2); // n4 - REQUIRE(p2->add_child_before(n4, n2)); // n2 n4 - - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 6); - - REQUIRE(n1->parent_changes == 1); - REQUIRE(n2->parent_changes == 2); - } - { - auto p = fake_node::create(w); - auto n1 = fake_node::create(w); - auto n2 = fake_node::create(w); - auto n3 = fake_node::create(w); - - REQUIRE(p->add_child(n1)); // n1 - REQUIRE(p->add_child_after(n1, n2)); // n1 n2 - REQUIRE(p->add_child_after(n1, n3)); // n1 n3 n2 - - REQUIRE(p->add_child_after(n1, n1)); // n1 n3 n2 - REQUIRE(p->add_child_after(n3, n2)); // n1 n3 n2 - - REQUIRE_FALSE(p->add_child_after(n1, nullptr)); - REQUIRE_FALSE(p->add_child_after(nullptr, n1)); - REQUIRE_FALSE(p->add_child_after(nullptr, nullptr)); - REQUIRE_FALSE(p->add_child_after(p, n3)); - - REQUIRE(p->add_child_after(n3, n1)); // n3 n1 n2 - REQUIRE(p->add_child_after(n2, n3)); // n1 n2 n3 - - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 5); - - // to another parent - auto p2 = node::create(w); - auto n4 = node::create(w, p2); // n4 - REQUIRE(p2->add_child_after(n4, n2)); // n4 n2 - - REQUIRE(p->parent_changes == 0); - REQUIRE(p->children_changes == 6); - - REQUIRE(n1->parent_changes == 1); - REQUIRE(n2->parent_changes == 2); - } - } SECTION("for_each_child") { auto p = node::create(w); array ns{ @@ -826,23 +600,6 @@ TEST_CASE("node") { REQUIRE_FALSE(n2->prev_sibling()); REQUIRE_FALSE(n2->next_sibling()); } - SECTION("destroy_node/events") { - fake_node::reset_counters(); - - auto p1 = fake_node::create(w); - auto p2 = fake_node::create(w, p1); - auto n1 = fake_node::create(w, p2); - auto n2 = fake_node::create(w, p2); - - REQUIRE(fake_node::s_parent_changes == 3); - REQUIRE(fake_node::s_children_changes == 3); - - p2->remove_from_parent(); - p2.reset(); - - REQUIRE(fake_node::s_parent_changes == 6); - REQUIRE(fake_node::s_children_changes == 4); - } SECTION("transform") { auto p = node::create(w);