#5584: Solid patch renderable is not rendered twice anymore when node…

… is selected
codereader · Nov 18, 2021 · 8913d75 · 8913d75
1 parent aec7f05
commit 8913d75
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 38 deletions.
diff --git a/include/irenderable.h b/include/irenderable.h
@@ -101,6 +101,13 @@ class IRenderableCollector
                                const LitObject* litObject = nullptr,
                                const IRenderEntity* entity = nullptr) = 0;
 
+    /**
+     * Submits a renderable object that is used for highlighting an object.
+     * Depending on the view, this might be a coloured, transparent overlay
+     * or a wireframe outline.
+     */
+    virtual void addHighlightRenderable(const OpenGLRenderable& renderable, const Matrix4& localToWorld) = 0;
+
     /**
      * \brief Submit a light source for the render operation.
      *

diff --git a/libs/render/CamRenderer.h b/libs/render/CamRenderer.h
@@ -208,27 +208,7 @@ class CamRenderer :
                        const LitObject* litObject = nullptr,
                        const IRenderEntity* entity = nullptr) override
     {
-        if (_editMode == IMap::EditMode::Merge && (_flags & Highlight::Flags::MergeAction) != 0)
-        {
-            const auto& mergeShader = (_flags & Highlight::Flags::MergeActionAdd) != 0 ? _shaders.mergeActionShaderAdd :
-                (_flags & Highlight::Flags::MergeActionRemove) != 0 ? _shaders.mergeActionShaderRemove : 
-                (_flags & Highlight::Flags::MergeActionConflict) != 0 ? _shaders.mergeActionShaderConflict : _shaders.mergeActionShaderChange;
-
-            if (mergeShader)
-            {
-                mergeShader->addRenderable(renderable, localToWorld, nullptr, entity);
-            }
-        }
-
-        if ((_flags & Highlight::Flags::Primitives) != 0 && _shaders.primitiveHighlightShader)
-        {
-            _shaders.primitiveHighlightShader->addRenderable(renderable, localToWorld, nullptr, entity);
-        }
-
-        if ((_flags & Highlight::Flags::Faces) != 0 && _shaders.faceHighlightShader)
-        {
-            _shaders.faceHighlightShader->addRenderable(renderable, localToWorld, nullptr, entity);
-        }
+        addHighlightRenderable(renderable, localToWorld);
 
         // Construct an entry for this shader in the map if it is the first
         // time we've seen it
@@ -252,6 +232,31 @@ class CamRenderer :
         iter->second.emplace_back(std::move(lr));
     }
 
+    void addHighlightRenderable(const OpenGLRenderable& renderable, const Matrix4& localToWorld) override
+    {
+        if (_editMode == IMap::EditMode::Merge && (_flags & Highlight::Flags::MergeAction) != 0)
+        {
+            const auto& mergeShader = (_flags & Highlight::Flags::MergeActionAdd) != 0 ? _shaders.mergeActionShaderAdd :
+                (_flags & Highlight::Flags::MergeActionRemove) != 0 ? _shaders.mergeActionShaderRemove :
+                (_flags & Highlight::Flags::MergeActionConflict) != 0 ? _shaders.mergeActionShaderConflict : _shaders.mergeActionShaderChange;
+
+            if (mergeShader)
+            {
+                mergeShader->addRenderable(renderable, localToWorld, nullptr, nullptr);
+            }
+        }
+
+        if ((_flags & Highlight::Flags::Primitives) != 0 && _shaders.primitiveHighlightShader)
+        {
+            _shaders.primitiveHighlightShader->addRenderable(renderable, localToWorld, nullptr, nullptr);
+        }
+
+        if ((_flags & Highlight::Flags::Faces) != 0 && _shaders.faceHighlightShader)
+        {
+            _shaders.faceHighlightShader->addRenderable(renderable, localToWorld, nullptr, nullptr);
+        }
+    }
+
 #ifdef RENDERABLE_GEOMETRY
     void addGeometry(RenderableGeometry& geometry, std::size_t flags) override
     {

diff --git a/radiant/xyview/XYRenderer.h b/radiant/xyview/XYRenderer.h
@@ -71,50 +71,55 @@ class XYRenderer :
                        const Matrix4& localToWorld,
                        const LitObject* /* litObject */,
                        const IRenderEntity* entity = nullptr) override
