#5584: OpenGLShader now offers an interface to attach indexed vertice…

…s directly, returning a handle for later manipulation.

include/irender.h

@@ -2,6 +2,7 @@
 
 #include "imodule.h"
 #include "iwindingrenderer.h"
+#include "isurfacerenderer.h"
 #include <functional>
 #include <vector>
 
@@ -390,7 +391,8 @@ typedef std::shared_ptr<Material> MaterialPtr;
  * Shaders and rendering the geometry attached to each one.
  */
 class Shader :
-    public render::IWindingRenderer
+    public render::IWindingRenderer,
+    public render::ISurfaceRenderer
 {
 public:
 	// Observer interface to get notified on (un-)realisation
@@ -429,8 +431,6 @@ class Shader :
 							   const LightSources* lights = nullptr,
                                const IRenderEntity* entity = nullptr) = 0;
 
-    virtual void addSurface(const std::vector<ArbitraryMeshVertex>& vertices, const std::vector<unsigned int>& indices) = 0;
-
     virtual void addGeometry(RenderableGeometry& geometry) = 0;
 
     /**

include/isurfacerenderer.h

@@ -0,0 +1,40 @@
+#pragma once
+
+#include <vector>
+#include <limits>
+#include <cstdint>
+#include "render/ArbitraryMeshVertex.h"
+
+namespace render
+{
+
+/**
+ * A surface renderer accepts a variable number of indexed surfaces and arranges 
+ * them into one or more continuous blocks of vertices for efficient rendering.
+ *
+ * The internal arrangement has the goal of reducing the amount of draw calls for
+ * suraces sharing a single material. Allocating a surface slot yields a handle which
+ * allows for later update or deallocation of the slot.
+ */
+class ISurfaceRenderer
+{
+public:
+    virtual ~ISurfaceRenderer() {}
+
+    using Slot = std::uint64_t;
+    static constexpr Slot InvalidSlot = std::numeric_limits<Slot>::max();
+
+    // Allocate a slot to hold the given surface data of the given size
+    // Returns the handle which can be used to update or deallocate the data later
+    virtual Slot addSurface(const std::vector<ArbitraryMeshVertex>& vertices,
+        const std::vector<unsigned int>& indices) = 0;
+
+    // Releases a previously allocated slot. This invalidates the handle.
+    virtual void removeSurface(Slot slot) = 0;
+
+    // Sets the surface data
+    virtual void updateSurface(Slot slot, const std::vector<ArbitraryMeshVertex>& vertices,
+        const std::vector<unsigned int>& indices) = 0;
+};
+
+}

libs/render/CompactWindingVertexBuffer.h

@@ -106,19 +106,8 @@ class CompactWindingVertexBuffer
         _vertices.erase(firstVertexToRemove, firstVertexToRemove + _size);
 
         // Since all the windings have the same structure, the index array will always look the same
-        // after shifting the index values of the remaining windings. So just cut off the last one
-#if 0
-        // Shift all indices after this slot towards the left, applying the -size offset
-        auto firstIndexToAdjust = (slot + 1) * _numIndicesPerWinding;
-        auto lastIndexToAdjust = _indices.size() - 1;
-        auto indexToReassign = slot * _numIndicesPerWinding;
-
-        for (auto i = firstIndexToAdjust; i < lastIndexToAdjust; ++i, ++indexToReassign)
-        {
-            _indices[indexToReassign] = _indices[i] - static_cast<unsigned int>(_size);
-        }
-#endif
-        // Cut off one winding from the end of the index array
+        // after shifting the index values of the remaining windings. 
+        // So just cut off one winding from the end of the index array
         _indices.resize(_indices.size() - _numIndicesPerWinding);
     }
 };

radiantcore/patch/Patch.cpp

@@ -332,6 +332,7 @@ void Patch::controlPointsChanged()
     transformChanged();
     evaluateTransform();
     updateTesselation();
