Simplified data VoxelDataMap structure

storage/voxel_data_block.cpp

@@ -7,7 +7,7 @@ namespace zylann::voxel {
 void VoxelDataBlock::set_modified(bool modified) {
 #ifdef TOOLS_ENABLED
 	if (_modified == false && modified) {
-		ZN_PRINT_VERBOSE(format("Marking block {} as modified", position));
+		ZN_PRINT_VERBOSE(format("Marking block {} as modified", _position));
 	}
 #endif
 	_modified = modified;

storage/voxel_data_block.h

@@ -11,15 +11,39 @@ namespace zylann::voxel {
 // Stores loaded voxel data for a chunk of the volume. Mesh and colliders are stored separately.
 class VoxelDataBlock {
 public:
-	const Vector3i position;
-	const unsigned int lod_index = 0;
 	RefCount viewers;
 
-	static VoxelDataBlock *create(
+	VoxelDataBlock() {}
-			Vector3i bpos, std::shared_ptr<VoxelBufferInternal> &buffer, unsigned int size, unsigned int p_lod_index) {
+
-		ERR_FAIL_COND_V(buffer == nullptr, nullptr);
+	VoxelDataBlock(Vector3i bpos, std::shared_ptr<VoxelBufferInternal> &buffer, unsigned int p_lod_index) :
-		ERR_FAIL_COND_V(buffer->get_size() != Vector3i(size, size, size), nullptr);
+			_position(bpos), _lod_index(p_lod_index), _voxels(buffer) {}
-		return memnew(VoxelDataBlock(bpos, buffer, p_lod_index));
+
+	VoxelDataBlock(VoxelDataBlock &&src) :
+			viewers(src.viewers),
+			_position(src._position),
+			_lod_index(src._lod_index),
+			_voxels(std::move(src._voxels)),
+			_needs_lodding(src._needs_lodding),
+			_modified(src._modified),
+			_edited(src._edited) {}
+
+	VoxelDataBlock &operator=(VoxelDataBlock &&src) {
+		viewers = src.viewers;
+		_position = src._position;
+		_lod_index = src._lod_index;
+		_voxels = std::move(src._voxels);
+		_needs_lodding = src._needs_lodding;
+		_modified = src._modified;
+		_edited = src._edited;
+		return *this;
+	}
+
+	inline const Vector3i &get_position() const {
+		return _position;
+	}
+
+	inline unsigned int get_lod_index() const {
+		return _lod_index;
 	}
 
 	VoxelBufferInternal &get_voxels() {
@@ -71,11 +95,12 @@ public:
 	}
 
 private:
-	VoxelDataBlock(Vector3i bpos, std::shared_ptr<VoxelBufferInternal> &buffer, unsigned int p_lod_index) :
+	Vector3i _position;
-			position(bpos), lod_index(p_lod_index), _voxels(buffer) {}
 
 	std::shared_ptr<VoxelBufferInternal> _voxels;
 
+	uint8_t _lod_index = 0;
+
 	// The block was edited, which requires its LOD counterparts to be recomputed
 	bool _needs_lodding = false;
 

storage/voxel_data_map.cpp

@@ -65,9 +65,12 @@ VoxelDataBlock *VoxelDataMap::create_default_block(Vector3i bpos) {
 	std::shared_ptr<VoxelBufferInternal> buffer = make_shared_instance<VoxelBufferInternal>();
 	buffer->create(_block_size, _block_size, _block_size);
 	buffer->set_default_values(_default_voxel);
-	VoxelDataBlock *block = VoxelDataBlock::create(bpos, buffer, _block_size, _lod_index);
+#ifdef DEBUG_ENABLED
-	set_block(bpos, block);
+	ZN_ASSERT_RETURN_V(!has_block(bpos), nullptr);
-	return block;
+#endif
+	VoxelDataBlock &map_block = _blocks_map[bpos];
+	map_block = std::move(VoxelDataBlock(bpos, buffer, _lod_index));
+	return &map_block;
 }
 
 VoxelDataBlock *VoxelDataMap::get_or_create_block_at_voxel_pos(Vector3i pos) {
@@ -120,13 +123,7 @@ int VoxelDataMap::get_default_voxel(unsigned int channel) {
 VoxelDataBlock *VoxelDataMap::get_block(Vector3i bpos) {
 	auto it = _blocks_map.find(bpos);
 	if (it != _blocks_map.end()) {
-		const unsigned int i = it->second;
+		return &it->second;
-#ifdef DEBUG_ENABLED
-		CRASH_COND(i >= _blocks.size());
-#endif
-		VoxelDataBlock *block = _blocks[i];
-		CRASH_COND(block == nullptr); // The map should not contain null blocks
-		return block;
 	}
 	return nullptr;
 }
@@ -134,66 +131,31 @@ VoxelDataBlock *VoxelDataMap::get_block(Vector3i bpos) {
 const VoxelDataBlock *VoxelDataMap::get_block(Vector3i bpos) const {
 	auto it = _blocks_map.find(bpos);
 	if (it != _blocks_map.end()) {
-		const unsigned int i = it->second;
+		return &it->second;
-#ifdef DEBUG_ENABLED
-		CRASH_COND(i >= _blocks.size());
-#endif
-		// TODO This function can't cache _last_accessed_block, because it's const, so repeated accesses are hashing
-		// again...
-		const VoxelDataBlock *block = _blocks[i];
-		CRASH_COND(block == nullptr); // The map should not contain null blocks
-		return block;
 	}
 	return nullptr;
 }
 
-void VoxelDataMap::set_block(Vector3i bpos, VoxelDataBlock *block) {
-	ERR_FAIL_COND(block == nullptr);
-	CRASH_COND(bpos != block->position);
-#ifdef DEBUG_ENABLED
-	CRASH_COND(_blocks_map.find(bpos) != _blocks_map.end());
-#endif
-	unsigned int i = _blocks.size();
-	_blocks.push_back(block);
-	_blocks_map.insert(std::make_pair(bpos, i));
-}
-
-void VoxelDataMap::remove_block_internal(Vector3i bpos, unsigned int index) {
-	// TODO `erase` can occasionally be very slow (milliseconds) if the map contains lots of items.
-	// This might be caused by internal rehashing/resizing.
-	// We should look for a faster container, or reduce the number of entries.
-
-	// This function assumes the block is already freed
-	_blocks_map.erase(bpos);
-
-	VoxelDataBlock *moved_block = _blocks.back();
-#ifdef DEBUG_ENABLED
-	CRASH_COND(index >= _blocks.size());
-#endif
-	_blocks[index] = moved_block;
-	_blocks.pop_back();
-
-	if (index < _blocks.size()) {
-		auto it = _blocks_map.find(moved_block->position);
-		CRASH_COND(it == _blocks_map.end());
-		it->second = index;
-	}
-}
-
 VoxelDataBlock *VoxelDataMap::set_block_buffer(
 		Vector3i bpos, std::shared_ptr<VoxelBufferInternal> &buffer, bool overwrite) {
 	ERR_FAIL_COND_V(buffer == nullptr, nullptr);
+
 	VoxelDataBlock *block = get_block(bpos);
+
 	if (block == nullptr) {
-		block = VoxelDataBlock::create(bpos, buffer, _block_size, _lod_index);
+		VoxelDataBlock &map_block = _blocks_map[bpos];
-		set_block(bpos, block);
+		map_block = std::move(VoxelDataBlock(bpos, buffer, _lod_index));
+		block = &map_block;
+
 	} else if (overwrite) {
 		block->set_voxels(buffer);
+
 	} else {
 		ZN_PROFILE_MESSAGE("Redundant data block");
 		ZN_PRINT_VERBOSE(format(
 				"Discarded block {} lod {}, there was already data and overwriting is not enabled", bpos, _lod_index));
 	}
+
 	return block;
 }
 
@@ -328,29 +290,18 @@ void VoxelDataMap::paste(Vector3i min_pos, VoxelBufferInternal &src_buffer, unsi
 }
 
 void VoxelDataMap::clear() {
-	for (auto it = _blocks.begin(); it != _blocks.end(); ++it) {
-		VoxelDataBlock *block = *it;
-		if (block == nullptr) {
-			ERR_PRINT("Unexpected nullptr in VoxelMap::clear()");
-		} else {
-			memdelete(block);
-		}
-	}
-	_blocks.clear();
 	_blocks_map.clear();
 }
 
 int VoxelDataMap::get_block_count() const {
-#ifdef DEBUG_ENABLED
+	return _blocks_map.size();
-	const unsigned int blocks_map_size = _blocks_map.size();
-	CRASH_COND(_blocks.size() != blocks_map_size);
-#endif
-	return _blocks.size();
 }
 
 bool VoxelDataMap::is_area_fully_loaded(const Box3i voxels_box) const {
 	Box3i block_box = voxels_box.downscaled(get_block_size());
-	return block_box.all_cells_match([this](Vector3i pos) { return has_block(pos); });
+	return block_box.all_cells_match([this](Vector3i pos) { //
+		return has_block(pos);
+	});
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

storage/voxel_data_map.h

@@ -84,15 +84,8 @@ public:
 	void remove_block(Vector3i bpos, Action_T pre_delete) {
 		auto it = _blocks_map.find(bpos);
 		if (it != _blocks_map.end()) {
-			const unsigned int i = it->second;
+			pre_delete(it->second);
-#ifdef DEBUG_ENABLED
+			_blocks_map.erase(it);
-			CRASH_COND(i >= _blocks.size());
-#endif
-			VoxelDataBlock *block = _blocks[i];
-			ERR_FAIL_COND(block == nullptr);
-			pre_delete(*block);
-			memdelete(block);
-			remove_block_internal(bpos, i);
 		}
 	}
 
@@ -108,23 +101,15 @@ public:
 
 	template <typename Op_T>
 	inline void for_each_block(Op_T op) {
-		for (auto it = _blocks.begin(); it != _blocks.end(); ++it) {
+		for (auto it = _blocks_map.begin(); it != _blocks_map.end(); ++it) {
-			VoxelDataBlock *block = *it;
+			op(it->second);
-#ifdef DEBUG_ENABLED
-			CRASH_COND(block == nullptr);
-#endif
-			op(*block);
 		}
 	}
 
 	template <typename Op_T>
 	inline void for_each_block(Op_T op) const {
-		for (auto it = _blocks.begin(); it != _blocks.end(); ++it) {
+		for (auto it = _blocks_map.begin(); it != _blocks_map.end(); ++it) {
-			const VoxelDataBlock *block = *it;
+			op(it->second);
-#ifdef DEBUG_ENABLED
-			CRASH_COND(block == nullptr);
-#endif
-			op(*block);
 		}
 	}
 
@@ -182,10 +167,9 @@ public:
 	}
 
 private:
-	void set_block(Vector3i bpos, VoxelDataBlock *block);
+	//void set_block(Vector3i bpos, VoxelDataBlock *block);
 	VoxelDataBlock *get_or_create_block_at_voxel_pos(Vector3i pos);
 	VoxelDataBlock *create_default_block(Vector3i bpos);
-	void remove_block_internal(Vector3i bpos, unsigned int index);
 
 	void set_block_size_pow2(unsigned int p);
 
@@ -194,11 +178,11 @@ private:
 	FixedArray<uint64_t, VoxelBufferInternal::MAX_CHANNELS> _default_voxel;
 
 	// Blocks stored with a spatial hash in all 3D directions.
+	// This is dual storage: map and vector, for fast lookup and also fast iteration.
 	// Before I used Godot's HashMap with RELATIONSHIP = 2 because that delivers better performance compared to
 	// defaults, but it sometimes has very long stalls on removal, which std::unordered_map doesn't seem to have
 	// (not as badly). Also overall performance is slightly better.
-	std::unordered_map<Vector3i, unsigned int> _blocks_map;
+	std::unordered_map<Vector3i, VoxelDataBlock> _blocks_map;
-	std::vector<VoxelDataBlock *> _blocks;
 
 	// This was a possible optimization in a single-threaded scenario, but it's not in multithread.
 	// We want to be able to do shared read-accesses but this is a mutable variable.

terrain/fixed_lod/voxel_terrain.cpp

@@ -582,7 +582,7 @@ struct ScheduleSaveAction {
 			} else {
 				b.voxels = block.get_voxels_shared();
 			}
-			b.position = block.position;
+			b.position = block.get_position();
 			blocks_to_save.push_back(b);
 			block.set_modified(false);
 		}
@@ -1018,13 +1018,13 @@ void VoxelTerrain::emit_data_block_loaded(const VoxelDataBlock &block) {
 	// absolutely necessary, buffers aren't exposed. Workaround: use VoxelTool
 	//const Variant vbuffer = block->voxels;
 	//const Variant *args[2] = { &vpos, &vbuffer };
-	emit_signal(VoxelStringNames::get_singleton().block_loaded, block.position);
+	emit_signal(VoxelStringNames::get_singleton().block_loaded, block.get_position());
 }
 
 void VoxelTerrain::emit_data_block_unloaded(const VoxelDataBlock &block) {
 	// const Variant vbuffer = block->voxels;
 	// const Variant *args[2] = { &vpos, &vbuffer };
-	emit_signal(VoxelStringNames::get_singleton().block_unloaded, block.position);
+	emit_signal(VoxelStringNames::get_singleton().block_unloaded, block.get_position());
 }
 
 bool VoxelTerrain::try_get_paired_viewer_index(uint32_t id, size_t &out_i) const {

terrain/variable_lod/voxel_lod_terrain.cpp

@@ -111,8 +111,8 @@ struct ScheduleSaveAction {
 				block.get_voxels_const().duplicate_to(*b.voxels, true);
 			}
 
-			b.position = block.position;
+			b.position = block.get_position();
-			b.lod = block.lod_index;
+			b.lod = block.get_lod_index();
 			blocks_to_save.push_back(b);
 			block.set_modified(false);
 		}
@@ -2086,7 +2086,7 @@ void VoxelLodTerrain::update_gizmos() {
 
 		RWLockRead rlock(data_lod.map_lock);
 		data_lod.map.for_each_block([&dr, parent_transform, data_block_size, basis](const VoxelDataBlock &block) {
-			const Transform3D local_transform(basis, block.position * data_block_size);
+			const Transform3D local_transform(basis, block.get_position() * data_block_size);
 			const Transform3D t = parent_transform * local_transform;
 			const Color8 c = Color8(block.is_modified() ? 255 : 0, 255, 0, 255);
 			dr.draw_box_mm(t, c);

terrain/variable_lod/voxel_lod_terrain_update_task.cpp

@@ -176,8 +176,8 @@ struct BeforeUnloadDataAction {
 			VoxelLodTerrainUpdateData::BlockToSave b;
 			// We don't copy since the block will be unloaded anyways
 			b.voxels = block.get_voxels_shared();
-			b.position = block.position;
+			b.position = block.get_position();
-			b.lod = block.lod_index;
+			b.lod = block.get_lod_index();
 			blocks_to_save.push_back(b);
 		}
 	}

terrain/voxel_data_block_enter_info.cpp

@@ -17,12 +17,12 @@ Ref<gd::VoxelBuffer> VoxelDataBlockEnterInfo::_b_get_voxels() const {
 
 Vector3i VoxelDataBlockEnterInfo::_b_get_position() const {
 	ERR_FAIL_COND_V(voxel_block == nullptr, Vector3i());
-	return voxel_block->position;
+	return voxel_block->get_position();
 }
 
 int VoxelDataBlockEnterInfo::_b_get_lod_index() const {
 	ERR_FAIL_COND_V(voxel_block == nullptr, 0);
-	return voxel_block->lod_index;
+	return voxel_block->get_lod_index();
 }
 
 bool VoxelDataBlockEnterInfo::_b_are_voxels_edited() const {