Added lockless synchronization to VertexBufferUpdateCallback 14/294514/2
authorAdam Bialogonski <adam.b@samsung.com>
Tue, 20 Jun 2023 11:23:55 +0000 (12:23 +0100)
committerAdam Bialogonski <adam.b@samsung.com>
Tue, 20 Jun 2023 15:46:30 +0000 (16:46 +0100)
Change-Id: I3ae62318868840403b9619c02e5c98fbcd7c9766

automated-tests/src/dali/utc-Dali-VertexBuffer.cpp
dali/internal/event/rendering/vertex-buffer-impl.cpp
dali/internal/event/rendering/vertex-buffer-impl.h
dali/internal/render/renderers/render-vertex-buffer.cpp
dali/internal/render/renderers/render-vertex-buffer.h
dali/internal/update/manager/update-manager.h
dali/public-api/rendering/vertex-buffer.cpp
dali/public-api/rendering/vertex-buffer.h

index 71b7f92..605cbd5 100644 (file)
@@ -28,6 +28,7 @@ struct VertexBufferUpdater
 {
   struct Diagnostics
   {
+    bool     success{false};
     uint32_t counter{0};
     void*    lastPtr{nullptr};
     size_t   lastSize{0};
@@ -38,6 +39,7 @@ struct VertexBufferUpdater
 
   uint32_t UpdateVertices(void* ptr, size_t size)
   {
+    diagnostics.success      = true;
     diagnostics.lastPtr      = ptr;
     diagnostics.lastSize     = size;
     diagnostics.lastReturned = returnSize;
@@ -71,6 +73,23 @@ struct VertexBufferUpdater
     return value;
   }
 
+  Diagnostics GetValueWithTimeout()
+  {
+    auto future = promise.get_future();
+    auto status = future.wait_for(1s);
+    if(status == std::future_status::ready)
+    {
+      promise = {};
+      return promise.get_future().get();
+    }
+
+    promise = {};
+    // reset promise automatically
+    Diagnostics failure{};
+    failure.success = false;
+    return failure;
+  }
+
   bool IsValueReady()
   {
     // fake-wait for two frames
@@ -629,3 +648,77 @@ int UtcDaliVertexBufferUpdateCallback(void)
 
   END_TEST;
 }
+
+int UtcDaliSetAndRemoveVertexBufferUpdateCallback(void)
+{
+  TestApplication application;
+
+  // Create vertex buffer
+  VertexBuffer vertexBuffer = VertexBuffer::New(Property::Map() = {
+                                                  {"aPosition", Property::Type::VECTOR2},
+                                                  {"aTexCoord", Property::Type::VECTOR2}});
+
+  // set callback
+  auto callback = std::make_unique<VertexBufferUpdater>();
+  vertexBuffer.SetVertexBufferUpdateCallback(callback->CreateCallback());
+
+  struct Vertex
+  {
+    Vector2 pos;
+    Vector2 uv;
+  };
+
+  std::vector<Vertex> vertices;
+  vertices.resize(16);
+  vertexBuffer.SetData(vertices.data(), 16);
+
+  Geometry geometry = Geometry::New();
+  geometry.AddVertexBuffer(vertexBuffer);
+  Shader   shader   = CreateShader();
+  Renderer renderer = Renderer::New(geometry, shader);
+  Actor    actor    = Actor::New();
+  actor.SetProperty(Actor::Property::SIZE, Vector3::ONE * 100.f);
+  actor.AddRenderer(renderer);
+  application.GetScene().Add(actor);
+
+  auto& gl    = application.GetGlAbstraction();
+  auto& trace = gl.GetDrawTrace();
+  trace.Enable(true);
+  trace.EnableLogging(true);
+
+  callback->SetCallbackReturnValue(16 * sizeof(Vertex));
+
+  application.SendNotification();
+  application.Render();
+
+  auto value = callback->GetValue();
+
+  // Test whether callback ran
+  DALI_TEST_EQUALS(value.counter, 1, TEST_LOCATION);
+  DALI_TEST_EQUALS(value.lastSize, 16 * sizeof(Vertex), TEST_LOCATION);
+  DALI_TEST_EQUALS(value.lastReturned, 16 * sizeof(Vertex), TEST_LOCATION);
+  DALI_TEST_NOT_EQUALS(value.lastPtr, (void*)nullptr, 0, TEST_LOCATION);
+
+  // test whether draw call has been issued (return value indicates end of array to be drawn)
+  auto result = trace.FindMethod("DrawArrays");
+  DALI_TEST_EQUALS(result, true, TEST_LOCATION);
+  result = trace.FindMethodAndParams("DrawArrays", "4, 0, 16");
+  DALI_TEST_EQUALS(result, true, TEST_LOCATION);
+
+  // Test 2. Update and render only half of vertex buffer
+  callback->SetCallbackReturnValue(8 * sizeof(Vertex));
+  trace.Reset();
+
+  // Remove the callback
+  vertexBuffer.ClearVertexBufferUpdateCallback();
+
+  application.SendNotification();
+  application.Render();
+
+  // Use 1sec timeout as callback won't be executed and future won't be filled
+  value = callback->GetValueWithTimeout();
+  // Test whether callback ran
+  DALI_TEST_EQUALS(value.success, false, TEST_LOCATION);
+
+  END_TEST;
+}
index b1b58a0..73e67a5 100644 (file)
@@ -184,7 +184,28 @@ uint32_t VertexBuffer::GetDivisor() const
 
 void VertexBuffer::SetVertexBufferUpdateCallback(VertexBufferUpdateCallback& callback)
 {
-  SceneGraph::SetVertexBufferUpdateCallback(mEventThreadServices.GetUpdateManager(), *mRenderObject, &callback);
+  if(mVertexBufferUpdateCallback)
+  {
+    ClearVertexBufferUpdateCallback();
+  }
+  mVertexBufferUpdateCallback = &callback;
+  SceneGraph::SetVertexBufferUpdateCallbackMessage(mEventThreadServices.GetUpdateManager(), *mRenderObject, &callback);
+}
+
+void VertexBuffer::ClearVertexBufferUpdateCallback()
+{
+  if(mVertexBufferUpdateCallback)
+  {
+    if(mRenderObject)
+    {
+      // Sets nullptr as callback. This function invokes SetVertexBufferUpdateCallback() directly
+      // on the render object bypassing the message queue. The implicit synchronization provides
+      // thread-safety. The synchronization mechanism uses atomic state changes and spin-lock.
+      // Spin-lock prevents from rescheduling thread execution.
+      mRenderObject->SetVertexBufferUpdateCallback(nullptr);
+    }
+    mVertexBufferUpdateCallback = nullptr;
+  }
 }
 
 const Render::VertexBuffer* VertexBuffer::GetRenderObject() const
index 17f3b62..1dd8fcb 100644 (file)
@@ -69,10 +69,15 @@ public:
   uint32_t GetDivisor() const;
 
   /**
-   * @copydoc Dali::VertexBuffer::SetVertexBufferUpdateCallback()
+   * @copydoc Dali::VertexBuffer::SetVertexBufferUpdateCallbackMessage()
    */
   void SetVertexBufferUpdateCallback(VertexBufferUpdateCallback& callback);
 
+  /**
+   * @copydoc Dali::VertexBuffer::ClearVertexBufferUpdateCallback()
+   */
+  void ClearVertexBufferUpdateCallback();
+
 public: // Default property extensions from Object
   /**
    * @brief Get the render thread side of the VertexBuffer
@@ -102,12 +107,13 @@ private: // unimplemented methods
   VertexBuffer(const VertexBuffer&);
   VertexBuffer& operator=(const VertexBuffer&);
 
-private:                                        // data
-  EventThreadServices&  mEventThreadServices;   ///<Used to send messages to the render thread via update thread
-  Render::VertexBuffer* mRenderObject{nullptr}; ///<Render side object
-  uint32_t              mBufferFormatSize{0};
-  uint32_t              mSize{0};    ///< Number of elements in the buffer
-  uint32_t              mDivisor{0}; ///< How many elements to skip in instanced draw
+private:                                                            // data
+  EventThreadServices&        mEventThreadServices;                 ///<Used to send messages to the render thread via update thread
+  Render::VertexBuffer*       mRenderObject{nullptr};               ///<Render side object
+  VertexBufferUpdateCallback* mVertexBufferUpdateCallback{nullptr}; ///<Vertex buffer update callback pointer
+  uint32_t                    mBufferFormatSize{0};
+  uint32_t                    mSize{0};    ///< Number of elements in the buffer
+  uint32_t                    mDivisor{0}; ///< How many elements to skip in instanced draw
 };
 
 /**
index 0ccf583..d899f7b 100644 (file)
@@ -54,7 +54,9 @@ void VertexBuffer::SetData(Dali::Vector<uint8_t>* data, uint32_t size)
 
 void VertexBuffer::SetVertexBufferUpdateCallback(Dali::VertexBufferUpdateCallback* callback)
 {
+  mVertexBufferStateLock.ChangeState(VertexBufferSyncState::UNLOCKED, VertexBufferSyncState::LOCKED_FOR_EVENT);
   mVertexBufferUpdateCallback.reset(callback);
+  mVertexBufferStateLock.ChangeState(VertexBufferSyncState::LOCKED_FOR_EVENT, VertexBufferSyncState::UNLOCKED);
 }
 
 bool VertexBuffer::Update(Graphics::Controller& graphicsController)
@@ -89,6 +91,7 @@ bool VertexBuffer::Update(Graphics::Controller& graphicsController)
   }
 
   // To execute the callback the buffer must be already initialized.
+  mVertexBufferStateLock.ChangeState(VertexBufferSyncState::UNLOCKED, VertexBufferSyncState::LOCKED_FOR_UPDATE);
   if(mVertexBufferUpdateCallback && mGpuBuffer)
   {
     // If running callback, we may end up with less elements in the buffer
@@ -97,7 +100,7 @@ bool VertexBuffer::Update(Graphics::Controller& graphicsController)
     mGpuBuffer->UpdateDataBufferWithCallback(graphicsController, mVertexBufferUpdateCallback.get(), updatedSize);
     mElementCount = updatedSize / mFormat->size;
   }
-
+  mVertexBufferStateLock.ChangeState(VertexBufferSyncState::LOCKED_FOR_UPDATE, VertexBufferSyncState::UNLOCKED);
   return true;
 }
 
index e007977..6382537 100644 (file)
@@ -33,6 +33,34 @@ namespace Internal
 {
 namespace Render
 {
+/**
+ * Helper class using atomic swaps for lockless synchronization
+ */
+template<class T>
+struct StateLock
+{
+  StateLock(T v)
+  {
+    value = v;
+  }
+
+  /**
+   * Attempts to change state 'from' to 'to' and repeats
+   * when no success
+   */
+  void ChangeState(T from, T to)
+  {
+    // using spin lock
+    T expected = from;
+    while(!value.compare_exchange_weak(expected, to, std::memory_order_release, std::memory_order_relaxed))
+    {
+      expected = from; // it's needed to revert the value of 'expected'
+    };
+  }
+
+  std::atomic<T> value{};
+};
+
 class VertexBuffer
 {
 public:
@@ -188,6 +216,22 @@ private:
   uint32_t                                          mElementCount; ///< Number of valid elements in the buffer
   std::unique_ptr<Dali::VertexBufferUpdateCallback> mVertexBufferUpdateCallback;
 
+  /**
+   * Enum values for locking mechanism.
+   *
+   * Locking mechanism uses synchronized state machine. From 'UNLOCKED' state
+   * it's either LOCKED_FOR_EVENT or LOCKED_FOR_UPDATE states possible.
+   * Both states can be reverted only to UNLOCKED.
+   */
+  enum class VertexBufferSyncState : int
+  {
+    UNLOCKED,          ///< Currently unlocked
+    LOCKED_FOR_EVENT,  ///< Locked for Event thread to access
+    LOCKED_FOR_UPDATE, ///< Locked for Update thread to access
+  };
+
+  StateLock<VertexBufferSyncState> mVertexBufferStateLock{VertexBufferSyncState::UNLOCKED};
+
   bool mDataChanged; ///< Flag to know if data has changed in a frame
 };
 
index b42bc15..a7406cc 100644 (file)
@@ -1289,7 +1289,7 @@ inline void SetVertexBufferDivisorMessage(UpdateManager& manager, Render::Vertex
   new(slot) LocalType(&manager, &UpdateManager::SetVertexBufferDivisor, &vertexBuffer, divisor);
 }
 
-inline void SetVertexBufferUpdateCallback(UpdateManager& manager, Render::VertexBuffer& vertexBuffer, Dali::VertexBufferUpdateCallback* callback)
+inline void SetVertexBufferUpdateCallbackMessage(UpdateManager& manager, Render::VertexBuffer& vertexBuffer, Dali::VertexBufferUpdateCallback* callback)
 {
   // Message has ownership of VertexBuffer data while in transit from event -> update
   using LocalType = MessageValue2<UpdateManager, Render::VertexBuffer*, Dali::VertexBufferUpdateCallback*>;
index 69ac95a..90c3afe 100644 (file)
@@ -31,7 +31,7 @@ struct VertexBufferUpdateCallback::Impl
   {
   }
 
-  uint32_t Invoke(void* data, size_t size)
+  uint32_t Invoke(void* data, size_t size) const
   {
     return CallbackBase::ExecuteReturn<uint32_t>(*mCallback, data, size);
   }
@@ -112,6 +112,11 @@ void VertexBuffer::SetVertexBufferUpdateCallback(std::unique_ptr<VertexBufferUpd
   GetImplementation(*this).SetVertexBufferUpdateCallback(*updateCallback.release());
 }
 
+void VertexBuffer::ClearVertexBufferUpdateCallback()
+{
+  GetImplementation(*this).ClearVertexBufferUpdateCallback();
+}
+
 VertexBuffer::VertexBuffer(Internal::VertexBuffer* pointer)
 : BaseHandle(pointer)
 {
index c8dad49..9ba49d3 100644 (file)
@@ -267,6 +267,13 @@ public:
    */
   void SetVertexBufferUpdateCallback(std::unique_ptr<VertexBufferUpdateCallback>&& updateCallback);
 
+  /**
+   * @brief Clears attached vertex buffer update callback
+   *
+   * This function provides implicit thread safety.
+   */
+  void ClearVertexBufferUpdateCallback();
+
 public:
   /**
    * @brief The constructor.