From f4a5b59430baabcd16052db7a6e98b688fd82c8e Mon Sep 17 00:00:00 2001
From: codereader <greebo@angua.at>
Date: Sat, 29 Oct 2022 09:05:20 +0200
Subject: [PATCH] #6009: ModelNodeBase subtypes need to implement
 createRenderableSurfaces to populate the renderable surface collection

---
 radiantcore/model/ModelNodeBase.cpp    |  7 +++++++
 radiantcore/model/ModelNodeBase.h      |  6 ++++++
 radiantcore/model/StaticModelNode.cpp  | 20 ++++++++++----------
 radiantcore/model/StaticModelNode.h    |  2 ++
 radiantcore/model/md5/MD5ModelNode.cpp | 18 +++++++++---------
 radiantcore/model/md5/MD5ModelNode.h   |  1 +
 6 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/radiantcore/model/ModelNodeBase.cpp b/radiantcore/model/ModelNodeBase.cpp
index 5876ddb46d..58aa0f1290 100644
--- a/radiantcore/model/ModelNodeBase.cpp
+++ b/radiantcore/model/ModelNodeBase.cpp
@@ -17,6 +17,13 @@ void ModelNodeBase::renderHighlights(IRenderableCollector& collector, const Volu
     }
 }
 
+void ModelNodeBase::destroyRenderableSurfaces()
+{
+    detachFromShaders();
+
+    _renderableSurfaces.clear();
+}
+
 void ModelNodeBase::detachFromShaders()
 {
     // Detach any existing surfaces. In case we need them again,
diff --git a/radiantcore/model/ModelNodeBase.h b/radiantcore/model/ModelNodeBase.h
index 0e8ad31aa6..60ced8b3c5 100644
--- a/radiantcore/model/ModelNodeBase.h
+++ b/radiantcore/model/ModelNodeBase.h
@@ -26,6 +26,12 @@ class ModelNodeBase :
     void renderHighlights(IRenderableCollector& collector, const VolumeTest& volume) override;
 
 protected:
+    // To be implemented by subclasses, this should populate the _renderableSurfaces collection
+    virtual void createRenderableSurfaces() = 0;
+
+    // Detaches all surfaces from their shaders and clears the _renderableSurfaces collection
+    virtual void destroyRenderableSurfaces();
+
     void attachToShaders();
     void detachFromShaders();
     void queueRenderableUpdate();
diff --git a/radiantcore/model/StaticModelNode.cpp b/radiantcore/model/StaticModelNode.cpp
index 8e50ebdac3..1bbcc59be3 100644
--- a/radiantcore/model/StaticModelNode.cpp
+++ b/radiantcore/model/StaticModelNode.cpp
@@ -20,11 +20,8 @@ StaticModelNode::StaticModelNode(const StaticModelPtr& picoModel) :
     skinChanged("");
 }
 
-void StaticModelNode::onInsertIntoScene(scene::IMapRootNode& root)
+void StaticModelNode::createRenderableSurfaces()
 {
-    _model->connectUndoSystem(root.getUndoSystem());
-
-    // Renderables will acquire their shaders in onPreRender
     _model->foreachSurface([&](const StaticModelSurface& surface)
     {
         if (surface.getVertexArray().empty() || surface.getIndexArray().empty())
@@ -34,6 +31,14 @@ void StaticModelNode::onInsertIntoScene(scene::IMapRootNode& root)
 
         _renderableSurfaces.emplace_back(std::make_shared<RenderableModelSurface>(surface, _renderEntity, localToWorld()));
     });
+}
+
+void StaticModelNode::onInsertIntoScene(scene::IMapRootNode& root)
+{
+    _model->connectUndoSystem(root.getUndoSystem());
+
+    // Renderables will acquire their shaders in onPreRender
+    createRenderableSurfaces();
 
     Node::onInsertIntoScene(root);
 }
@@ -42,12 +47,7 @@ void StaticModelNode::onRemoveFromScene(scene::IMapRootNode& root)
 {
     _model->disconnectUndoSystem(root.getUndoSystem());
 
-    for (auto& surface : _renderableSurfaces)
-    {
-        surface->detach();
-    }
-
-    _renderableSurfaces.clear();
+    destroyRenderableSurfaces();
 
     Node::onRemoveFromScene(root);
 }
diff --git a/radiantcore/model/StaticModelNode.h b/radiantcore/model/StaticModelNode.h
index 9660e5ec9f..88a16433ce 100644
--- a/radiantcore/model/StaticModelNode.h
+++ b/radiantcore/model/StaticModelNode.h
@@ -90,6 +90,8 @@ class StaticModelNode final :
     void onModelScaleApplied();
 
 protected:
+    void createRenderableSurfaces() override;
+
 	void _onTransformationChanged() override;
 	void _applyTransformation() override;
     void onVisibilityChanged(bool isVisibleNow) override;
diff --git a/radiantcore/model/md5/MD5ModelNode.cpp b/radiantcore/model/md5/MD5ModelNode.cpp
index 798db444dc..7e56e636b6 100644
--- a/radiantcore/model/md5/MD5ModelNode.cpp
+++ b/radiantcore/model/md5/MD5ModelNode.cpp
@@ -72,9 +72,8 @@ scene::INode::Type MD5ModelNode::getNodeType() const
     return Type::Model;
 }
 
-void MD5ModelNode::onInsertIntoScene(scene::IMapRootNode& root)
+void MD5ModelNode::createRenderableSurfaces()
 {
-    // Renderables will acquire their shaders in onPreRender
     _model->foreachSurface([&](const MD5Surface& surface)
     {
         if (surface.getVertexArray().empty() || surface.getIndexArray().empty())
@@ -86,20 +85,21 @@ void MD5ModelNode::onInsertIntoScene(scene::IMapRootNode& root)
             std::make_shared<model::RenderableModelSurface>(surface, _renderEntity, localToWorld())
         );
     });
+}
+
+void MD5ModelNode::onInsertIntoScene(scene::IMapRootNode& root)
+{
+    // Renderables will acquire their shaders in onPreRender
+    createRenderableSurfaces();
 
     Node::onInsertIntoScene(root);
 }
 
 void MD5ModelNode::onRemoveFromScene(scene::IMapRootNode& root)
 {
-    Node::onRemoveFromScene(root);
-
-    for (auto& surface : _renderableSurfaces)
-    {
-        surface->detach();
-    }
+    destroyRenderableSurfaces();
 
-    _renderableSurfaces.clear();
+    Node::onRemoveFromScene(root);
 }
 
 void MD5ModelNode::testSelect(Selector& selector, SelectionTest& test)
diff --git a/radiantcore/model/md5/MD5ModelNode.h b/radiantcore/model/md5/MD5ModelNode.h
index 788bfa389b..8d80155746 100644
--- a/radiantcore/model/md5/MD5ModelNode.h
+++ b/radiantcore/model/md5/MD5ModelNode.h
@@ -78,6 +78,7 @@ class MD5ModelNode :
     void transformChangedLocal() override;
 
 protected:
+    void createRenderableSurfaces() override;
     void onVisibilityChanged(bool isVisibleNow) override;
 
 private: