DrawableProxy2D operation fixes. Should fix crashes with worker threads and sprite flickering. Note that there is no frustum culling for Drawable2D's, as the vertex buffer for all of them is generated at once, and culling would be problematic for multiple views. Closes #252.

Source/Engine/Graphics/Geometry.cpp

@@ -135,11 +135,12 @@ bool Geometry::SetDrawRange(PrimitiveType type, unsigned indexStart, unsigned in
     return true;
 }
 
-bool Geometry::SetDrawRange(PrimitiveType type, unsigned indexStart, unsigned indexCount, unsigned minVertex, unsigned vertexCount)
+bool Geometry::SetDrawRange(PrimitiveType type, unsigned indexStart, unsigned indexCount, unsigned minVertex, unsigned vertexCount, bool checkIllegal)
 {
     if (indexBuffer_)
     {
-        if (indexStart + indexCount > indexBuffer_->GetIndexCount())
+        // We can allow setting an illegal draw range now if the caller guarantees to resize / fill the buffer later
+        if (checkIllegal && indexStart + indexCount > indexBuffer_->GetIndexCount())
         {
             LOGERROR("Illegal draw range " + String(indexStart) + " to " + String(indexStart + indexCount - 1) +
                 ", index buffer has " + String(indexBuffer_->GetIndexCount()) + " indices");

Source/Engine/Graphics/Geometry.h

@@ -54,7 +54,7 @@ public:
     /// Set the draw range.
     bool SetDrawRange(PrimitiveType type, unsigned indexStart, unsigned indexCount, bool getUsedVertexRange = true);
     /// Set the draw range.
-    bool SetDrawRange(PrimitiveType type, unsigned indexStart, unsigned indexCount, unsigned vertexStart, unsigned vertexCount);
+    bool SetDrawRange(PrimitiveType type, unsigned indexStart, unsigned indexCount, unsigned vertexStart, unsigned vertexCount, bool checkIllegal = true);
     /// Set the LOD distance.
     void SetLodDistance(float distance);
     /// Override raw vertex data to be returned for CPU-side operations.

Source/Engine/Urho2D/DrawableProxy2D.cpp

@@ -26,10 +26,12 @@
 #include "Drawable2D.h"
 #include "DrawableProxy2D.h"
 #include "Geometry.h"
+#include "GraphicsEvents.h"
 #include "IndexBuffer.h"
 #include "Log.h"
 #include "Material.h"
 #include "Node.h"
+#include "Profiler.h"
 #include "Scene.h"
 #include "VertexBuffer.h"
 #include "Sort.h"
@@ -43,8 +45,11 @@ DrawableProxy2D::DrawableProxy2D(Context* context) :
     Drawable(context, DRAWABLE_GEOMETRY),
     indexBuffer_(new IndexBuffer(context_)),
     vertexBuffer_(new VertexBuffer(context_)),
+    vertexCount_(0),
+    indexCount_(0),
     orderDirty_(true)
 {
+    SubscribeToEvent(E_BEGINVIEWUPDATE, HANDLER(DrawableProxy2D, HandleBeginViewUpdate));
 }
 
 DrawableProxy2D::~DrawableProxy2D()
@@ -56,48 +61,117 @@ void DrawableProxy2D::RegisterObject(Context* context)
     context->RegisterFactory<DrawableProxy2D>();
 }
 
-void DrawableProxy2D::UpdateBatches(const FrameInfo& frame)
+void DrawableProxy2D::HandleBeginViewUpdate(StringHash eventType, VariantMap& eventData)
 {
+    using namespace BeginViewUpdate;
+    
+    // Check that we are updating the correct scene
+    if (GetScene() != eventData[P_SCENE].GetPtr())
+        return;
+    
+    PROFILE(UpdateDrawableProxy2D);
+    
+    if (orderDirty_)
+    {
+        Sort(drawables_.Begin(), drawables_.End(), CompareDrawable2Ds);
+        orderDirty_ = false;
+    }
+    
+    vertexCount_ = 0;
+    
+    if (drawablesVisible_.Size() != drawables_.Size())
+        drawablesVisible_.Resize(drawables_.Size());
+    
+    /// \todo We could add frustum culling, but that would be problematic if we have several viewports. Right now all Drawable2D's
+    /// are always submitted for rendering
+    for (unsigned i = 0; i < drawables_.Size(); ++i)
+    {
+        Material* usedMaterial = drawables_[i]->GetUsedMaterial();
+        const Vector<Vertex2D>& vertices = drawables_[i]->GetVertices();
+        if (drawables_[i]->GetUsedMaterial() && vertices.Size())
+        {
+            drawablesVisible_[i] = true;
+            vertexCount_ += vertices.Size();
+        }
+        else
+            drawablesVisible_[i] = false;
+    }
+    
+    indexCount_ = vertexCount_ / 4 * 6;
+    
+    // Go through the drawables to form geometries & batches, but upload the actual vertex data later
+    materials_.Clear();
+    
+    Material* material = 0;
+    unsigned iStart = 0;
+    unsigned iCount = 0;
+    unsigned vStart = 0;
+    unsigned vCount = 0;
+
+    for (unsigned d = 0; d < drawables_.Size(); ++d)
+    {
+        if (!drawablesVisible_[d])
+            continue;
+        
+        Material* usedMaterial = drawables_[d]->GetUsedMaterial();
+        const Vector<Vertex2D>& vertices = drawables_[d]->GetVertices();
+
+        if (material != usedMaterial)
+        {
+            if (material)
+            {
+                AddBatch(material, iStart, iCount, vStart, vCount);
+                iStart += iCount;
+                iCount = 0;
+                vStart += vCount;
+                vCount = 0;
+            }
+
+            material = usedMaterial;
+        }
+
+        iCount += vertices.Size() / 4 * 6;
+        vCount += vertices.Size();
+    }
+
+    if (material)
+        AddBatch(material, iStart, iCount, vStart, vCount);
+
+    // Now the amount of batches is known. Build the part of source batches that are sensitive to threading issues
+    // (material & geometry pointers)
     unsigned count = materials_.Size();
     batches_.Resize(count);
 
     for (unsigned i = 0; i < count; ++i)
     {
-        batches_[i].distance_ = 10.0f + (count - i) * 0.001f;
         batches_[i].material_ = materials_[i];
         batches_[i].geometry_ = geometries_[i];
-        batches_[i].worldTransform_ = &Matrix3x4::IDENTITY;
     }
 }
 
-void DrawableProxy2D::UpdateGeometry(const FrameInfo& frame)
+void DrawableProxy2D::UpdateBatches(const FrameInfo& frame)
 {
-    materials_.Clear();
+    unsigned count = batches_.Size();
 
-    if (orderDirty_)
+    // Update non-thread critical parts of the source batches
+    for (unsigned i = 0; i < count; ++i)
     {
-        Sort(drawables_.Begin(), drawables_.End(), CompareDrawable2Ds);
-        orderDirty_ = false;
+        batches_[i].distance_ = 10.0f + (count - i) * 0.001f;
+        batches_[i].worldTransform_ = &Matrix3x4::IDENTITY;
     }
+}
 
-    float timeStep = frame.timeStep_;
-
-    unsigned vertexCount = 0;
-    for (unsigned i = 0; i < drawables_.Size(); ++i)
-        vertexCount += drawables_[i]->GetVertices().Size();
-
-    if (vertexCount == 0)
-        return;
-
-    unsigned indexCount = vertexCount / 4 * 6;
-    if (indexBuffer_->GetIndexCount() < indexCount)
+void DrawableProxy2D::UpdateGeometry(const FrameInfo& frame)
+{
+    // Fill index buffer
+    if (indexBuffer_->GetIndexCount() < indexCount_)
     {
-        bool largeIndices = indexCount > 0xffff;
-        indexBuffer_->SetSize(indexCount, largeIndices, true);
-        void* buffer = indexBuffer_->Lock(0, indexCount, true);
+        bool largeIndices = indexCount_ > 0xffff;
+        indexBuffer_->SetSize(indexCount_, largeIndices, true);
+        void* buffer = indexBuffer_->Lock(0, indexCount_, true);
         if (buffer)
         {
-            unsigned quadCount = indexCount / 6;
+            unsigned quadCount = indexCount_ / 6;
             if (largeIndices)
             {
                 unsigned* dest = reinterpret_cast<unsigned*>(buffer);
@@ -138,54 +212,36 @@ void DrawableProxy2D::UpdateGeometry(const FrameInfo& frame)
         }
     }
 
-    if (vertexBuffer_->GetVertexCount() < vertexCount)
-        vertexBuffer_->SetSize(vertexCount, MASK_VERTEX2D);
+    if (vertexBuffer_->GetVertexCount() < vertexCount_)
+        vertexBuffer_->SetSize(vertexCount_, MASK_VERTEX2D);
 
-    Vertex2D* dest = reinterpret_cast<Vertex2D*>(vertexBuffer_->Lock(0, vertexCount, true));
-    if (dest)
+    if (vertexCount_)
     {
-        Material* material = 0;
-        unsigned iStart = 0;
-        unsigned iCount = 0;
-        unsigned vStart = 0;
-        unsigned vCount = 0;
-
-        for (unsigned d = 0; d < drawables_.Size(); ++d)
+        Vertex2D* dest = reinterpret_cast<Vertex2D*>(vertexBuffer_->Lock(0, vertexCount_, true));
+        if (dest)
         {
-            Material* usedMaterial = drawables_[d]->GetUsedMaterial();
-            const Vector<Vertex2D>& vertices = drawables_[d]->GetVertices();
-            if (!usedMaterial || vertices.Empty())
-                continue;
+            Material* material = 0;
+            unsigned iStart = 0;
+            unsigned iCount = 0;
+            unsigned vStart = 0;
+            unsigned vCount = 0;
 
-            if (material != usedMaterial)
+            for (unsigned d = 0; d < drawables_.Size(); ++d)
             {
-                if (material)
-                {
-                    AddBatch(material, iStart, iCount, vStart, vCount);
-                    iStart += iCount;
-                    iCount = 0;
-                    vStart += vCount;
-                    vCount = 0;
-                }
-
-                material = usedMaterial;
+                if (!drawablesVisible_[d])
+                    continue;
+                
+                const Vector<Vertex2D>& vertices = drawables_[d]->GetVertices();
+                for (unsigned i = 0; i < vertices.Size(); ++i)
+                    dest[i] = vertices[i];
+                dest += vertices.Size();
             }
-
-            for (unsigned i = 0; i < vertices.Size(); ++i)
-                dest[i] = vertices[i];
-            dest += vertices.Size();
-
-            iCount += vertices.Size() / 4 * 6;
-            vCount += vertices.Size();
+            
+            vertexBuffer_->Unlock();
         }
-
-        if (material)
-            AddBatch(material, iStart, iCount, vStart, vCount);
-
-        vertexBuffer_->Unlock();
+        else
+            LOGERROR("Failed to lock vertex buffer");
     }
-    else
-        LOGERROR("Failed to lock vertex buffer");
 }
 
 UpdateGeometryType DrawableProxy2D::GetUpdateGeometryType()
@@ -237,7 +293,7 @@ void DrawableProxy2D::AddBatch(Material* material, unsigned indexStart, unsigned
         geometries_.Push(geometry);
     }
 
-    geometries_[batchSize - 1]->SetDrawRange(TRIANGLE_LIST, indexStart, indexCount, vertexStart, vertexCount);
+    geometries_[batchSize - 1]->SetDrawRange(TRIANGLE_LIST, indexStart, indexCount, vertexStart, vertexCount, false);
 }
 
 }

Source/Engine/Urho2D/DrawableProxy2D.h

@@ -64,6 +64,10 @@ protected:
     /// Add batch.
     void AddBatch(Material* material, unsigned indexStart, unsigned indexCount, unsigned vertexStart, unsigned vertexCount);
 
+private:
+    /// Handle view update begin event. Determine Drawable2D's and their batches here.
+    void HandleBeginViewUpdate(StringHash eventType, VariantMap& eventData);
+    
     /// Index buffer.
     SharedPtr<IndexBuffer> indexBuffer_;
     /// Vertex buffer.
@@ -74,6 +78,12 @@ protected:
     Vector<SharedPtr<Geometry> > geometries_;
     /// Drawables.
     PODVector<Drawable2D* > drawables_;
+    /// Drawable visibility results.
+    PODVector<bool> drawablesVisible_;
+    /// Total vertex count for the current frame.
+    unsigned vertexCount_;
+    /// Total index count for the current frame.
+    unsigned indexCount_;
     /// Order dirty.
     bool orderDirty_;
 };