Added GpuBuffer::WritePolicy 81/290281/10
authorAdam Bialogonski <adam.b@samsung.com>
Thu, 20 Apr 2023 10:31:36 +0000 (11:31 +0100)
committerAdam Bialogonski <adam.b@samsung.com>
Fri, 12 May 2023 13:34:47 +0000 (14:34 +0100)
The write policy tells the GpuBuffer what to do with the Graphics::Buffer object
during updating buffer content. It's possible to:

- RETAIN the content
- DISCARD the content

Both options have performance implications. Retaining the content may cause blocking the access to the buffer by the GPU driver in case the buffer is currently in use by the GPU. Frequent buffer updates may cause a massive performance hit. This policy allows writing a partial updates into the buffer.

Discarding the content means orphaning an underlying memory and allocating new buffer storage so it can be accessed immediately while GPU may be finishing work with the old content. The downside of it is no partial updates are possible. Area of buffer which isn't updated stays undefined. This policy is made for frequent updates (every frame).

Change-Id: Ic0e80d7b23208a455129c16fb3e23a4020791b50

automated-tests/src/dali-internal/CMakeLists.txt
automated-tests/src/dali-internal/utc-Dali-Internal-GpuBuffer.cpp [new file with mode: 0644]
dali/internal/render/renderers/gpu-buffer.cpp
dali/internal/render/renderers/gpu-buffer.h
dali/internal/render/renderers/render-geometry.cpp
dali/internal/render/renderers/render-vertex-buffer.cpp

