Consolidate GPU buffer implementations

src/gpu/GrBuffer.h

-
 /*
- * Copyright 2011 Google Inc.
+ * Copyright 2016 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
-
-#ifndef GrGeometryBuffer_DEFINED
-#define GrGeometryBuffer_DEFINED
+#ifndef GrBuffer_DEFINED
+#define GrBuffer_DEFINED
 
 #include "GrGpuResource.h"
 
 class GrGpu;
 
-/**
- * Parent class for vertex and index buffers
- */
-class GrGeometryBuffer : public GrGpuResource {
+class GrBuffer : public GrGpuResource {
 public:
-    
+    /**
+     * Computes a scratch key for a buffer with a "dynamic" access pattern. (Buffers with "static"
+     * and "stream" access patterns are disqualified by nature from being cached and reused.)

[bsalomon, 2016/03/24 14:30:44]

why not stream?

[Chris Dalton, 2016/03/24 15:04:40]

Stream is defined as "specified once, and used at most a few times". If we
cached and reused it, it would be respecified many times.

We can always change it in the future though. (In fact, skia actually sends
GL_STREAM_DRAW when you ask for dynamic), I just intended for this change to be
a pure refactor with zero change in functionality.

[bsalomon, 2016/03/24 15:55:47]

Gotcha. I see you changed the alloc pool to request dynamic, so should have the
same behavior as before.

[Chris Dalton, 2016/03/24 16:07:06]

Yep. I traced every GL call for all of gm and tests, with multiple configs. The
200 MB of trace is still identical after applying this patch.

[Chris Dalton, 2016/03/24 18:14:10]

Done.

+     */
+    static void ComputeScratchKeyForDynamicBuffer(GrBufferType type, size_t size,
+                                                  GrScratchKey* key) {
+        static const GrScratchKey::ResourceType kType = GrScratchKey::GenerateResourceType();
+        if (uint32_t hi = size >> 32) {
+            GrScratchKey::Builder builder(key, kType, 3);
+            builder[0] = type;
+            builder[1] = size;
+            builder[2] = hi;
+        } else {
+            GrScratchKey::Builder builder(key, kType, 2);
+            // TODO: There's not always reason to cache a buffer by type. In some (all?) APIs it's
+            // just a chunk of memory we can use/reuse for any type of data. We really only need to
+            // differentiate between the "read" types (e.g. kGpuToCpu_BufferType) and "draw" types.
+            builder[0] = type;
+            builder[1] = size;
+        }
+    }
 
-    /**
-     *Retrieves whether the buffer was created with the dynamic flag
-     *
-     * @return true if the buffer was created with the dynamic flag
-     */
-    bool dynamic() const { return fDynamic; }
+    GrBufferType type() const { return fType; }
+
+    GrAccessPattern accessPattern() const { return fAccessPattern; }
 
     /**
      * Returns true if the buffer is a wrapper around a CPU array. If true it
      * indicates that map will always succeed and will be free.
      */
     bool isCPUBacked() const { return fCPUBacked; }
 
     /**
      * Maps the buffer to be written by the CPU.
      *
      * The previous content of the buffer is invalidated. It is an error
      * to draw from the buffer while it is mapped. It is an error to call map
      * on an already mapped buffer. It may fail if the backend doesn't support
      * mapping the buffer. If the buffer is CPU backed then it will always
      * succeed and is a free operation. Must be matched by an unmap() call.
      * Currently only one map at a time is supported (no nesting of
      * map/unmap).
      *
      * Note that buffer mapping does not go through GrContext and therefore is
      * not serialized with other operations.
      *
      * @return a pointer to the data or nullptr if the map fails.
      */
-     void* map() { return (fMapPtr = this->onMap()); }
+     void* map() {
+         this->onMap();

[bsalomon, 2016/03/24 14:30:44]

if (!fMapPtr) {
   this->onMap();
}
return fMapPtr

?

[Chris Dalton, 2016/03/24 15:04:40]

Good idea.

Now the comment above says: "It is an error to call map on an already mapped
buffer."

Would you prefer an assert here? Or just changing the comment? I don't care
either way.

[bsalomon, 2016/03/24 15:55:47]

I think we can just delete the comment. 

+         return fMapPtr;
+     }
 
     /**
      * Unmaps the buffer.
      *
      * The pointer returned by the previous map call will no longer be valid.
      */
      void unmap() {
          SkASSERT(fMapPtr);
          this->onUnmap();
          fMapPtr = nullptr;
@@ skipping 28 matching lines @@
      *
      * @return returns true if the update succeeds, false otherwise.
      */
     bool updateData(const void* src, size_t srcSizeInBytes) {
         SkASSERT(!this->isMapped());
         SkASSERT(srcSizeInBytes <= fGpuMemorySize);
         return this->onUpdateData(src, srcSizeInBytes);
     }
 
 protected:
-    GrGeometryBuffer(GrGpu* gpu, size_t gpuMemorySize, bool dynamic, bool cpuBacked)
-        : INHERITED(gpu, kCached_LifeCycle)
-        , fMapPtr(nullptr)
-        , fGpuMemorySize(gpuMemorySize)
-        , fDynamic(dynamic)
-        , fCPUBacked(cpuBacked) {}
+    GrBuffer(GrGpu* gpu, GrBufferType type, size_t gpuMemorySize, GrAccessPattern accessPattern,
+             bool cpuBacked)
+        : INHERITED(gpu, kCached_LifeCycle),
+          fMapPtr(nullptr),
+          fType(type),
+          fGpuMemorySize(gpuMemorySize), // TODO: Zero for cpu backed buffers?
+          fAccessPattern(accessPattern),
+          fCPUBacked(cpuBacked) {
+        if (!fCPUBacked && SkIsPow2(fGpuMemorySize) && kDynamic_GrAccessPattern == fAccessPattern) {
+            GrScratchKey key;
+            ComputeScratchKeyForDynamicBuffer(fType, fGpuMemorySize, &key);
+            this->setScratchKey(key);
+        }
+    }
+
+    void* fMapPtr;
 
 private:
     virtual size_t onGpuMemorySize() const { return fGpuMemorySize; }
 
-    virtual void* onMap() = 0;
+    virtual void onMap() = 0;
     virtual void onUnmap() = 0;
     virtual bool onUpdateData(const void* src, size_t srcSizeInBytes) = 0;
 
-    void*    fMapPtr;
-    size_t   fGpuMemorySize;
-    bool     fDynamic;
-    bool     fCPUBacked;
+    GrBufferType      fType;
+    size_t            fGpuMemorySize;
+    GrAccessPattern   fAccessPattern;
+    bool              fCPUBacked;
 
     typedef GrGpuResource INHERITED;
 };
 
 #endif