From 7357060377eb8ad1fb998598414beebde0731d15 Mon Sep 17 00:00:00 2001 From: Marc Gilleron Date: Wed, 26 Sep 2018 01:22:23 +0100 Subject: [PATCH] Several fixes, a few bugs remain - Spread mesh updates on the main thread to reduce stalls - Fix some ambiguous situations with an additional block state - Expose block states for debug drawing --- voxel_terrain.cpp | 118 +++++++++++++++++++++++++++++++--------------- voxel_terrain.h | 34 +++++++++---- 2 files changed, 105 insertions(+), 47 deletions(-) diff --git a/voxel_terrain.cpp b/voxel_terrain.cpp index c756a89..1f968cf 100644 --- a/voxel_terrain.cpp +++ b/voxel_terrain.cpp @@ -168,23 +168,32 @@ Ref VoxelTerrain::get_material(int id) const { void VoxelTerrain::make_block_dirty(Vector3i bpos) { // TODO Immediate update viewer distance? - if (is_block_dirty(bpos) == false) { + VoxelTerrain::BlockDirtyState *state = _dirty_blocks.getptr(bpos); + + if(state == NULL) { + // The block is not dirty, so it will either be loaded or updated if(_map->has_block(bpos)) { _blocks_pending_update.push_back(bpos); - _dirty_blocks[bpos] = BLOCK_UPDATE; + _dirty_blocks[bpos] = BLOCK_UPDATE_NOT_SENT; } else { _blocks_pending_load.push_back(bpos); _dirty_blocks[bpos] = BLOCK_LOAD; } - //OS::get_singleton()->print("Dirty (%i, %i, %i)", bpos.x, bpos.y, bpos.z); - - // TODO What if a block is made dirty, goes through threaded update, then gets changed again before it gets updated? - // this will make the second change ignored, which is not correct! + } else if(*state == BLOCK_UPDATE_SENT) { + // The updater is already processing the block, + // but the block was modified again so we schedule another update + *state = BLOCK_UPDATE_NOT_SENT; + _blocks_pending_update.push_back(bpos); } + + //OS::get_singleton()->print("Dirty (%i, %i, %i)", bpos.x, bpos.y, bpos.z); + + // TODO What if a block is made dirty, goes through threaded update, then gets changed again before it gets updated? + // this will make the second change ignored, which is not correct! } void VoxelTerrain::immerge_block(Vector3i bpos) { @@ -205,6 +214,7 @@ Dictionary VoxelTerrain::get_statistics() const { provider["min_time"] = _stats.provider.min_time; provider["max_time"] = _stats.provider.max_time; provider["remaining_blocks"] = _stats.provider.remaining_blocks; + provider["dropped_blocks"] = _stats.dropped_provider_blocks; Dictionary updater; updater["min_time"] = _stats.updater.min_time; @@ -212,6 +222,8 @@ Dictionary VoxelTerrain::get_statistics() const { updater["remaining_blocks"] = _stats.updater.remaining_blocks; updater["updated_blocks"] = _stats.updated_blocks; updater["mesh_alloc_time"] = _stats.mesh_alloc_time; + updater["dropped_blocks"] = _stats.dropped_updater_blocks; + updater["remaining_main_thread_blocks"] = _stats.remaining_main_thread_blocks; Dictionary d; d["provider"] = provider; @@ -530,10 +542,19 @@ void VoxelTerrain::_process() { //print_line(String("Receiving {0} blocks").format(varray(output.emerged_blocks.size()))); _stats.provider = output.stats; + _stats.dropped_provider_blocks = 0; for(int i = 0; i < output.emerged_blocks.size(); ++i) { const VoxelProviderThread::EmergeOutput &o = output.emerged_blocks[i]; + Vector3i block_pos = _map->voxel_to_block(o.origin_in_voxels); + + VoxelTerrain::BlockDirtyState *state = _dirty_blocks.getptr(block_pos); + if(state == NULL || *state != BLOCK_LOAD) { + // That block was not requested, drop it + ++_stats.dropped_provider_blocks; + continue; + } // Check return // TODO Shouldn't halt execution though, as it can bring the map in an invalid state! @@ -542,7 +563,6 @@ void VoxelTerrain::_process() { // TODO Discard blocks out of range // Store buffer - Vector3i block_pos = _map->voxel_to_block(o.origin_in_voxels); bool update_neighbors = !_map->has_block(block_pos); _map->set_block_buffer(block_pos, o.voxels); @@ -558,13 +578,12 @@ void VoxelTerrain::_process() { // TODO What if the map is really composed of empty blocks? if (_map->is_block_surrounded(npos)) { - VoxelTerrain::BlockDirtyState *state = _dirty_blocks.getptr(npos); - if (state && *state == BLOCK_UPDATE) { + if (state && *state == BLOCK_UPDATE_NOT_SENT || *state == BLOCK_UPDATE_SENT) { // Assuming it is scheduled to be updated already. continue; } - _dirty_blocks[npos] = BLOCK_UPDATE; + _dirty_blocks[npos] = BLOCK_UPDATE_NOT_SENT; _blocks_pending_update.push_back(npos); } } @@ -572,7 +591,7 @@ void VoxelTerrain::_process() { } } else { // Only update the block, neighbors will probably follow if needed - _dirty_blocks[block_pos] = BLOCK_UPDATE; + _dirty_blocks[block_pos] = BLOCK_UPDATE_NOT_SENT; _blocks_pending_update.push_back(block_pos); //OS::get_singleton()->print("Update (%i, %i, %i)\n", block_pos.x, block_pos.y, block_pos.z); } @@ -606,6 +625,11 @@ void VoxelTerrain::_process() { iblock.voxels = nbuffer; iblock.position = block_pos; input.blocks.push_back(iblock); + + VoxelTerrain::BlockDirtyState *state = _dirty_blocks.getptr(block_pos); + CRASH_COND(state == NULL); + CRASH_COND(*state != BLOCK_UPDATE_NOT_SENT); + *state = BLOCK_UPDATE_SENT; } _block_updater->push(input); @@ -614,28 +638,41 @@ void VoxelTerrain::_process() { // Get mesh updates { - VoxelMeshUpdater::Output output; - _block_updater->pop(output); + { + VoxelMeshUpdater::Output output; + _block_updater->pop(output); - _stats.updater = output.stats; - _stats.updated_blocks = output.blocks.size(); + _stats.updater = output.stats; + _stats.updated_blocks = output.blocks.size(); + _stats.dropped_updater_blocks = 0; + + _blocks_pending_main_thread_update.append_array(output.blocks); + } Ref world = get_world(); - uint32_t time_before = os.get_ticks_msec(); + uint32_t timeout = os.get_ticks_msec() + 10; + int queue_index = 0; - for (int i = 0; i < output.blocks.size(); ++i) { - const VoxelMeshUpdater::OutputBlock &ob = output.blocks[i]; + // The following is done on the main thread because Godot doesn't really support multithreaded Mesh allocation. + // This also proved to be very slow compared to the meshing process itself... hopefully Vulkan will improve this? + + for (; queue_index < _blocks_pending_main_thread_update.size() && os.get_ticks_msec() < timeout; ++queue_index) { + + const VoxelMeshUpdater::OutputBlock &ob = _blocks_pending_main_thread_update[queue_index]; + + VoxelTerrain::BlockDirtyState *state = _dirty_blocks.getptr(ob.position); + if (state && *state == BLOCK_UPDATE_SENT) { + _dirty_blocks.erase(ob.position); + } VoxelBlock *block = _map->get_block(ob.position); if (block == NULL) { - clear_block_update_state(ob.position); + // That block is no longer loaded, drop the result + ++_stats.dropped_updater_blocks; continue; } - // Note: I allocate the mesh here because Godot doesn't supports doing it in another thread without hanging the main one. - // Hopefully Vulkan will improve this? - Ref mesh; mesh.instance(); @@ -649,7 +686,6 @@ void VoxelTerrain::_process() { mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, surface); mesh->surface_set_material(surface_index, _materials[i]); - ++surface_index; } @@ -668,9 +704,10 @@ void VoxelTerrain::_process() { mesh = Ref(); block->set_mesh(mesh, world); - clear_block_update_state(ob.position); } + shift_up(_blocks_pending_main_thread_update, queue_index); + uint32_t time_taken = os.get_ticks_msec() - time_before; _stats.mesh_alloc_time = time_taken; } @@ -678,19 +715,6 @@ void VoxelTerrain::_process() { //print_line(String("d:") + String::num(_dirty_blocks.size()) + String(", q:") + String::num(_block_update_queue.size())); } -// To be called once a block is updated -void VoxelTerrain::clear_block_update_state(Vector3i block_pos) { - VoxelTerrain::BlockDirtyState *state = _dirty_blocks.getptr(block_pos); - if (state) { - if (*state == BLOCK_UPDATE) - _dirty_blocks.erase(block_pos); - else - ;//print_line("Block update found non-update state"); - } else { - ;//print_line("Block update found no update state"); - } -} - //void VoxelTerrain::block_removed(VoxelBlock & block) { // MeshInstance * mesh_instance = block.get_mesh_instance(*this); // if (mesh_instance) { @@ -758,6 +782,19 @@ Vector3 VoxelTerrain::_block_to_voxel_binding(Vector3 pos) { return Vector3i(_map->block_to_voxel(pos)).to_vec3(); } +// For debugging purpose +VoxelTerrain::BlockDirtyState VoxelTerrain::get_block_state(Vector3 p_bpos) const { + Vector3i bpos = p_bpos; + const VoxelTerrain::BlockDirtyState *state = _dirty_blocks.getptr(bpos); + if(state) { + return *state; + } else { + if(!_map->has_block(bpos)) + return BLOCK_NONE; + return BLOCK_IDLE; + } +} + void VoxelTerrain::_bind_methods() { ClassDB::bind_method(D_METHOD("set_provider", "provider"), &VoxelTerrain::set_provider); @@ -787,10 +824,17 @@ void VoxelTerrain::_bind_methods() { ClassDB::bind_method(D_METHOD("raycast", "origin", "direction", "max_distance"), &VoxelTerrain::_raycast_binding, DEFVAL(100)); ClassDB::bind_method(D_METHOD("get_statistics"), &VoxelTerrain::get_statistics); + ClassDB::bind_method(D_METHOD("get_block_state", "block_pos"), &VoxelTerrain::get_block_state); ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "provider", PROPERTY_HINT_RESOURCE_TYPE, "VoxelProvider"), "set_provider", "get_provider"); ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "voxel_library", PROPERTY_HINT_RESOURCE_TYPE, "VoxelLibrary"), "set_voxel_library", "get_voxel_library"); ADD_PROPERTY(PropertyInfo(Variant::INT, "view_distance"), "set_view_distance", "get_view_distance"); ADD_PROPERTY(PropertyInfo(Variant::NODE_PATH, "viewer_path"), "set_viewer_path", "get_viewer_path"); ADD_PROPERTY(PropertyInfo(Variant::BOOL, "generate_collisions"), "set_generate_collisions", "get_generate_collisions"); + + BIND_ENUM_CONSTANT(BLOCK_NONE); + BIND_ENUM_CONSTANT(BLOCK_LOAD); + BIND_ENUM_CONSTANT(BLOCK_UPDATE_NOT_SENT); + BIND_ENUM_CONSTANT(BLOCK_UPDATE_SENT); + BIND_ENUM_CONSTANT(BLOCK_IDLE); } diff --git a/voxel_terrain.h b/voxel_terrain.h index 6f8b2fb..d7ecbe4 100644 --- a/voxel_terrain.h +++ b/voxel_terrain.h @@ -18,6 +18,15 @@ class VoxelLibrary; class VoxelTerrain : public Spatial /*, public IVoxelMapObserver*/ { GDCLASS(VoxelTerrain, Spatial) public: + + enum BlockDirtyState { + BLOCK_NONE, + BLOCK_LOAD, + BLOCK_UPDATE_NOT_SENT, + BLOCK_UPDATE_SENT, + BLOCK_IDLE + }; + VoxelTerrain(); ~VoxelTerrain(); @@ -50,9 +59,17 @@ public: VoxelMeshUpdater::Stats updater; VoxelProviderThread::Stats provider; uint32_t mesh_alloc_time; - uint32_t updated_blocks; + int updated_blocks; + int dropped_provider_blocks; + int dropped_updater_blocks; + int remaining_main_thread_blocks; - Stats(): mesh_alloc_time(0), updated_blocks(0) + Stats(): + mesh_alloc_time(0), + updated_blocks(0), + dropped_provider_blocks(0), + dropped_updater_blocks(0), + remaining_main_thread_blocks(0) { } }; @@ -68,11 +85,6 @@ private: void make_all_view_dirty_deferred(); - enum BlockDirtyState { - BLOCK_LOAD, - BLOCK_UPDATE - }; - Spatial *get_viewer(NodePath path) const; void immerge_block(Vector3i bpos); @@ -93,8 +105,7 @@ private: void set_voxel(Vector3 pos, int value, int c); int get_voxel(Vector3 pos, int c); - - void clear_block_update_state(Vector3i block_pos); + BlockDirtyState get_block_state(Vector3 p_bpos) const; static void remove_positions_outside_box(Vector &positions, Rect3i box, HashMap &state_map); @@ -110,7 +121,8 @@ private: Vector _blocks_pending_load; Vector _blocks_pending_update; - HashMap _dirty_blocks; // only the key is relevant + HashMap _dirty_blocks; // TODO Rename _block_states + Vector _blocks_pending_main_thread_update; Ref _provider; VoxelProviderThread *_provider_thread; @@ -129,4 +141,6 @@ private: Stats _stats; }; +VARIANT_ENUM_CAST(VoxelTerrain::BlockDirtyState) + #endif // VOXEL_TERRAIN_H