index 2308ba1..5655364 100644 (file)
@@ -13,6 +13,7 @@ SET(TC_SOURCES
         utc-Dali-Internal-FixedSizeMemoryPool.cpp
         utc-Dali-Internal-FrustumCulling.cpp
         utc-Dali-Internal-Gesture.cpp
+        utc-Dali-Internal-GpuBuffer.cpp
         utc-Dali-Internal-Handles.cpp
         utc-Dali-Internal-IndexedConstStringMap.cpp
         utc-Dali-Internal-IndexedIntegerMap.cpp
diff --git a/automated-tests/src/dali-internal/utc-Dali-Internal-GpuBuffer.cpp b/automated-tests/src/dali-internal/utc-Dali-Internal-GpuBuffer.cpp
new file mode 100644 (file)
index 0000000..8ee64bf
--- /dev/null
@@ -0,0 +1,135 @@
+/*
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include <dali-test-suite-utils.h>
+#include <mesh-builder.h>
+#include <stdlib.h>
+#include <memory>
+
+#include <dali/internal/render/renderers/gpu-buffer.h>
+
+// access private members
+#define private public
+#include <dali/internal/render/renderers/pipeline-cache.h>
+
+#include <dlfcn.h>
+
+using namespace Dali;
+
+template<class Object, class... Args>
+void InvokeNext(Object* obj, Args... args)
+{
+  auto    addr = __builtin_return_address(0);
+  Dl_info info;
+  dladdr(addr, &info);
+  auto func = dlsym(RTLD_NEXT, info.dli_sname);
+  typedef void (*FuncPtr)(void*, Args...);
+  auto memb = FuncPtr(func);
+  memb(obj, args...);
+}
+
+template<class Ret, class Object, class... Args>
+Ret InvokeReturnNext(Object* obj, Args... args)
+{
+  auto    addr = __builtin_return_address(0);
+  Dl_info info;
+  dladdr(addr, &info);
+  auto func = dlsym(RTLD_NEXT, info.dli_sname);
+  typedef Ret (*FuncPtr)(void*, Args...);
+  auto memb = FuncPtr(func);
+  return memb(obj, args...);
+}
+
+namespace Dali
+{
+namespace Internal
+{
+namespace Render
+{
+} // namespace Render
+} // namespace Internal
+} // namespace Dali
+
+int UtcDaliCoreGpuBufferDiscardWritePolicy(void)
+{
+  TestApplication application;
+  tet_infoline("Testing Dali::Internal::Render::GpuBuffer WritePolicy::DISCARD");
+
+  TestGraphicsController& controller = application.GetGraphicsController();
+
+  // Create GPU Buffer with Discard policy and as a vertex buffer
+  std::unique_ptr<Internal::GpuBuffer> buffer = std::make_unique<Internal::GpuBuffer>(controller,
+                                                                                      0u | Graphics::BufferUsage::VERTEX_BUFFER,
+                                                                                      Internal::GpuBuffer::WritePolicy::DISCARD);
+
+  // should be 0 as unset yet
+  DALI_TEST_EQUALS(buffer->GetBufferSize(), 0, TEST_LOCATION);
+
+  std::vector<uint8_t> data;
+  data.resize(1000000);
+  buffer->UpdateDataBuffer(controller, 1000000, data.data());
+
+  // we should have valid Graphics::Buffer now
+  auto ptr0    = buffer->GetGraphicsObject();
+  auto result0 = (ptr0 != nullptr);
+  DALI_TEST_EQUALS(result0, true, TEST_LOCATION);
+
+  // now write again, with DISCARD policy, we should get totally new Graphics object
+  buffer->UpdateDataBuffer(controller, 1000000, data.data());
+
+  auto ptr1    = buffer->GetGraphicsObject();
+  auto result1 = (ptr0 != nullptr);
+  DALI_TEST_EQUALS(result1, true, TEST_LOCATION);
+  DALI_TEST_NOT_EQUALS(ptr0, ptr1, 0, TEST_LOCATION);
+
+  END_TEST;
+}
+
+int UtcDaliCoreGpuBufferRetainWritePolicy(void)
+{
+  TestApplication application;
+  tet_infoline("Testing Dali::Internal::Render::GpuBuffer WritePolicy::RETAIN");
+
+  TestGraphicsController& controller = application.GetGraphicsController();
+
+  // Create GPU Buffer with Discard policy and as a vertex buffer
+  std::unique_ptr<Internal::GpuBuffer> buffer = std::make_unique<Internal::GpuBuffer>(controller,
+                                                                                      0u | Graphics::BufferUsage::VERTEX_BUFFER,
+                                                                                      Internal::GpuBuffer::WritePolicy::RETAIN);
+
+  // should be 0 as unset yet
+  DALI_TEST_EQUALS(buffer->GetBufferSize(), 0, TEST_LOCATION);
+
+  std::vector<uint8_t> data;
+  data.resize(1000000);
+  buffer->UpdateDataBuffer(controller, 1000000, data.data());
+
+  // we should have valid Graphics::Buffer now
+  auto ptr0    = buffer->GetGraphicsObject();
+  auto result0 = (ptr0 != nullptr);
+  DALI_TEST_EQUALS(result0, true, TEST_LOCATION);
+
+  // with RETAIN policy, the buffer shouldn't change as long it uses same spec)
+  buffer->UpdateDataBuffer(controller, 1000000, data.data());
+
+  auto ptr1    = buffer->GetGraphicsObject();
+  auto result1 = (ptr0 != nullptr);
+  DALI_TEST_EQUALS(result1, true, TEST_LOCATION);
+  DALI_TEST_EQUALS(ptr0, ptr1, 0, TEST_LOCATION);
+
+  END_TEST;
+}
\ No newline at end of file
index 0d2003a..11cc2a9 100644 (file)
@@ -31,8 +31,9 @@ namespace
 {
 } // namespace
 
-GpuBuffer::GpuBuffer(Graphics::Controller& graphicsController, Graphics::BufferUsageFlags usage)
-: mUsage(usage)
+GpuBuffer::GpuBuffer(Graphics::Controller& graphicsController, Graphics::BufferUsageFlags usage, GpuBuffer::WritePolicy writePolicy)
+: mUsage(usage),
+  mWritePolicy(writePolicy)
 {
 }
 
@@ -41,7 +42,18 @@ void GpuBuffer::UpdateDataBuffer(Graphics::Controller& graphicsController, uint3
   DALI_ASSERT_DEBUG(size > 0);
   mSize = size;
 
-  if(!mGraphicsObject || size > mCapacity)
+  /**
+   *  We will create a new buffer in following cases:
+   *  1. The buffer doesn't exist
+   *  2. The spec changes (size of new buffer changes)
+   *  3. The policy is to discard on write
+   *
+   *  Third option will try to recycle existing buffer when its spec stays unchanged.
+   *  It will allow the GPU driver to release the underlying memory and attach new memory
+   *  to the GPU buffer object. It will prevent memory locking during writes in case the
+   *  buffer is in use by the GPU.
+   */
+  if(!mGraphicsObject || size > mCapacity || mWritePolicy == WritePolicy::DISCARD)
   {
     Graphics::BufferCreateInfo createInfo{};
     createInfo.SetUsage(mUsage).SetSize(size);
index 42b2a75..645480a 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_INTERNAL_RENDERERS_GPU_BUFFER_H
 
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,11 +39,30 @@ class GpuBuffer
 {
 public:
   /**
+   * When writing into the buffer, the WritePolicy
+   * determines whether the current content would be preserved
+   * or discarded.
+   *
+   * RETAIN - buffer content is retained
+   *
+   * DISCARD - buffer content is discarded. In this case, writing into
+   *           a part of buffer will result with undefined content outside
+   *           the specified area. The client should rewrite whole area
+   *           in order to have coherent and valid data.
+   */
+  enum class WritePolicy
+  {
+    RETAIN, ///< Buffer content is preserved
+    DISCARD ///< Buffer content is invalidated and discarded
+  };
+
+  /**
    * constructor
    * @param[in] graphicsController the graphics controller
    * @param[in] usage The type of buffer
+   * @param[in] writePolicy The buffer data write policy to be used, default is WritePolicy::RETAIN
    */
-  GpuBuffer(Graphics::Controller& graphicsController, Graphics::BufferUsageFlags usage);
+  GpuBuffer(Graphics::Controller& graphicsController, Graphics::BufferUsageFlags usage, GpuBuffer::WritePolicy writePolicy);
 
   /**
    * Destructor, non virtual as no virtual methods or inheritance
@@ -61,7 +80,7 @@ public:
 
   /**
    * Get the size of the buffer
-   * @return size
+   * @return size Size of the buffer in bytes
    */
   [[nodiscard]] uint32_t GetBufferSize() const
   {
@@ -83,6 +102,7 @@ private:
   uint32_t                              mCapacity{0}; ///< buffer capacity
   uint32_t                              mSize{0};     ///< buffer size
   Graphics::BufferUsageFlags            mUsage;
+  WritePolicy                           mWritePolicy{WritePolicy::RETAIN}; ///< data write policy for the buffer
 };
 
 } // namespace Internal
index 83be141..03afcd7 100644 (file)
@@ -127,7 +127,8 @@ void Geometry::Upload(Graphics::Controller& graphicsController)
       {
         if(mIndexBuffer == nullptr)
         {
-          mIndexBuffer = new GpuBuffer(graphicsController, 0 | Graphics::BufferUsage::INDEX_BUFFER);
+          // Currently we are unable to reuse index buffer so the write policy is to preserve current content
+          mIndexBuffer = new GpuBuffer(graphicsController, 0 | Graphics::BufferUsage::INDEX_BUFFER, GpuBuffer::WritePolicy::RETAIN);
         }
 
         uint32_t bufferSize = static_cast<uint32_t>(sizeof(uint16_t) * mIndices.Size());
index b140cef..a383551 100644 (file)
@@ -62,7 +62,7 @@ bool VertexBuffer::Update(Graphics::Controller& graphicsController)
   {
     if(!mGpuBuffer)
     {
-      mGpuBuffer = new GpuBuffer(graphicsController, 0 | Graphics::BufferUsage::VERTEX_BUFFER);
+      mGpuBuffer = new GpuBuffer(graphicsController, 0 | Graphics::BufferUsage::VERTEX_BUFFER, GpuBuffer::WritePolicy::DISCARD);
     }
 
     // Update the GpuBuffer