From 2701f638831690a62e710d9937f7c6382faf5759 Mon Sep 17 00:00:00 2001 From: Lars Mueller Date: Mon, 6 May 2024 21:20:30 +0200 Subject: [PATCH] Deletion --- src/unittest/test_k_d_tree.cpp | 75 ++++++++++---------- src/util/k_d_tree.h | 122 +++++++++++++++------------------ 2 files changed, 91 insertions(+), 106 deletions(-) diff --git a/src/unittest/test_k_d_tree.cpp b/src/unittest/test_k_d_tree.cpp index 00ce2a571..924a61423 100644 --- a/src/unittest/test_k_d_tree.cpp +++ b/src/unittest/test_k_d_tree.cpp @@ -1,21 +1,5 @@ -/* -Minetest -Copyright (C) 2024 sfan5 - -This program is free software; you can redistribute it and/or modify -it under the terms of the GNU Lesser General Public License as published by -the Free Software Foundation; either version 2.1 of the License, or -(at your option) any later version. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU Lesser General Public License for more details. - -You should have received a copy of the GNU Lesser General Public License along -with this program; if not, write to the Free Software Foundation, Inc., -51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -*/ +// Copyright (C) 2024 Lars Müller +// SPDX-License-Identifier: LGPL-2.1-or-later #include "noise.h" #include "test.h" @@ -32,8 +16,8 @@ class TestKdTree : public TestBase void runTests(IGameDef *gamedef); - // TODO void cube(); - void randomInserts(); + // TODO basic small cube test + void randomOps(); }; template @@ -44,9 +28,11 @@ class ObjectVector { entries.push_back(Entry{p, id}); } void remove(Id id) { - entries.erase(std::find_if(entries.begin(), entries.end(), [&](const auto &e) { + const auto it = std::find_if(entries.begin(), entries.end(), [&](const auto &e) { return e.id == id; - })); + }); + assert(it != entries.end()); + entries.erase(it); } template void rangeQuery(const Point &min, const Point &max, const F &cb) { @@ -71,10 +57,10 @@ static TestKdTree g_test_instance; void TestKdTree::runTests(IGameDef *gamedef) { rawstream << "-------- k-d-tree" << std::endl; - TEST(randomInserts); + TEST(randomOps); } -void TestKdTree::randomInserts() { +void TestKdTree::randomOps() { PseudoRandom pr(814538); ObjectVector<3, f32, u16> objvec; @@ -87,21 +73,32 @@ void TestKdTree::randomInserts() { kds.insert(point, id); } - for (int i = 0; i < 1000; ++i) { - std::array min, max; - for (uint8_t d = 0; d < 3; ++d) { - min[d] = pr.range(-1500, 1500); - max[d] = min[d] + pr.range(1, 2500); + const auto testRandomQueries = [&]() { + for (int i = 0; i < 1000; ++i) { + std::array min, max; + for (uint8_t d = 0; d < 3; ++d) { + min[d] = pr.range(-1500, 1500); + max[d] = min[d] + pr.range(1, 2500); + } + std::unordered_set expected_ids; + objvec.rangeQuery(min, max, [&](auto _, u16 id) { + UASSERT(expected_ids.count(id) == 0); + expected_ids.insert(id); + }); + kds.rangeQuery(min, max, [&](auto point, u16 id) { + UASSERT(expected_ids.count(id) == 1); + expected_ids.erase(id); + }); + UASSERT(expected_ids.empty()); } - std::unordered_set expected_ids; - objvec.rangeQuery(min, max, [&](auto _, u16 id) { - UASSERT(expected_ids.count(id) == 0); - expected_ids.insert(id); - }); - kds.rangeQuery(min, max, [&](auto point, u16 id) { - UASSERT(expected_ids.count(id) == 1); - expected_ids.erase(id); - }); - UASSERT(expected_ids.empty()); + }; + + testRandomQueries(); + + for (u16 id = 1; id < 800; ++id) { + objvec.remove(id); + kds.remove(id); } + + testRandomQueries(); } \ No newline at end of file diff --git a/src/util/k_d_tree.h b/src/util/k_d_tree.h index 3f2972059..487cb903f 100644 --- a/src/util/k_d_tree.h +++ b/src/util/k_d_tree.h @@ -1,13 +1,22 @@ +// Copyright (C) 2024 Lars Müller +// SPDX-License-Identifier: LGPL-2.1-or-later + #pragma once #include +#include #include #include #include #include -#include -using Idx = std::size_t; +using Idx = uint16_t; + +// TODO docs and explanation + +// TODO profile and tweak knobs + +// TODO cleanup template class Points { @@ -221,14 +230,15 @@ class KdTree { } //! Build a tree + // FIXME something must be wrong here, otherwise deleting stuff would work KdTree(Idx n, Id const *ids, std::array pts) : items(n, pts) - , tree(std::make_unique(n)) , ids(std::make_unique(n)) + , tree(std::make_unique(n)) , deleted(n) { std::copy(ids, ids + n, this->ids.get()); - init(0, items); + init(0, 0, items.indices); } //! Merge two trees. Both trees are assumed to have a power of two size. @@ -245,29 +255,6 @@ class KdTree { init(0, 0, items.indices); } - // TODO remove dead code - /* SortedPoints alivePoints() { - const auto newIndices = std::make_unique(items.n); - Idx j = 0; - for (Idx i = 0; i < items.n; ++i) - newIndices[i] = deleted[i] ? j : j++; - Idx aliveN = std::count(deleted.begin(), deleted.end(), false); - SortedPoints res(aliveN); - for (uint8_t d = 0; d < Dim; ++d) { - const auto pts = &res.points[d * aliveN]; - const auto indices = &res.indices[d * aliveN]; - const auto items_pts = &items.points[d * items.n]; - const auto items_indices = &items.indices[d * aliveN]; - Idx j = 0; - for (Idx i = 0; i < items.n; ++i) { - if (deleted[i]) continue; - pts[j++] = items_pts[i]; - indices[newIndices[i]] = items_indices[i]; - } - } - return res; - } */ - template void rangeQuery(const Point &min, const Point &max, const F &cb) const { @@ -277,16 +264,15 @@ class KdTree { } void remove(Idx internalIdx) { + assert(!deleted[internalIdx]); deleted[internalIdx] = true; } template - void foreach(F cb) { - for (Idx i = 0; i < cap(); ++i) { - if (!deleted[i]) { + void foreach(F cb) const { + for (Idx i = 0; i < cap(); ++i) + if (!deleted[i]) cb(i, items.points.getPoint(i), ids[i]); - } - } } //! Capacity, not size, since some items may be marked as deleted @@ -329,7 +315,7 @@ class KdTree { } else { rangeQuery(rightChild, nextSplit, min, max, cb); rangeQuery(leftChild, nextSplit, min, max, cb); - if (deleted[root]) + if (deleted[ptid]) return; const auto point = items.points.getPoint(ptid); for (uint8_t d = 0; d < Dim; ++d) @@ -378,7 +364,7 @@ class DynamicKdTrees { void remove(const Id id) { const auto del_entry = del_entries.at(id); trees.at(del_entry.treeIdx).remove(del_entry.inTree); - del_entries.erase(del_entry); + del_entries.erase(id); // TODO use iterator right away... ++deleted; if (deleted > n_entries/2) // we want to shift out the one! compactify(); @@ -391,57 +377,59 @@ class DynamicKdTrees { } private: void compactify() { - n_entries -= deleted; + assert(n_entries >= deleted); + n_entries -= deleted; // note: this should be exactly n_entries/2 + deleted = 0; + // reset map, freeing memory (instead of clearing) del_entries = std::unordered_map(); + // Collect all live points and corresponding IDs. - const auto ids = std::make_unique(n_entries); - Points pts; + const auto live_ids = std::make_unique(n_entries); + Points live_points(n_entries); Idx i = 0; - for (const auto tree : trees) { - if (tree.empty()) - continue; - tree.foreach([&](Idx _, std::array point, Id id) { - for (uint8_t d = 0; d < Dim; ++d) - pts.get(d)[i] = point[d]; - ids[i] = id; + for (const auto &tree : trees) { + tree.foreach([&](Idx _, auto point, Id id) { + assert(i < n_entries); + live_points.setPoint(i, point); + live_ids[i] = id; ++i; }); } + assert(i == n_entries); // Construct a new forest. - // The pattern will be the current pattern, shifted down by one. - auto id_ptr = ids.get(); + // The "tree pattern" will effectively just be shifted down by one. + auto id_ptr = live_ids.get(); std::array point_ptrs; - std::vector newTrees(trees.size() - 1); Idx n = 1; for (uint8_t d = 0; d < Dim; ++d) - point_ptrs[d] = pts.begin(d); - for (uint8_t i = 0; i < newTrees.size(); ++i, n *= 2) { - if (trees[i+1].empty()) - continue; - - // TODO maybe optimize from log² -> log? - // This can be achieved by doing a sorted merge of live points, - // then doing a radix sort. - Tree tree(n, id_ptr, point_ptrs); - id_ptr += n; - for (uint8_t d = 0; d < Dim; ++d) - point_ptrs[d] += n; - tree.foreach([&](Idx i, auto _, Id id) { - del_entries[id] = {n, i}; - }); - newTrees[i] = std::move(tree); + point_ptrs[d] = live_points.begin(d); + for (uint8_t treeIdx = 0; treeIdx < trees.size() - 1; ++treeIdx, n *= 2) { + Tree tree; + if (!trees[treeIdx+1].empty()) { + // TODO maybe optimize from log² -> log? + // This could be achieved by doing a sorted merge of live points, then doing a radix sort. + tree = std::move(Tree(n, id_ptr, point_ptrs)); + id_ptr += n; + for (uint8_t d = 0; d < Dim; ++d) + point_ptrs[d] += n; + // TODO dedupe + tree.foreach([&](Idx objIdx, auto _, Id id) { + del_entries[id] = {treeIdx, objIdx}; + }); + } + trees[treeIdx] = std::move(tree); } - trees = std::move(newTrees); + trees.pop_back(); // "shift out" tree with the most elements } - // could use an array here since we've got a good bound on the size ahead of time but meh + // could use an array (rather than a vector) here since we've got a good bound on the size ahead of time but meh std::vector trees; struct DelEntry { uint8_t treeIdx; Idx inTree; }; std::unordered_map del_entries; - Idx n_entries; - Idx deleted; + Idx n_entries = 0; + Idx deleted = 0; }; \ No newline at end of file