diff --git a/storage/voxel_data_block.cpp b/storage/voxel_data_block.cpp index e437610d..d5d5b113 100644 --- a/storage/voxel_data_block.cpp +++ b/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; diff --git a/storage/voxel_data_block.h b/storage/voxel_data_block.h index 3a100c6c..a8e8d008 100644 --- a/storage/voxel_data_block.h +++ b/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( - Vector3i bpos, std::shared_ptr &buffer, unsigned int size, unsigned int p_lod_index) { - ERR_FAIL_COND_V(buffer == nullptr, nullptr); - ERR_FAIL_COND_V(buffer->get_size() != Vector3i(size, size, size), nullptr); - return memnew(VoxelDataBlock(bpos, buffer, p_lod_index)); + VoxelDataBlock() {} + + VoxelDataBlock(Vector3i bpos, std::shared_ptr &buffer, unsigned int p_lod_index) : + _position(bpos), _lod_index(p_lod_index), _voxels(buffer) {} + + 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 &buffer, unsigned int p_lod_index) : - position(bpos), lod_index(p_lod_index), _voxels(buffer) {} + Vector3i _position; std::shared_ptr _voxels; + uint8_t _lod_index = 0; + // The block was edited, which requires its LOD counterparts to be recomputed bool _needs_lodding = false; diff --git a/storage/voxel_data_map.cpp b/storage/voxel_data_map.cpp index bbd93e31..64f76fe4 100644 --- a/storage/voxel_data_map.cpp +++ b/storage/voxel_data_map.cpp @@ -65,9 +65,12 @@ VoxelDataBlock *VoxelDataMap::create_default_block(Vector3i bpos) { std::shared_ptr buffer = make_shared_instance(); buffer->create(_block_size, _block_size, _block_size); buffer->set_default_values(_default_voxel); - VoxelDataBlock *block = VoxelDataBlock::create(bpos, buffer, _block_size, _lod_index); - set_block(bpos, block); - return block; +#ifdef DEBUG_ENABLED + ZN_ASSERT_RETURN_V(!has_block(bpos), nullptr); +#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; -#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 &it->second; } 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; -#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 &it->second; } 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 &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); - set_block(bpos, block); + VoxelDataBlock &map_block = _blocks_map[bpos]; + 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 - const unsigned int blocks_map_size = _blocks_map.size(); - CRASH_COND(_blocks.size() != blocks_map_size); -#endif - return _blocks.size(); + return _blocks_map.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); + }); } //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/storage/voxel_data_map.h b/storage/voxel_data_map.h index 1640e575..097bbc16 100644 --- a/storage/voxel_data_map.h +++ b/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; -#ifdef DEBUG_ENABLED - 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); + pre_delete(it->second); + _blocks_map.erase(it); } } @@ -108,23 +101,15 @@ public: template inline void for_each_block(Op_T op) { - for (auto it = _blocks.begin(); it != _blocks.end(); ++it) { - VoxelDataBlock *block = *it; -#ifdef DEBUG_ENABLED - CRASH_COND(block == nullptr); -#endif - op(*block); + for (auto it = _blocks_map.begin(); it != _blocks_map.end(); ++it) { + op(it->second); } } template inline void for_each_block(Op_T op) const { - for (auto it = _blocks.begin(); it != _blocks.end(); ++it) { - const VoxelDataBlock *block = *it; -#ifdef DEBUG_ENABLED - CRASH_COND(block == nullptr); -#endif - op(*block); + for (auto it = _blocks_map.begin(); it != _blocks_map.end(); ++it) { + op(it->second); } } @@ -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 _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 _blocks_map; - std::vector _blocks; + std::unordered_map _blocks_map; // 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. diff --git a/terrain/fixed_lod/voxel_terrain.cpp b/terrain/fixed_lod/voxel_terrain.cpp index 0252563c..5451fcd2 100644 --- a/terrain/fixed_lod/voxel_terrain.cpp +++ b/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 { diff --git a/terrain/variable_lod/voxel_lod_terrain.cpp b/terrain/variable_lod/voxel_lod_terrain.cpp index 0b1de7e5..4ea77079 100644 --- a/terrain/variable_lod/voxel_lod_terrain.cpp +++ b/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.lod = block.lod_index; + b.position = block.get_position(); + 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); diff --git a/terrain/variable_lod/voxel_lod_terrain_update_task.cpp b/terrain/variable_lod/voxel_lod_terrain_update_task.cpp index 9647e127..1469a319 100644 --- a/terrain/variable_lod/voxel_lod_terrain_update_task.cpp +++ b/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.lod = block.lod_index; + b.position = block.get_position(); + b.lod = block.get_lod_index(); blocks_to_save.push_back(b); } } diff --git a/terrain/voxel_data_block_enter_info.cpp b/terrain/voxel_data_block_enter_info.cpp index e0c4d9bc..d4254bc5 100644 --- a/terrain/voxel_data_block_enter_info.cpp +++ b/terrain/voxel_data_block_enter_info.cpp @@ -17,12 +17,12 @@ Ref 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 {