+    _node.onControlPointsChanged();
 
     for (Observers::iterator i = _observers.begin(); i != _observers.end();)
     {

radiantcore/patch/PatchNode.cpp

@@ -317,14 +317,13 @@ void PatchNode::renderSolid(RenderableCollector& collector, const VolumeTest& vo
 	// Don't render invisible shaders
 	if (!isForcedVisible() && !m_patch.hasVisibleMaterial()) return;
 
-    const_cast<PatchNode&>(*this)._renderableSurface.setShader(m_patch._shader.getGLShader());
-    const_cast<PatchNode&>(*this)._renderableSurface.update();
-
     // Defer the tesselation calculation to the last minute
-	const_cast<Patch&>(m_patch).evaluateTransform();
+    const_cast<Patch&>(m_patch).evaluateTransform();
     const_cast<Patch&>(m_patch).updateTesselation();
 
-	assert(_renderEntity); // patches rendered without parent - no way!
+    const_cast<PatchNode&>(*this)._renderableSurface.update(m_patch._shader.getGLShader());
+
+    assert(_renderEntity); // patches rendered without parent - no way!
 
 #if 0
     // Render the patch itself
@@ -484,6 +483,7 @@ void PatchNode::transformComponents(const Matrix4& matrix) {
 void PatchNode::_onTransformationChanged()
 {
 	m_patch.transformChanged();
+    _renderableSurface.queueUpdate();
 }
 
 void PatchNode::_applyTransformation()
@@ -506,3 +506,8 @@ const Vector3& PatchNode::getUntransformedOrigin()
 
     return _untransformedOrigin;
 }
+
+void PatchNode::onControlPointsChanged()
+{
+    _renderableSurface.queueUpdate();
+}

radiantcore/patch/PatchNode.h

@@ -146,6 +146,8 @@ class PatchNode :
     // Returns the center of the untransformed world AABB
     const Vector3& getUntransformedOrigin() override;
 
+    void onControlPointsChanged();
+
 protected:
 	// Gets called by the Transformable implementation whenever
 	// scale, rotation or translation is changed.

radiantcore/patch/PatchRenderables.cpp

@@ -158,19 +158,31 @@ void RenderablePatchVectorsNTB::render(RenderableCollector& collector, const Vol
 	collector.addRenderable(*_shader, *this, localToWorld);
 }
 
-void RenderablePatchTesselation::setShader(const ShaderPtr& shader)
+void RenderablePatchTesselation::queueUpdate()
 {
-    _shader = shader;
+    _needsUpdate = true;
 }
 
-void RenderablePatchTesselation::update()
+void RenderablePatchTesselation::update(const ShaderPtr& shader)
 {
-    if (!_shader) return;
-    if (!_needsUpdate) return;
+    bool shaderChanged = _shader != shader;
+
+    if (!_needsUpdate && !shaderChanged) return;
 
     _needsUpdate = false;
+    auto sizeChanged = _tess.vertices.size() != _size;
+
+    if (_shader && _surfaceSlot != render::ISurfaceRenderer::InvalidSlot && (shaderChanged || sizeChanged))
+    {
+        _shader->removeSurface(_surfaceSlot);
+        _surfaceSlot = render::ISurfaceRenderer::InvalidSlot;
+        _size = 0;
+    }
 
-    // Load all the vertices into the target vector
+    _shader = shader;
+    _size = _tess.vertices.size();
+
+    // Generate the index array
     std::vector<unsigned int> indices;
     indices.reserve((_tess.height - 1) * (_tess.width - 1) * 6); // 6 => 2 triangles per quad
 
@@ -191,5 +203,12 @@ void RenderablePatchTesselation::update()
         }
     }
 
-    _shader->addSurface(_tess.vertices, indices);
+    if (_surfaceSlot == render::ISurfaceRenderer::InvalidSlot)
+    {
+        _surfaceSlot = shader->addSurface(_tess.vertices, indices);
+    }
+    else
+    {
+        shader->updateSurface(_surfaceSlot, _tess.vertices, indices);
+    }
 }

radiantcore/patch/PatchRenderables.h

@@ -99,13 +99,18 @@ class RenderablePatchTesselation
     const PatchTesselation& _tess;
     bool _needsUpdate;
     ShaderPtr _shader;
+    std::size_t _size;
+
+    render::ISurfaceRenderer::Slot _surfaceSlot;
 
 public:
     RenderablePatchTesselation(const PatchTesselation& tess) :
         _tess(tess),
-        _needsUpdate(true)
+        _needsUpdate(true),
+        _surfaceSlot(render::ISurfaceRenderer::InvalidSlot),
+        _size(0)
     {}
 
-    void update();
-    void setShader(const ShaderPtr& shader);
+    void queueUpdate();
+    void update(const ShaderPtr& shader);
 };

radiantcore/rendersystem/backend/OpenGLShader.cpp

@@ -111,6 +111,7 @@ void OpenGLShader::addRenderable(const OpenGLRenderable& renderable,
     }
 }
 
