|
FhSim
3.1.0
Marine systems simulation
|
| ID | 0022 |
| Class | BUG |
| Severity | 2 |
| Status | blocked |
| Models | VisualFlowPlane |
| Found | 2026-09-23 during the design of the baked scalar-field visualisers |
| Decision needed | Is the commented-out render-queue override (FlowPlane.cpp:55 and :94-97) wanted or abandoned? If abandoned, m_defaultRenderQueueGroup and its documentation comment go away with it; if wanted, the destructor has to restore it and the code has to be re-enabled. |
The constructor creates three Ogre objects and the destructor releases none of them.
src/environment/visual/FlowPlane.cpp:44-52 creates a mesh, an entity and a scene node, each under a name made unique by a monotonically increasing instance counter (FlowPlane.cpp:23-25):
flatplate::CreateMesh registers the mesh with Ogre::MeshManager via manual->convertToMesh(name) (src/environment/visual/MeshGenerators.cpp:283), so it outlives the ManualObject that built it.
src/environment/visual/FlowPlane.h:61 is the whole of the cleanup:
No destroyEntity, no destroySceneNode, no MeshManager::remove. The object is held by std::shared_ptr in the SimObject (src/environment/VisualFlowPlane.cpp:197), so the destructor does run — it just does nothing.
Second, the documentation claims a restore that no code performs. src/environment/visual/FlowPlane.h:128:
The member is written once at FlowPlane.cpp:54 and never read anywhere. Both setRenderQueueGroup calls that would have used it are commented out, at FlowPlane.cpp:55 and at FlowPlane.cpp:94-97 in SetTransform. So the group is never changed, never restored, and the comment describes behaviour that does not exist.
Destroying a VisualFlowPlane leaves its entity attached to a live scene node and its mesh registered in MeshManager. The plane keeps being rendered and keeps consuming its vertex buffer for the rest of the process, and because the instance counter never rewinds, a model that creates and destroys planes accumulates one orphaned mesh, entity and node per creation. Within a single simulation run, where planes are built in RenderInit and live until shutdown, nothing visibly breaks; the cost lands on any future use that tears a plane down, and on Ogre shutdown diagnostics.
The stale comment is the part that misleads a reader today: it states that the render-queue group is restored on destruction, which invites the conclusion that the destructor has a body and that the render-queue override is live. Neither is true.
FlowPlane a real destructor: detach and destroyEntity(m_entity), destroySceneNode(m_node), and Ogre::MeshManager::getSingleton().remove(meshName), keeping the mesh name in a member so the destructor can name it. This is the shape the new baked visualisers are required to have, so FlowPlane would match its siblings. Holding the Ogre::Root* or the SceneManager* as a member is part of the change: the constructor takes it as an argument today and keeps neither.m_defaultRenderQueueGroup, its assignment at FlowPlane.cpp:54 and the commented-out blocks, which removes the false claim at FlowPlane.h:128 with them. If it is wanted, re-enable it and restore the saved group in the destructor from option 1 — but that is a rendering-order change with its own visual consequences, which is why it needs the owner decision rather than a tidy-up.Option 1 is self-contained and safe; option 2 is what the Decision needed row asks about.
An Ogre-backed (_NEEDS_GUI) test that records sceneMgr->getRootSceneNode()->numChildren() and Ogre::MeshManager::getSingleton().resourceExists("FlowPlaneMesh<N>"), constructs a FlowPlane in an inner scope, lets it go out of scope, and asserts both are back to their pre-construction values. Today the child count is one higher and the mesh still exists.
Destroying the entity and node is safe only if nothing else holds them; m_subMesh is a raw pointer into the mesh (FlowPlane.h:106) and is used in Update (FlowPlane.cpp:68), so destruction ordering matters if a plane is ever destroyed mid-frame. Removing the mesh from MeshManager while another entity still references it would be a use-after-free, so the removal must stay keyed to the per-instance mesh name rather than the shared material or a shared mesh. Reviving the render-queue override (option 2) changes draw order against the ocean surface and the seafloor, which is a visual regression risk on every existing scene that renders a flow plane.