Better workaround for osx / nv bug

Review URL: http://codereview.appspot.com/5727044/

git-svn-id: http://skia.googlecode.com/svn/trunk@3308 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/include/gpu/gl/GrGLConfig.h b/include/gpu/gl/GrGLConfig.h
index 2cc4f31..806f055 100644 (file)
--- a/include/gpu/gl/GrGLConfig.h
+++ b/include/gpu/gl/GrGLConfig.h
@@ -152,12 +152,11 @@
 /**
  * There is a strange bug that occurs on Macs with NVIDIA GPUs. We don't
  * fully understand it. When (element) array buffers are continually
- * respecified using glBufferData at the same (or perhaps just a small) size
- * performance can fall off of a cliff. The driver winds up performing many
- * DMA mapping / unmappings and chews up ~50% of the core. However, it has been
- * observed that respecifiying the buffer with a larger size and then writing
- * the small amount of actual data using glBufferSubData prevents the bad
- * behavior.
+ * respecified using glBufferData performance can fall off of a cliff. The
+ * driver winds up performing many DMA mapping / unmappings and chews up ~50% of
+ * the core. However, it has been observed that occaisonally respecifiying the
+ * buffer using glBufferData and then writing data using glBufferSubData
+ * prevents the bad behavior.
  *
  * There is a lot of uncertainty around this issue. In Chrome backgrounding
  * the tab somehow initiates this behavior and we don't know what the connection
@@ -169,11 +168,8 @@
  * The issue is tracked at:
  * http://code.google.com/p/chromium/issues/detail?id=114865
  *
- * When GR_GL_USE_BUFFER_DATA_NULL_HINT is not set we encounter the bug
- * if we keep uploading small vetex / index sizes. So, every 1K uploads we
- * act as though GR_GL_USE_BUFFER_DATA_NULL_HINT was set: we specify the buffer
- * at full size with a NULL data ptr and then do a partial update with
- * glBufferSubData.
+ * When the workaround is enabled we will use the glBufferData / glBufferSubData
+ * trick every 128 array buffer uploads.
  *
  * Hopefully we will understand this better and have a cleaner fix or get a
  * OS/driver level fix.

diff --git a/src/gpu/gl/GrGLIndexBuffer.cpp b/src/gpu/gl/GrGLIndexBuffer.cpp
index f424e78..60bd9a9 100644 (file)
--- a/src/gpu/gl/GrGLIndexBuffer.cpp
+++ b/src/gpu/gl/GrGLIndexBuffer.cpp
@@ -105,37 +105,29 @@ bool GrGLIndexBuffer::updateData(const void* src, size_t srcSizeInBytes) {
     this->bind();
     GrGLenum usage = dynamic() ? GR_GL_DYNAMIC_DRAW : GR_GL_STATIC_DRAW;
 
-    bool doNullHint = GR_GL_USE_BUFFER_DATA_NULL_HINT;
-#if GR_GL_MAC_BUFFER_OBJECT_PERFOMANCE_WORKAROUND
-    GrAssert(!doNullHint);
-    static int N = 0;
-    doNullHint = (0 == N % 1024);
-    ++N;
-#endif
-
-    if (doNullHint) {
-        if (this->sizeInBytes() == srcSizeInBytes) {
-            GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER,
-                                srcSizeInBytes, src, usage));
-        } else {
-            // 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));
-            GL_CALL(BufferSubData(GR_GL_ELEMENT_ARRAY_BUFFER,
-                                  0, srcSizeInBytes, src));
-        }
+#if GR_GL_USE_BUFFER_DATA_NULL_HINT
+    if (this->sizeInBytes() == srcSizeInBytes) {
+        GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER,
+                            srcSizeInBytes, src, usage));
     } else {
-        // 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 (lock() does a glBufferData(..size, NULL..))
-        GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER, 
-                           srcSizeInBytes, src, usage));
+        // 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));
+        GL_CALL(BufferSubData(GR_GL_ELEMENT_ARRAY_BUFFER,
+                              0, srcSizeInBytes, src));
     }
+#else
+    // 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 (lock() does a glBufferData(..size, NULL..))
+    GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER, 
+                       srcSizeInBytes, src, usage));
+#endif
     return true;
 }
 

diff --git a/src/gpu/gl/GrGLVertexBuffer.cpp b/src/gpu/gl/GrGLVertexBuffer.cpp
index 4928868..126a95f 100644 (file)
--- a/src/gpu/gl/GrGLVertexBuffer.cpp
+++ b/src/gpu/gl/GrGLVertexBuffer.cpp
@@ -103,32 +103,44 @@ bool GrGLVertexBuffer::updateData(const void* src, size_t srcSizeInBytes) {
     GrGLenum usage = dynamic() ? GR_GL_DYNAMIC_DRAW : GR_GL_STATIC_DRAW;
 
     bool doNullHint = GR_GL_USE_BUFFER_DATA_NULL_HINT;
+#if GR_GL_USE_BUFFER_DATA_NULL_HINT
+    if (this->sizeInBytes() == srcSizeInBytes) {
+        GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
+    } else {
+        // 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));
+        GL_CALL(BufferSubData(GR_GL_ARRAY_BUFFER, 0, srcSizeInBytes, src));
+    }
+#else
+    // 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 (lock() does a glBufferData(..size, NULL..))
+    bool doSubData = false;
 #if GR_GL_MAC_BUFFER_OBJECT_PERFOMANCE_WORKAROUND
-    GrAssert(!doNullHint);
     static int N = 0;
-    doNullHint = (0 == N % 1024);
+    // 128 was chosen experimentally. At 256 a slight hitchiness was noticed
+    // when dragging a Chromium window around with a canvas tab backgrounded.
+    doSubData = 0 == (N % 128);
     ++N;
 #endif
-    if (doNullHint) {
-        if (this->sizeInBytes() == srcSizeInBytes) {
-            GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
-        } else {
-            // 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));
-            GL_CALL(BufferSubData(GR_GL_ARRAY_BUFFER, 0, srcSizeInBytes, src));
-        }
+    if (doSubData) {
+        // The workaround is to do a glBufferData followed by glBufferSubData.
+        // Chromium's command buffer may turn a glBufferSubData where the size
+        // exactly matches the buffer size into a glBufferData. So we tack 1
+        // extra byte onto the glBufferData.
+        GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes + 1,
+                           NULL, usage));
+        GL_CALL(BufferSubData(GR_GL_ARRAY_BUFFER, 0, srcSizeInBytes, src));
     } else {
-        // 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 (lock() does a glBufferData(..size, NULL..))
         GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
     }
+#endif
     return true;
 }