+#if 0
 void OpenGLShader::addSurface(const std::vector<ArbitraryMeshVertex>& vertices, const std::vector<unsigned int>& indices)
 {
     if (!_vertexBuffer)
@@ -125,19 +126,21 @@ void OpenGLShader::addSurface(const std::vector<ArbitraryMeshVertex>& vertices,
     _vertexBuffer->addVertices(vertices.begin(), vertices.end());
     _vertexBuffer->addIndicesToLastBatch(indices.begin(), indices.size(), indexOffset);
 }
-
+#endif
 void OpenGLShader::drawSurfaces()
 {
     glEnableClientState(GL_VERTEX_ARRAY);
     glEnableClientState(GL_TEXTURE_COORD_ARRAY);
     glEnableClientState(GL_NORMAL_ARRAY);
 
-    // Surfaces like windings are using CW culling
+    // Surfaces are using CW culling
     glFrontFace(GL_CW);
 
     if (hasSurfaces())
     {
-        _vertexBuffer->renderAllBatches(GL_TRIANGLES, false);
+        //_vertexBuffer->renderAllBatches(GL_TRIANGLES, false);
+
+        SurfaceRenderer::render();
 #if 0
         // Render all triangles
 
@@ -196,20 +199,37 @@ void OpenGLShader::clearGeometry()
 
 bool OpenGLShader::hasSurfaces() const
 {
-    return _vertexBuffer && _vertexBuffer->getNumVertices() > 0;
+    return !SurfaceRenderer::empty() || _vertexBuffer && _vertexBuffer->getNumVertices() > 0;
+}
+
+ISurfaceRenderer::Slot OpenGLShader::addSurface(const std::vector<ArbitraryMeshVertex>& vertices,
+    const std::vector<unsigned int>& indices)
+{
+    return SurfaceRenderer::addSurface(vertices, indices);
+}
+
+void OpenGLShader::removeSurface(ISurfaceRenderer::Slot slot)
+{
+    SurfaceRenderer::removeSurface(slot);
+}
+
+void OpenGLShader::updateSurface(ISurfaceRenderer::Slot slot, const std::vector<ArbitraryMeshVertex>& vertices,
+    const std::vector<unsigned int>& indices)
+{
+    SurfaceRenderer::updateSurface(slot, vertices, indices);
 }
 
 IWindingRenderer::Slot OpenGLShader::addWinding(const std::vector<ArbitraryMeshVertex>& vertices)
 {
     return WindingRenderer::addWinding(vertices);
 }
 
-void OpenGLShader::removeWinding(Slot slot)
+void OpenGLShader::removeWinding(IWindingRenderer::Slot slot)
 {
     WindingRenderer::removeWinding(slot);
 }
 
-void OpenGLShader::updateWinding(Slot slot, const std::vector<ArbitraryMeshVertex>& vertices)
+void OpenGLShader::updateWinding(IWindingRenderer::Slot slot, const std::vector<ArbitraryMeshVertex>& vertices)
 {
     WindingRenderer::updateWinding(slot, vertices);
 }

radiantcore/rendersystem/backend/OpenGLShader.h

@@ -7,6 +7,7 @@
 #include "string/string.h"
 #include "render/IndexedVertexBuffer.h"
 #include "render/WindingRenderer.h"
+#include "SurfaceRenderer.h"
 
 #include <list>
 #include <sigc++/connection.h>
@@ -21,7 +22,8 @@ class OpenGLRenderSystem;
  */
 class OpenGLShader final : 
 	public Shader,
-    protected WindingRenderer
+    protected WindingRenderer,
+    protected SurfaceRenderer
 {
 private:
     // Name used to construct the shader
@@ -103,17 +105,22 @@ class OpenGLShader final :
 					   const LightSources* lights,
                        const IRenderEntity* entity) override;
 
-    void addSurface(const std::vector<ArbitraryMeshVertex>& vertices, const std::vector<unsigned int>& indices) override;
+    //void addSurface(const std::vector<ArbitraryMeshVertex>& vertices, const std::vector<unsigned int>& indices) override;
     bool hasSurfaces() const;
     void drawSurfaces();
 
+    ISurfaceRenderer::Slot addSurface(const std::vector<ArbitraryMeshVertex>& vertices, const std::vector<unsigned int>& indices) override;
+    void removeSurface(ISurfaceRenderer::Slot slot) override;
+    void updateSurface(ISurfaceRenderer::Slot slot, const std::vector<ArbitraryMeshVertex>& vertices,
+        const std::vector<unsigned int>& indices) override;
+
     void addGeometry(RenderableGeometry& geometry) override;
     bool hasGeometry() const;
     void clearGeometry();
 
-    Slot addWinding(const std::vector<ArbitraryMeshVertex>& vertices) override;
-    void removeWinding(Slot slot) override;
-    void updateWinding(Slot slot, const std::vector<ArbitraryMeshVertex>& vertices) override;
+    IWindingRenderer::Slot addWinding(const std::vector<ArbitraryMeshVertex>& vertices) override;
+    void removeWinding(IWindingRenderer::Slot slot) override;
+    void updateWinding(IWindingRenderer::Slot slot, const std::vector<ArbitraryMeshVertex>& vertices) override;
     bool hasWindings() const;
 
     void setVisible(bool visible) override;