Validate bound buffer for draw calls

third_party/WebKit/Source/modules/webgl/WebGLVertexArrayObjectBase.cpp

Index: third_party/WebKit/Source/modules/webgl/WebGLVertexArrayObjectBase.cpp
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLVertexArrayObjectBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGLVertexArrayObjectBase.cpp
index 842546731cd6b336cd710b789745e3147d3fe49c..0b5ec68bc6ebe5baaa75cf6cd51ba260377bf36d 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLVertexArrayObjectBase.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLVertexArrayObjectBase.cpp
@@ -18,6 +18,10 @@ WebGLVertexArrayObjectBase::WebGLVertexArrayObjectBase(WebGLRenderingContextBase
     , m_boundElementArrayBuffer(nullptr)
 {
     m_arrayBufferList.resize(ctx->maxVertexAttribs());
+    m_attribEnabled.resize(ctx->maxVertexAttribs());
+    for (size_t i = 0; i < m_attribEnabled.size(); ++i) {
+        m_attribEnabled[i] = false;
+    }
 
     switch (m_type) {
     case VaoTypeDefault:
@@ -80,7 +84,7 @@ void WebGLVertexArrayObjectBase::setElementArrayBuffer(WebGLBuffer* buffer)
 
 WebGLBuffer* WebGLVertexArrayObjectBase::getArrayBufferForAttrib(size_t index)
 {
-    ASSERT(index < context()->maxVertexAttribs());
+    DCHECK(index < context()->maxVertexAttribs());
     return m_arrayBufferList[index].get();
 }
 
@@ -94,6 +98,27 @@ void WebGLVertexArrayObjectBase::setArrayBufferForAttrib(GLuint index, WebGLBuff
     m_arrayBufferList[index] = buffer;
 }
 
+void WebGLVertexArrayObjectBase::setAttribEnabled(GLuint index, bool enabled)
+{
+    DCHECK(index < context()->maxVertexAttribs());
+    m_attribEnabled[index] = enabled;
+}
+
+bool WebGLVertexArrayObjectBase::getAttribEnabled(GLuint index)
+{
+    DCHECK(index < context()->maxVertexAttribs());
+    return m_attribEnabled[index];
+}
+
+bool WebGLVertexArrayObjectBase::isAllEnabledAttribBufferBound()

[Ken Russell (switch to Gerrit), 2016/06/01 20:55:24]

This function is potentially very hot -- it will be called during each draw
call. Have you benchmarked this change with something like the Animometer 3D
benchmark:
http://trac.webkit.org/export/HEAD/trunk/PerformanceTests/Animometer/develope...
?

I think it is worth optimizing this now, rather than waiting to see if there are
performance bugs with it. There can be a cached value for the result of this
call. The cache needs to be recomputed if enableVertexAttribArray(i) is called
and m_arrayBufferList[i] is null, or if unbindBuffer gets called. (It's not
legal to call vertexAttribPointer in WebGL unless there is a bound ARRAY_BUFFER,
so the only reason the state would be reset to this situation is that a buffer
is deleted, I think.) 

Could you please double-check my thinking here, implement this and benchmark it?
Thanks.

[qiankun, 2016/06/06 11:28:36]

Thanks for your suggestion. I updated this CL to support cache. PTAL.
I also tested Animometer-WebGL tests. By targeting 60FPS and keeping other
options unchanged, I got following socres:
no change: 11578.37
change without cache: 12602.25
change with cache: 12344.05

Suppose the score should be the higher the better. So I think this CL doesn't
take effect on performance.

+{
+    for (size_t i = 0; i < m_attribEnabled.size(); ++i) {
+        if (m_attribEnabled[i] && !m_arrayBufferList[i])
+            return false;
+    }
+    return true;
+}
+
 void WebGLVertexArrayObjectBase::unbindBuffer(WebGLBuffer* buffer)
 {
     if (m_boundElementArrayBuffer == buffer) {