More framework support for TransferBuffers

src/gpu/gl/GrGLGpu.cpp

Index: src/gpu/gl/GrGLGpu.cpp
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index a83386840e68c959efb43d57c03d80878d3d6875..cbede382f216ece5ba5442f088dc9216844a7d8b 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -648,6 +648,26 @@ bool GrGLGpu::onWritePixels(GrSurface* surface,
     return false;
 }
 
+bool GrGLGpu::onTransferPixels(GrSurface* surface,
+                               int left, int top, int width, int height,
+                               GrPixelConfig config, GrTransferBuffer* buffer,
+                               size_t offset, size_t rowBytes) {
+    SkASSERT(!buffer->isMapped());
+    GrGLTransferBuffer* glBuffer = reinterpret_cast<GrGLTransferBuffer*>(buffer);
+    // bind the transfer buffer
+    SkASSERT(GR_GL_PIXEL_UNPACK_BUFFER == glBuffer->bufferType() ||
+                GR_GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM == glBuffer->bufferType());

[bsalomon, 2015/12/18 15:24:38]

align?

[jvanverth1, 2016/01/04 21:24:09]

Done.

+    GL_CALL(BindBuffer(glBuffer->bufferType(), glBuffer->bufferID()));
+
+    bool result = onWritePixels(surface, left, top, width, height, config, 

[bsalomon, 2015/12/18 15:24:38]

this->

[jvanverth1, 2016/01/04 21:24:09]

Done.

+                                (void*) offset, rowBytes);
+
+    // unbind
+    GL_CALL(BindBuffer(glBuffer->bufferType(), 0));

[bsalomon, 2015/12/18 15:24:38]

should we lazily unbind when we see a call to writePixels?

[jvanverth1, 2016/01/04 21:24:09]

I'm not sure we need an unbind -- removed.

+
+    return result;
+}
+
 static inline GrGLenum check_alloc_error(const GrSurfaceDesc& desc,
                                          const GrGLInterface* interface) {
     if (SkToBool(desc.fFlags & kCheckAllocation_GrSurfaceFlag)) {
@@ -664,7 +684,7 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
                             GrPixelConfig dataConfig,
                             const void* data,
                             size_t rowBytes) {
-    SkASSERT(data || isNewTexture);
+//*** for now    SkASSERT(data || isNewTexture);

[bsalomon, 2015/12/18 15:24:38]

b/c data might be an offset? Maybe we should rename the param dataOrOffset?

Or is there a slightly bigger rewrite we should do that passes the
transferBuffer or nullptr here (and does the bind here)? Not sure whether it's
better or not, just tossing out stuff.

[jvanverth1, 2016/01/04 21:24:09]

Rewrote this to take an upload type -- some of the things we do for regular
uploads aren't possible for transfers (e.g. handling bottom-origin textures by
swapping rows).

 
     // If we're uploading compressed data then we should be using uploadCompressedTexData
     SkASSERT(!GrPixelConfigIsCompressed(dataConfig));
@@ -839,7 +859,7 @@ bool GrGLGpu::uploadCompressedTexData(const GrSurfaceDesc& desc,
                                       const void* data,
                                       bool isNewTexture,
                                       int left, int top, int width, int height) {
-    SkASSERT(data || isNewTexture);
+//*** for now    SkASSERT(data || isNewTexture);
 
     // No support for software flip y, yet...
     SkASSERT(kBottomLeft_GrSurfaceOrigin != desc.fOrigin);
@@ -1499,16 +1519,17 @@ GrTransferBuffer* GrGLGpu::onCreateTransferBuffer(size_t size, TransferType xfer
     if (desc.fID) {
         CLEAR_ERROR_BEFORE_ALLOC(this->glInterface());
         // make sure driver can allocate memory for this bmapuffer
-        GrGLenum type;
+        GrGLenum target;
         if (GrGLCaps::kChromium_TransferBufferType == xferBufferType) {
-            type = toGpu ? GR_GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM
+            target = toGpu ? GR_GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM
                          : GR_GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM;
         } else {
             SkASSERT(GrGLCaps::kPBO_TransferBufferType == xferBufferType);
-            type = toGpu ? GR_GL_PIXEL_UNPACK_BUFFER : GR_GL_PIXEL_PACK_BUFFER;
+            target = toGpu ? GR_GL_PIXEL_UNPACK_BUFFER : GR_GL_PIXEL_PACK_BUFFER;
         }
-        GL_ALLOC_CALL(this->glInterface(),
-                      BufferData(type,
+        GL_CALL(BindBuffer(target, desc.fID));
+        GL_ALLOC_CALL(this->glInterface(), 
+                      BufferData(target,
                                  (GrGLsizeiptr) desc.fSizeInBytes,
                                  nullptr,  // data ptr
                                  (toGpu ? GR_GL_STREAM_DRAW : GR_GL_STREAM_READ)));
@@ -1516,7 +1537,7 @@ GrTransferBuffer* GrGLGpu::onCreateTransferBuffer(size_t size, TransferType xfer
             GL_CALL(DeleteBuffers(1, &desc.fID));
             return nullptr;
         }
-        GrTransferBuffer* transferBuffer = new GrGLTransferBuffer(this, desc, type);
+        GrTransferBuffer* transferBuffer = new GrGLTransferBuffer(this, desc, target);
         return transferBuffer;
     }
 
@@ -1716,11 +1737,14 @@ void* GrGLGpu::mapBuffer(GrGLuint id, GrGLenum type, GrGLBufferImpl::Usage usage
             if (currentSize != requestedSize) {
                 GL_CALL(BufferData(type, requestedSize, nullptr, glUsage));
             }
-            static const GrGLbitfield kWriteAccess = GR_GL_MAP_INVALIDATE_BUFFER_BIT |
-                                                     GR_GL_MAP_WRITE_BIT;
+            GrGLbitfield writeAccess = GR_GL_MAP_WRITE_BIT;
+            // TODO: allow the client to specify invalidation in the stream draw case
+            if (GrGLBufferImpl::kStreamDraw_Usage != usage) {
+                writeAccess |= GR_GL_MAP_INVALIDATE_BUFFER_BIT;
+            }
             GL_CALL_RET(mapPtr, MapBufferRange(type, 0, requestedSize, readOnly ? 
                                                                        GR_GL_MAP_READ_BIT :                                                       
-                                                                       kWriteAccess));
+                                                                       writeAccess));
             break;
         }
         case GrGLCaps::kChromium_MapBufferType: