When we're not using the NULL buffer data hint update with glBufferData rather than glBufferSubData.
Review URL: http://codereview.appspot.com/5253047/
git-svn-id: http://skia.googlecode.com/svn/trunk@2443 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gpu/include/GrConfig.h b/gpu/include/GrConfig.h
index 982d63e..1d07af1 100644
--- a/gpu/include/GrConfig.h
+++ b/gpu/include/GrConfig.h
@@ -356,9 +356,9 @@
/**
* GR_GEOM_BUFFER_LOCK_THRESHOLD gives a threshold (in bytes) for when Gr should
- * lock a GrGeometryBuffer to update its contents. It will use Lock() if the
- * size of the udpated region is greater than the threshold. Otherwise it will
- * use updateData() or updateSubData().
+ * lock a GrGeometryBuffer to update its contents. It will use lock() if the
+ * size of the updated region is greater than the threshold. Otherwise it will
+ * use updateData().
*/
#if !defined(GR_GEOM_BUFFER_LOCK_THRESHOLD)
#define GR_GEOM_BUFFER_LOCK_THRESHOLD (1 << 15)
diff --git a/gpu/include/GrGLConfig.h b/gpu/include/GrGLConfig.h
index b56f2ca..44a3b86 100644
--- a/gpu/include/GrGLConfig.h
+++ b/gpu/include/GrGLConfig.h
@@ -65,9 +65,9 @@
* glBufferData(GL_..._BUFFER, size, NULL, usage); //<--hint, NULL means
* glBufferSubData(GL_..._BUFFER, 0, lessThanSize, data) // old data can't be
* // used again.
- * However, this can cause a performance decrease on Chrome cmd buffer because
- * it will create a new allocation and memset the whole thing to zero (for
- * security reasons). Defaults to 1 (enabled).
+ * However, this can be an unoptimization on some platforms, esp. Chrome.
+ * Chrome's cmd buffer will create a new allocation and memset the whole thing
+ * to zero (for security reasons). Defaults to 1 (enabled).
*
* GR_GL_PER_GL_FUNC_CALLBACK: When set to 1 the GrGLInterface object provides
* a function pointer that is called just before every gl function. The ptr must
diff --git a/gpu/include/GrUserConfig.h b/gpu/include/GrUserConfig.h
index 09eb9f4..9f61638 100644
--- a/gpu/include/GrUserConfig.h
+++ b/gpu/include/GrUserConfig.h
@@ -58,8 +58,8 @@
/*
* This gives a threshold in bytes of when to lock a GrGeometryBuffer vs using
- * updateData or updateSubData. (Note the depending on the underlying 3D API
- * the update functions may always be implemented using a lock)
+ * updateData. (Note the depending on the underlying 3D API the update functions
+ * may always be implemented using a lock)
*/
//#define GR_GEOM_BUFFER_LOCK_THRESHOLD (1<<15)
diff --git a/gpu/src/GrBufferAllocPool.cpp b/gpu/src/GrBufferAllocPool.cpp
index 715d4a9..c01192d 100644
--- a/gpu/src/GrBufferAllocPool.cpp
+++ b/gpu/src/GrBufferAllocPool.cpp
@@ -169,10 +169,13 @@
}
}
- // We could honor the space request using updateSubData on the current VB
- // (if there is room). But we don't currently use draw calls to GL that
+ // We could honor the space request using by a partial update of the current
+ // VB (if there is room). But we don't currently use draw calls to GL that
// allow the driver to know that previously issued draws won't read from
- // the part of the buffer we update.
+ // the part of the buffer we update. Also, the GL buffer implementation
+ // may be cheating on the actual buffer size by shrinking the buffer on
+ // updateData() if the amount of data passed is less than the full buffer
+ // size.
if (!createBlock(size)) {
return NULL;
diff --git a/gpu/src/GrGLIndexBuffer.cpp b/gpu/src/GrGLIndexBuffer.cpp
index 084a4c6..b64668e 100644
--- a/gpu/src/GrGLIndexBuffer.cpp
+++ b/gpu/src/GrGLIndexBuffer.cpp
@@ -104,31 +104,28 @@
}
this->bind();
GrGLenum usage = dynamic() ? GR_GL_DYNAMIC_DRAW : GR_GL_STATIC_DRAW;
+#if !GR_GL_USE_BUFFER_DATA_NULL_HINT
+ // Note that we're cheating on the size here. Currently no methods
+ // allow a partial update that preserves contents of non-updated
+ // portions of the buffer (and lock() does a glBufferData(..size, NULL..))
+ GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER, srcSizeInBytes, src, usage));
+#else
if (this->sizeInBytes() == srcSizeInBytes) {
GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER,
srcSizeInBytes, src, usage));
} else {
-#if GR_GL_USE_BUFFER_DATA_NULL_HINT
+ // Before we call glBufferSubData we give the driver a hint using
+ // glBufferData with NULL. This makes the old buffer contents
+ // inaccessible to future draws. The GPU may still be processing draws
+ // that reference the old contents. With this hint it can assign a
+ // different allocation for the new contents to avoid flushing the gpu
+ // past draws consuming the old contents.
GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER,
this->sizeInBytes(), NULL, usage));
-#endif
GL_CALL(BufferSubData(GR_GL_ELEMENT_ARRAY_BUFFER,
0, srcSizeInBytes, src));
}
- return true;
-}
-
-bool GrGLIndexBuffer::updateSubData(const void* src,
- size_t srcSizeInBytes,
- size_t offset) {
- GrAssert(fBufferID);
- GrAssert(!isLocked());
- if (srcSizeInBytes + offset > this->sizeInBytes()) {
- return false;
- }
- this->bind();
- GL_CALL(BufferSubData(GR_GL_ELEMENT_ARRAY_BUFFER,
- offset, srcSizeInBytes, src));
+#endif
return true;
}
diff --git a/gpu/src/GrGLIndexBuffer.h b/gpu/src/GrGLIndexBuffer.h
index e4e77c9..c3e2287 100644
--- a/gpu/src/GrGLIndexBuffer.h
+++ b/gpu/src/GrGLIndexBuffer.h
@@ -30,9 +30,7 @@
virtual void unlock();
virtual bool isLocked() const;
virtual bool updateData(const void* src, size_t srcSizeInBytes);
- virtual bool updateSubData(const void* src,
- size_t srcSizeInBytes,
- size_t offset);
+
protected:
GrGLIndexBuffer(GrGpuGL* gpu,
GrGLuint id,
diff --git a/gpu/src/GrGLVertexBuffer.cpp b/gpu/src/GrGLVertexBuffer.cpp
index c542602..33c1e7e 100644
--- a/gpu/src/GrGLVertexBuffer.cpp
+++ b/gpu/src/GrGLVertexBuffer.cpp
@@ -101,28 +101,26 @@
}
this->bind();
GrGLenum usage = dynamic() ? GR_GL_DYNAMIC_DRAW : GR_GL_STATIC_DRAW;
+#if !GR_GL_USE_BUFFER_DATA_NULL_HINT
+ // Note that we're cheating on the size here. Currently no methods
+ // allow a partial update that preserves contents of non-updated
+ // portions of the buffer (and lock() does a glBufferData(..size, NULL..))
+ GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
+#else
if (this->sizeInBytes() == srcSizeInBytes) {
GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
} else {
-#if GR_GL_USE_BUFFER_DATA_NULL_HINT
+ // Before we call glBufferSubData we give the driver a hint using
+ // glBufferData with NULL. This makes the old buffer contents
+ // inaccessible to future draws. The GPU may still be processing draws
+ // that reference the old contents. With this hint it can assign a
+ // different allocation for the new contents to avoid flushing the gpu
+ // past draws consuming the old contents.
GL_CALL(BufferData(GR_GL_ARRAY_BUFFER,
this->sizeInBytes(), NULL, usage));
-#endif
GL_CALL(BufferSubData(GR_GL_ARRAY_BUFFER, 0, srcSizeInBytes, src));
}
- return true;
-}
-
-bool GrGLVertexBuffer::updateSubData(const void* src,
- size_t srcSizeInBytes,
- size_t offset) {
- GrAssert(fBufferID);
- GrAssert(!isLocked());
- if (srcSizeInBytes + offset > this->sizeInBytes()) {
- return false;
- }
- this->bind();
- GL_CALL(BufferSubData(GR_GL_ARRAY_BUFFER, offset, srcSizeInBytes, src));
+#endif
return true;
}
diff --git a/gpu/src/GrGLVertexBuffer.h b/gpu/src/GrGLVertexBuffer.h
index f83019d..15fc54a 100644
--- a/gpu/src/GrGLVertexBuffer.h
+++ b/gpu/src/GrGLVertexBuffer.h
@@ -26,9 +26,6 @@
virtual void unlock();
virtual bool isLocked() const;
virtual bool updateData(const void* src, size_t srcSizeInBytes);
- virtual bool updateSubData(const void* src,
- size_t srcSizeInBytes,
- size_t offset);
GrGLuint bufferID() const;
protected:
diff --git a/gpu/src/GrGeometryBuffer.h b/gpu/src/GrGeometryBuffer.h
index a977cda..c74b254 100644
--- a/gpu/src/GrGeometryBuffer.h
+++ b/gpu/src/GrGeometryBuffer.h
@@ -71,17 +71,6 @@
*/
virtual bool updateData(const void* src, size_t srcSizeInBytes) = 0;
- /**
- * Updates a portion of the buffer data.
- *
- * The contents of the buffer outside the update region are preserved.
- *
- * @return returns true if the update succeeds, false otherwise.
- */
- virtual bool updateSubData(const void* src,
- size_t srcSizeInBytes,
- size_t offset) = 0;
-
// GrResource overrides
virtual size_t sizeInBytes() const { return fSizeInBytes; }