Add spatial index for objects (#14631)

src/unittest/CMakeLists.txt

@@ -10,6 +10,7 @@ set (UNITTEST_SRCS
 	${CMAKE_CURRENT_SOURCE_DIR}/test_connection.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_craft.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_datastructures.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/test_k_d_tree.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_filesys.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_inventory.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_irrptr.cpp

src/unittest/test_k_d_tree.cpp

@@ -0,0 +1,138 @@
+// Copyright (C) 2024 Lars Müller
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+#include "catch.h"
+#include "irrTypes.h"
+#include "noise.h"
+#include "util/k_d_tree.h"
+
+#include <algorithm>
+#include <unordered_set>
+
+template<uint8_t Dim, typename Component, typename Id>
+class ObjectVector
+{
+public:
+	using Point = std::array<Component, Dim>;
+
+	void insert(const Point &p, Id id)
+	{
+		entries.push_back(Entry{p, id});
+	}
+
+	void remove(Id id)
+	{
+		const auto it = std::find_if(entries.begin(), entries.end(), [&](const auto &e) {
+			return e.id == id;
+		});
+		assert(it != entries.end());
+		entries.erase(it);
+	}
+
+	void update(const Point &p, Id id)
+	{
+		remove(id);
+		insert(p, id);
+	}
+
+	template<typename F>
+	void rangeQuery(const Point &min, const Point &max, const F &cb)
+	{
+		for (const auto &e : entries) {
+			for (uint8_t d = 0; d < Dim; ++d)
+				if (e.point[d] < min[d] || e.point[d] > max[d])
+					goto next;
+			cb(e.point, e.id); // TODO check
+			next: {}
+		}
+	}
+
+private:
+	struct Entry {
+		Point point;
+		Id id;
+	};
+	std::vector<Entry> entries;
+};
+
+TEST_CASE("k-d-tree") {
+
+SECTION("single update") {
+	k_d_tree::DynamicKdTrees<3, u16, u16> kds;
+	for (u16 i = 1; i <= 5; ++i)
+		kds.insert({i, i, i}, i);
+	for (u16 i = 1; i <= 5; ++i) {
+		u16 j = i - 1;
+		kds.update({j, j, j}, i);
+	}
+}
+
+SECTION("random operations") {
+	PseudoRandom pr(Catch::getSeed());
+
+	ObjectVector<3, f32, u16> objvec;
+	k_d_tree::DynamicKdTrees<3, f32, u16> kds;
+
+	const auto randPos = [&]() {
+		std::array<f32, 3> point;
+		for (uint8_t d = 0; d < 3; ++d)
+			point[d] = pr.range(-1000, 1000);
+		return point;
+	};
+
+	const auto testRandomQuery = [&]() {
+		std::array<f32, 3> 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<u16> expected_ids;
+		objvec.rangeQuery(min, max, [&](auto _, u16 id) {
+			CHECK(expected_ids.count(id) == 0);
+			expected_ids.insert(id);
+		});
+		kds.rangeQuery(min, max, [&](auto point, u16 id) {
+			CHECK(expected_ids.count(id) == 1);
+			expected_ids.erase(id);
+		});
+		CHECK(expected_ids.empty());
+	};
+
+	for (u16 id = 1; id < 1000; ++id) {
+		const auto point = randPos();
+		objvec.insert(point, id);
+		kds.insert(point, id);
+		testRandomQuery();
+	}
+
+	const auto testRandomQueries = [&]() {
+		for (int i = 0; i < 1000; ++i) {
+			testRandomQuery();
+		}
+	};
+
+	testRandomQueries();
+
+	for (u16 id = 1; id < 800; ++id) {
+		objvec.remove(id);
+		kds.remove(id);
+	}
+
+	testRandomQueries();
+
+	for (u16 id = 800; id < 1000; ++id) {
+		const auto point = randPos();
+		objvec.update(point, id);
+		kds.update(point, id);
+	}
+
+	testRandomQueries();
+
+	for (u16 id = 800; id < 1000; ++id) {
+		objvec.remove(id);
+		kds.remove(id);
+		testRandomQuery();
+	}
+}
+
+}

src/unittest/test_serveractiveobjectmgr.cpp

@@ -2,52 +2,162 @@
 // SPDX-License-Identifier: LGPL-2.1-or-later
 // Copyright (C) 2018 nerzhul, Loic Blot <loic.blot@unix-experience.fr>
 
-#include "test.h"
+#include "activeobjectmgr.h"
+#include "catch.h"
+#include "irrTypes.h"
+#include "irr_aabb3d.h"
 #include "mock_serveractiveobject.h"
 #include <algorithm>
-#include <queue>
+#include <iterator>
+#include <random>
+#include <utility>
 
 #include "server/activeobjectmgr.h"
+#include "server/serveractiveobject.h"
 
-#include "profiler.h"
+class TestServerActiveObjectMgr {
+	server::ActiveObjectMgr saomgr;
+	std::vector<u16> ids;
 
-
-class TestServerActiveObjectMgr : public TestBase
-{
 public:
-	TestServerActiveObjectMgr() { TestManager::registerTestModule(this); }
-	const char *getName() { return "TestServerActiveObjectMgr"; }
 
-	void runTests(IGameDef *gamedef);
+	u16 getFreeId() const { return saomgr.getFreeId(); }
 
-	void testFreeID();
-	void testRegisterObject();
-	void testRemoveObject();
-	void testGetObjectsInsideRadius();
-	void testGetAddedActiveObjectsAroundPos();
+	bool registerObject(std::unique_ptr<ServerActiveObject> obj)
+	{
+		auto *ptr = obj.get();
+		if (!saomgr.registerObject(std::move(obj)))
+			return false;
+		ids.push_back(ptr->getId());
+		return true;
+	}
+
+	void removeObject(u16 id)
+	{
+		const auto it = std::find(ids.begin(), ids.end(), id);
+		REQUIRE(it != ids.end());
+		ids.erase(it);
+		saomgr.removeObject(id);
+	}
+
+	void updateObjectPos(u16 id, const v3f &pos)
+	{
+		auto *obj = saomgr.getActiveObject(id);
+		REQUIRE(obj != nullptr);
+		obj->setPos(pos);
+		saomgr.updateObjectPos(id, pos); // HACK work around m_env == nullptr
+	}
+
+	void clear()
+	{
+		saomgr.clear();
+		ids.clear();
+	}
+
+	ServerActiveObject *getActiveObject(u16 id)
+	{
+		return saomgr.getActiveObject(id);
+	}
+
+	template<class T>
+	void getObjectsInsideRadius(T&& arg)
+	{
+		saomgr.getObjectsInsideRadius(std::forward(arg));
+	}
+
+	template<class T>
+	void getAddedActiveObjectsAroundPos(T&& arg)
+	{
+		saomgr.getAddedActiveObjectsAroundPos(std::forward(arg));
+	}
+
+	// Testing
+
+	bool empty() { return ids.empty(); }
+
+	template<class T>
+	u16 randomId(T &random)
+	{
+		REQUIRE(!ids.empty());
+		std::uniform_int_distribution<u16> index(0, ids.size() - 1);
+		return ids[index(random)];
+	}
+
+	void getObjectsInsideRadiusNaive(const v3f &pos, float radius,
+			std::vector<ServerActiveObject *> &result)
+	{
+		for (const auto &[id, obj] : saomgr.m_active_objects.iter()) {
+			if (obj->getBasePosition().getDistanceFromSQ(pos) <= radius * radius) {
+				result.push_back(obj.get());
+			}
+		}
+	}
+
+	void getObjectsInAreaNaive(const aabb3f &box,
+			std::vector<ServerActiveObject *> &result)
+	{
+		for (const auto &[id, obj] : saomgr.m_active_objects.iter()) {
+			if (box.isPointInside(obj->getBasePosition())) {
+				result.push_back(obj.get());
+			}
+		}
+	}
+
+	constexpr static auto compare_by_id = [](auto *sao1, auto *sao2) -> bool {
+		return sao1->getId() < sao2->getId();
+	};
+
+	static void sortById(std::vector<ServerActiveObject *> &saos)
+	{
+		std::sort(saos.begin(), saos.end(), compare_by_id);
+	}
+
+	void compareObjects(std::vector<ServerActiveObject *> &actual,
+			std::vector<ServerActiveObject *> &expected)
+	{
+		std::vector<ServerActiveObject *> unexpected, missing;
+		sortById(actual);
+		sortById(expected);
+
+		std::set_difference(actual.begin(), actual.end(),
+			expected.begin(), expected.end(),
+			std::back_inserter(unexpected), compare_by_id);
+
+		assert(unexpected.empty());
+
+		std::set_difference(expected.begin(), expected.end(),
+			actual.begin(), actual.end(),
+			std::back_inserter(missing), compare_by_id);
+		assert(missing.empty());
+	}
+
+	void compareObjectsInsideRadius(const v3f &pos, float radius)
+	{
+		std::vector<ServerActiveObject *> actual, expected;
+		saomgr.getObjectsInsideRadius(pos, radius, actual, nullptr);
+		getObjectsInsideRadiusNaive(pos, radius, expected);
+		compareObjects(actual, expected);
+	}
+
+	void compareObjectsInArea(const aabb3f &box)
+	{
+		std::vector<ServerActiveObject *> actual, expected;
+		saomgr.getObjectsInArea(box, actual, nullptr);
+		getObjectsInAreaNaive(box, expected);
+		compareObjects(actual, expected);
+	}
 };
 
-static TestServerActiveObjectMgr g_test_instance;
 
-void TestServerActiveObjectMgr::runTests(IGameDef *gamedef)
-{
-	TEST(testFreeID);
-	TEST(testRegisterObject)
-	TEST(testRemoveObject)
-	TEST(testGetObjectsInsideRadius);
-	TEST(testGetAddedActiveObjectsAroundPos);
-}
+TEST_CASE("server active object manager") {
 
-////////////////////////////////////////////////////////////////////////////////
-
-void TestServerActiveObjectMgr::testFreeID()
-{
-	server::ActiveObjectMgr saomgr;
+SECTION("free ID") {
+	TestServerActiveObjectMgr saomgr;
 	std::vector<u16> aoids;
 
 	u16 aoid = saomgr.getFreeId();
 	// Ensure it's not the same id
-	UASSERT(saomgr.getFreeId() != aoid);
+	REQUIRE(saomgr.getFreeId() != aoid);
 
 	aoids.push_back(aoid);
 
@@ -60,53 +170,50 @@ void TestServerActiveObjectMgr::testFreeID()
 		aoids.push_back(sao->getId());
 
 		// Ensure next id is not in registered list
-		UASSERT(std::find(aoids.begin(), aoids.end(), saomgr.getFreeId()) ==
+		REQUIRE(std::find(aoids.begin(), aoids.end(), saomgr.getFreeId()) ==
 				aoids.end());
 	}
 
 	saomgr.clear();
 }
 
-void TestServerActiveObjectMgr::testRegisterObject()
-{
-	server::ActiveObjectMgr saomgr;
+SECTION("register object") {
+	TestServerActiveObjectMgr saomgr;
 	auto sao_u = std::make_unique<MockServerActiveObject>();
 	auto sao = sao_u.get();
-	UASSERT(saomgr.registerObject(std::move(sao_u)));
+	REQUIRE(saomgr.registerObject(std::move(sao_u)));
 
 	u16 id = sao->getId();
 
 	auto saoToCompare = saomgr.getActiveObject(id);
-	UASSERT(saoToCompare->getId() == id);
-	UASSERT(saoToCompare == sao);
+	REQUIRE(saoToCompare->getId() == id);
+	REQUIRE(saoToCompare == sao);
 
 	sao_u = std::make_unique<MockServerActiveObject>();
 	sao = sao_u.get();
-	UASSERT(saomgr.registerObject(std::move(sao_u)));
-	UASSERT(saomgr.getActiveObject(sao->getId()) == sao);
-	UASSERT(saomgr.getActiveObject(sao->getId()) != saoToCompare);
+	REQUIRE(saomgr.registerObject(std::move(sao_u)));
+	REQUIRE(saomgr.getActiveObject(sao->getId()) == sao);
+	REQUIRE(saomgr.getActiveObject(sao->getId()) != saoToCompare);
 
 	saomgr.clear();
 }
 
-void TestServerActiveObjectMgr::testRemoveObject()
-{
-	server::ActiveObjectMgr saomgr;
+SECTION("remove object") {
+	TestServerActiveObjectMgr saomgr;
 	auto sao_u = std::make_unique<MockServerActiveObject>();
 	auto sao = sao_u.get();
-	UASSERT(saomgr.registerObject(std::move(sao_u)));
+	REQUIRE(saomgr.registerObject(std::move(sao_u)));
 
 	u16 id = sao->getId();
-	UASSERT(saomgr.getActiveObject(id) != nullptr)
+	REQUIRE(saomgr.getActiveObject(id) != nullptr);
 
 	saomgr.removeObject(sao->getId());
-	UASSERT(saomgr.getActiveObject(id) == nullptr);
+	REQUIRE(saomgr.getActiveObject(id) == nullptr);
 
 	saomgr.clear();
 }
 
-void TestServerActiveObjectMgr::testGetObjectsInsideRadius()
-{
+SECTION("get objects inside radius") {
 	server::ActiveObjectMgr saomgr;
 	static const v3f sao_pos[] = {
 			v3f(10, 40, 10),
@@ -122,15 +229,15 @@ void TestServerActiveObjectMgr::testGetObjectsInsideRadius()
 
 	std::vector<ServerActiveObject *> result;
 	saomgr.getObjectsInsideRadius(v3f(), 50, result, nullptr);
-	UASSERTCMP(int, ==, result.size(), 1);
+	CHECK(result.size() == 1);
 
 	result.clear();
 	saomgr.getObjectsInsideRadius(v3f(), 750, result, nullptr);
-	UASSERTCMP(int, ==, result.size(), 2);
+	CHECK(result.size() == 2);
 
 	result.clear();
 	saomgr.getObjectsInsideRadius(v3f(), 750000, result, nullptr);
-	UASSERTCMP(int, ==, result.size(), 5);
+	CHECK(result.size() == 5);
 
 	result.clear();
 	auto include_obj_cb = [](ServerActiveObject *obj) {
@@ -138,13 +245,12 @@ void TestServerActiveObjectMgr::testGetObjectsInsideRadius()
 	};
 
 	saomgr.getObjectsInsideRadius(v3f(), 750000, result, include_obj_cb);
-	UASSERTCMP(int, ==, result.size(), 4);
+	CHECK(result.size() == 4);
 
 	saomgr.clear();
 }
 
-void TestServerActiveObjectMgr::testGetAddedActiveObjectsAroundPos()
-{
+SECTION("get added active objects around pos") {
 	server::ActiveObjectMgr saomgr;
 	static const v3f sao_pos[] = {
 			v3f(10, 40, 10),
@@ -161,12 +267,64 @@ void TestServerActiveObjectMgr::testGetAddedActiveObjectsAroundPos()
 	std::vector<u16> result;
 	std::set<u16> cur_objects;
 	saomgr.getAddedActiveObjectsAroundPos(v3f(), "singleplayer", 100, 50, cur_objects, result);
-	UASSERTCMP(int, ==, result.size(), 1);
+	CHECK(result.size() == 1);
 
 	result.clear();
 	cur_objects.clear();
 	saomgr.getAddedActiveObjectsAroundPos(v3f(), "singleplayer", 740, 50, cur_objects, result);
-	UASSERTCMP(int, ==, result.size(), 2);
+	CHECK(result.size() == 2);
 
 	saomgr.clear();
 }
+
+SECTION("spatial index") {
+	TestServerActiveObjectMgr saomgr;
+	std::mt19937 gen(0xABCDEF);
+	std::uniform_int_distribution<s32> coordinate(-1000, 1000);
+	const auto random_pos = [&]() {
+		return v3f(coordinate(gen), coordinate(gen), coordinate(gen));
+	};
+
+	std::uniform_int_distribution<u32> percent(0, 99);
+	const auto modify = [&](u32 p_insert, u32 p_delete, u32 p_update) {
+		const auto p = percent(gen);
+		if (p < p_insert) {
+			saomgr.registerObject(std::make_unique<MockServerActiveObject>(nullptr, random_pos()));
+		} else if (p < p_insert + p_delete) {
+			if (!saomgr.empty())
+				saomgr.removeObject(saomgr.randomId(gen));
+		} else if (p < p_insert + p_delete + p_update) {
+			if (!saomgr.empty())
+				saomgr.updateObjectPos(saomgr.randomId(gen), random_pos());
+		}
+	};
+
+	const auto test_queries = [&]() {
+		std::uniform_real_distribution<f32> radius(0, 100);
+		saomgr.compareObjectsInsideRadius(random_pos(), radius(gen));
+
+		aabb3f box(random_pos(), random_pos());
+		box.repair();
+		saomgr.compareObjectsInArea(box);
+	};
+
+	// Grow: Insertion twice as likely as deletion
+	for (u32 i = 0; i < 3000; ++i) {
+		modify(50, 25, 25);
+		test_queries();
+	}
+
+	// Stagnate: Insertion and deletion equally likely
+	for (u32 i = 0; i < 3000; ++i) {
+		modify(25, 25, 50);
+		test_queries();
+	}
+
+	// Shrink: Deletion twice as likely as insertion
+	while (!saomgr.empty()) {
+		modify(25, 50, 25);
+		test_queries();
+	}
+}
+
+}