Consolidate GPU buffer implementations

src/gpu/GrBuffer.h

Index: src/gpu/GrBuffer.h
diff --git a/src/gpu/GrGeometryBuffer.h b/src/gpu/GrBuffer.h
similarity index 55%
rename from src/gpu/GrGeometryBuffer.h
rename to src/gpu/GrBuffer.h
index 56a6cae3fb08f32f8e3e7fcbbb276ba81620b858..cf12d8bd29022ac1625450035571a4a52830f035 100644
--- a/src/gpu/GrGeometryBuffer.h
+++ b/src/gpu/GrBuffer.h
@@ -1,32 +1,44 @@
-
 /*
- * 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:
-    
-
     /**
-     *Retrieves whether the buffer was created with the dynamic flag
-     *
-     * @return true if the buffer was created with the dynamic flag
+     * 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.

      */
-    bool dynamic() const { return fDynamic; }
+    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;
+        }
+    }
+
+    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
@@ -50,7 +62,10 @@ public:
      *
      * @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.
@@ -99,24 +114,34 @@ public:
     }
 
 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;
 };