+    {
+        addHighlightRenderable(renderable, localToWorld);
+
+        shader.addRenderable(renderable, localToWorld, nullptr, entity);
+    }
+
+    void addHighlightRenderable(const OpenGLRenderable& renderable, const Matrix4& localToWorld) override
     {
         if (_editMode == IMap::EditMode::Merge)
         {
             if (_flags & Highlight::Flags::MergeAction)
             {
                 // This is a merge-relevant node that should be rendered in a special colour
                 const auto& mergeShader = (_flags & Highlight::Flags::MergeActionAdd) != 0 ? _shaders.mergeActionShaderAdd :
-                    (_flags & Highlight::Flags::MergeActionRemove) != 0 ? _shaders.mergeActionShaderRemove : 
+                    (_flags & Highlight::Flags::MergeActionRemove) != 0 ? _shaders.mergeActionShaderRemove :
                     (_flags & Highlight::Flags::MergeActionConflict) != 0 ? _shaders.mergeActionShaderConflict : _shaders.mergeActionShaderChange;
 
                 if (mergeShader)
                 {
-                    mergeShader->addRenderable(renderable, localToWorld, nullptr, entity);
+                    mergeShader->addRenderable(renderable, localToWorld, nullptr, nullptr);
                 }
             }
             else
             {
                 // Everything else is using the shader for non-merge-affected nodes
-                _shaders.nonMergeActionNodeShader->addRenderable(renderable, localToWorld, nullptr, entity);
+                _shaders.nonMergeActionNodeShader->addRenderable(renderable, localToWorld, nullptr, nullptr);
             }
 
             // Elements can still be selected in merge mode
             if ((_flags & Highlight::Flags::Primitives) != 0)
             {
-                _shaders.selectedShader->addRenderable(renderable, localToWorld, nullptr, entity);
+                _shaders.selectedShader->addRenderable(renderable, localToWorld, nullptr, nullptr);
             }
 
             return;
         }
-        
+
         // Regular editing mode, add all highlighted nodes to the corresponding shader
         if ((_flags & Highlight::Flags::Primitives) != 0)
         {
             if ((_flags & Highlight::Flags::GroupMember) != 0)
             {
-                _shaders.selectedShaderGroup->addRenderable(renderable, localToWorld, nullptr, entity);
+                _shaders.selectedShaderGroup->addRenderable(renderable, localToWorld, nullptr, nullptr);
             }
             else
             {
-                _shaders.selectedShader->addRenderable(renderable, localToWorld, nullptr, entity);
+                _shaders.selectedShader->addRenderable(renderable, localToWorld, nullptr, nullptr);
             }
         }
-
-        shader.addRenderable(renderable, localToWorld, nullptr, entity);
     }
 
     void render(const Matrix4& modelview, const Matrix4& projection)

diff --git a/radiantcore/brush/BrushNode.cpp b/radiantcore/brush/BrushNode.cpp
@@ -427,10 +427,7 @@ void BrushNode::renderHighlights(IRenderableCollector& collector, const VolumeTe
                 collector.setHighlightFlag(IRenderableCollector::Highlight::Faces, true);
 
             // greebo: BrushNodes have always an identity l2w, don't do any transforms
-            collector.addRenderable(
-                *face.getFaceShader().getGLShader(), face.getWinding(),
-                Matrix4::getIdentity(), this, _renderEntity
-            );
+            collector.addHighlightRenderable(face.getWinding(), Matrix4::getIdentity());
 
             if (highlight)
                 collector.setHighlightFlag(IRenderableCollector::Highlight::Faces, false);

diff --git a/radiantcore/patch/Patch.cpp b/radiantcore/patch/Patch.cpp
@@ -583,6 +583,7 @@ void Patch::updateTesselation(bool force)
         }
     }
 
+    _node.onTesselationChanged();
     _solidRenderable.queueUpdate();
 
     if (_patchDef3)

diff --git a/radiantcore/patch/PatchNode.cpp b/radiantcore/patch/PatchNode.cpp
@@ -386,10 +386,7 @@ void PatchNode::renderWireframe(IRenderableCollector& collector, const VolumeTes
 
 void PatchNode::renderHighlights(IRenderableCollector& collector, const VolumeTest& volume)
 {
-    collector.addRenderable(
-        *m_patch._shader.getGLShader(), m_patch._solidRenderable,
-        localToWorld(), this, _renderEntity
-    );
+    collector.addHighlightRenderable(m_patch._solidRenderable, localToWorld());
 
     // Render the selected components
     renderComponentsSelected(collector, volume);
@@ -551,6 +548,12 @@ const Vector3& PatchNode::getUntransformedOrigin()
     return _untransformedOrigin;
 }
 
+void PatchNode::onTesselationChanged()
+{
+    _renderableSurfaceSolid.queueUpdate();
+    _renderableSurfaceWireframe.queueUpdate();
+}
+
 void PatchNode::onControlPointsChanged()
 {
     _renderableSurfaceSolid.queueUpdate();

diff --git a/radiantcore/patch/PatchNode.h b/radiantcore/patch/PatchNode.h
@@ -153,6 +153,7 @@ class PatchNode final :
 
     void onControlPointsChanged();
     void onMaterialChanged();
+    void onTesselationChanged();
 
 protected:
 	// Gets called by the Transformable implementation whenever

diff --git a/test/Entity.cpp b/test/Entity.cpp
@@ -427,6 +427,10 @@ namespace
             renderablePtrs.push_back(std::make_pair(&shader, &renderable));
         }
 
+        void addHighlightRenderable(const OpenGLRenderable& renderable,
+            const Matrix4& localToWorld) override
+        {}
+
         void addLight(const RendererLight& light)
         {
             ++lights;