From 0c12c1f400e4797fed3df9a35aea64bfac9e8fc3 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Thu, 24 Jul 2025 14:18:38 +0200 Subject: [PATCH] Add a bit of debug code around MapBlock refcounting --- src/client/clientmap.cpp | 41 ++++++++++++++++++++++++++++------------ src/client/clientmap.h | 10 +++++++++- src/map.cpp | 17 ++++++++++++++--- src/mapblock.h | 18 ++++++++---------- src/mapsector.cpp | 12 +++++++++--- src/mapsector.h | 7 +++++-- 6 files changed, 74 insertions(+), 31 deletions(-) diff --git a/src/client/clientmap.cpp b/src/client/clientmap.cpp index aad5c700d..158f526de 100644 --- a/src/client/clientmap.cpp +++ b/src/client/clientmap.cpp @@ -202,8 +202,14 @@ void ClientMap::onSettingChanged(std::string_view name, bool all) ClientMap::~ClientMap() { + verbosestream << FUNCTION_NAME << std::endl; + g_settings->deregisterAllChangedCallbacks(this); + // avoid refcount warning from ~Map() + clearDrawList(); + clearDrawListShadow(); + for (auto &it : m_dynamic_buffers) it.second.drop(); } @@ -351,23 +357,29 @@ private: v3s16 volume; }; -void ClientMap::updateDrawList() +void ClientMap::clearDrawList() { - ScopeProfiler sp(g_profiler, "CM::updateDrawList()", SPT_AVG); - - m_needs_update_drawlist = false; - for (auto &i : m_drawlist) { MapBlock *block = i.second; block->refDrop(); } m_drawlist.clear(); - for (auto &block : m_keeplist) { + for (auto &block : m_keeplist) block->refDrop(); - } m_keeplist.clear(); + m_needs_update_drawlist = true; +} + +void ClientMap::updateDrawList() +{ + ScopeProfiler sp(g_profiler, "CM::updateDrawList()", SPT_AVG); + + clearDrawList(); + + m_needs_update_drawlist = false; + const v3s16 cam_pos_nodes = floatToInt(m_camera_position, BS); v3s16 p_blocks_min; @@ -1519,6 +1531,15 @@ void ClientMap::renderMapShadows(video::IVideoDriver *driver, g_profiler->avg(prefix + "material swaps [#]", material_swaps); } +void ClientMap::clearDrawListShadow() +{ + for (auto &i : m_drawlist_shadow) { + MapBlock *block = i.second; + block->refDrop(); + } + m_drawlist_shadow.clear(); +} + /* Custom update draw list for the pov of shadow light. */ @@ -1526,11 +1547,7 @@ void ClientMap::updateDrawListShadow(v3f shadow_light_pos, v3f shadow_light_dir, { ScopeProfiler sp(g_profiler, "CM::updateDrawListShadow()", SPT_AVG); - for (auto &i : m_drawlist_shadow) { - MapBlock *block = i.second; - block->refDrop(); - } - m_drawlist_shadow.clear(); + clearDrawListShadow(); // Number of blocks currently loaded by the client u32 blocks_loaded = 0; diff --git a/src/client/clientmap.h b/src/client/clientmap.h index 259ae849e..ce527bd23 100644 --- a/src/client/clientmap.h +++ b/src/client/clientmap.h @@ -89,12 +89,20 @@ public: void getBlocksInViewRange(v3s16 cam_pos_nodes, v3s16 *p_blocks_min, v3s16 *p_blocks_max, float range=-1.0f); + void updateDrawList(); - // @brief Calculate statistics about the map and keep the blocks alive + /// @brief clears m_drawlist and m_keeplist + void clearDrawList(); + + /// @brief Calculate statistics about the map and keep the blocks alive void touchMapBlocks(); + void updateDrawListShadow(v3f shadow_light_pos, v3f shadow_light_dir, float radius, float length); + void clearDrawListShadow(); + // Returns true if draw list needs updating before drawing the next frame. bool needsUpdateDrawList() { return m_needs_update_drawlist; } + void renderMap(video::IVideoDriver* driver, s32 pass); void renderMapShadows(video::IVideoDriver *driver, diff --git a/src/map.cpp b/src/map.cpp index 964d8b5af..a7e8d2b5f 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -30,12 +30,23 @@ Map::Map(IGameDef *gamedef): Map::~Map() { - /* - Free all MapSectors - */ + // Free all sectors + size_t used = 0; for (auto §or : m_sectors) { + sector.second->deleteBlocks(&used); delete sector.second; } + m_sectors.clear(); + + if (used > 0) { +#ifdef NDEBUG + std::ostream &to = infostream; +#else + std::ostream &to = warningstream; +#endif + PrintInfo(to); + to << used << " blocks deleted despite reference count > 0. Potential bug." << std::endl; + } } void Map::addEventReceiver(MapEventReceiver *event_receiver) diff --git a/src/mapblock.h b/src/mapblock.h index 9778b98b2..a827c3f6d 100644 --- a/src/mapblock.h +++ b/src/mapblock.h @@ -187,26 +187,28 @@ public: //// Position stuff //// + /// @return map position of block inline v3s16 getPos() { return m_pos; } + /// @return in-world position of the block (== pos * MAP_BLOCKSIZE) inline v3s16 getPosRelative() { return m_pos_relative; } - inline core::aabbox3d getBox() { + /// @return in-world box of the block + inline core::aabbox3d getBox() + { return getBox(getPosRelative()); } - static inline core::aabbox3d getBox(const v3s16 &pos_relative) + static inline core::aabbox3d getBox(v3s16 pos_relative) { return core::aabbox3d(pos_relative, - pos_relative - + v3s16(MAP_BLOCKSIZE, MAP_BLOCKSIZE, MAP_BLOCKSIZE) - - v3s16(1,1,1)); + pos_relative + v3s16(MAP_BLOCKSIZE - 1)); } //// @@ -360,7 +362,7 @@ public: } //// - //// Reference counting (see m_refcount) + //// Reference counting (different purposes on client vs. server) //// inline void refGrab() @@ -470,10 +472,6 @@ private: */ v3s16 m_pos_relative; - /* - Reference count; currently used for determining if this block is in - the list of blocks to be drawn. - */ short m_refcount = 0; /* diff --git a/src/mapsector.cpp b/src/mapsector.cpp index 5e943345c..4d6d47268 100644 --- a/src/mapsector.cpp +++ b/src/mapsector.cpp @@ -19,12 +19,18 @@ MapSector::~MapSector() deleteBlocks(); } -void MapSector::deleteBlocks() +void MapSector::deleteBlocks(size_t *used_count) { - // Clear cache m_block_cache = nullptr; - // Delete all blocks + size_t u = 0; + for (auto &it : m_blocks) { + if (it.second->refGet() > 0) + u++; + it.second.reset(); + } + if (used_count) + *used_count += u; m_blocks.clear(); } diff --git a/src/mapsector.h b/src/mapsector.h index d977bc63c..fdc6af532 100644 --- a/src/mapsector.h +++ b/src/mapsector.h @@ -29,7 +29,9 @@ public: MapSector(Map *parent, v2s16 pos, IGameDef *gamedef); virtual ~MapSector(); - void deleteBlocks(); + /// @brief Deletes all blocks (regardless of reference count). + /// @param used_count output: number of blocks which were still ref'd + void deleteBlocks(size_t *used_count = nullptr); v2s16 getPos() const { @@ -60,7 +62,8 @@ public: bool empty() const { return m_blocks.empty(); } - int size() const { return m_blocks.size(); } + size_t size() const { return m_blocks.size(); } + protected: // The pile of MapBlocks