Workaround VAO related driver bug.

If a VAO is bound, glVertexAttribPointer will incorrectly generate an
GL_INVALID_OPERATION if no ARRAY_BUFFER is bound and pointer is NULL.

BUG=373974
TEST=gpu_unittests, webgl conformance tests on AMD
R=kbr@chromium.org, piman@chromium.org, bajones@chromium.org, vmiura@chromium.org

Review URL: https://codereview.chromium.org/293033014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273678 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 81 insertions, 20 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 12bbd7c..793606f9 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1463,7 +1463,7 @@ class GLES2DecoderImpl : public GLES2Decoder,
   // simulated.
   bool SimulateAttrib0(
       const char* function_name, GLuint max_vertex_accessed, bool* simulated);
-  void RestoreStateForAttrib(GLuint attrib);
+  void RestoreStateForAttrib(GLuint attrib, bool restore_array_binding);
 
   // If an image is bound to texture, this will call Will/DidUseTexImage
   // if needed.
@@ -6238,21 +6238,23 @@ bool GLES2DecoderImpl::SimulateAttrib0(
   return true;
 }
 
-void GLES2DecoderImpl::RestoreStateForAttrib(GLuint attrib_index) {
+void GLES2DecoderImpl::RestoreStateForAttrib(
+    GLuint attrib_index, bool restore_array_binding) {
   const VertexAttrib* attrib =
       state_.vertex_attrib_manager->GetVertexAttrib(attrib_index);
-  const void* ptr = reinterpret_cast<const void*>(attrib->offset());
-  Buffer* buffer = attrib->buffer();
-  glBindBuffer(GL_ARRAY_BUFFER, buffer ? buffer->service_id() : 0);
-  glVertexAttribPointer(
-      attrib_index, attrib->size(), attrib->type(), attrib->normalized(),
-      attrib->gl_stride(), ptr);
+  if (restore_array_binding) {
+    const void* ptr = reinterpret_cast<const void*>(attrib->offset());
+    Buffer* buffer = attrib->buffer();
+    glBindBuffer(GL_ARRAY_BUFFER, buffer ? buffer->service_id() : 0);
+    glVertexAttribPointer(
+        attrib_index, attrib->size(), attrib->type(), attrib->normalized(),
+        attrib->gl_stride(), ptr);
+  }
   if (attrib->divisor())
     glVertexAttribDivisorANGLE(attrib_index, attrib->divisor());
   glBindBuffer(
-      GL_ARRAY_BUFFER,
-      state_.bound_array_buffer.get() ? state_.bound_array_buffer->service_id()
-                                      : 0);
+      GL_ARRAY_BUFFER, state_.bound_array_buffer.get() ?
+          state_.bound_array_buffer->service_id() : 0);
 
   // Never touch vertex attribute 0's state (in particular, never
   // disable it) when running on desktop GL because it will never be
@@ -6456,7 +6458,11 @@ error::Error GLES2DecoderImpl::DoDrawArrays(
       }
     }
     if (simulated_attrib_0) {
-      RestoreStateForAttrib(0);
+      // We don't have to restore attrib 0 generic data at the end of this
+      // function even if it is simulated. This is because we will simulate
+      // it in each draw call, and attrib 0 generic data queries use cached
+      // values instead of passing down to the underlying driver.
+      RestoreStateForAttrib(0, false);
     }
   }
   return error::kNoError;
@@ -6592,7 +6598,11 @@ error::Error GLES2DecoderImpl::DoDrawElements(
       }
     }
     if (simulated_attrib_0) {
-      RestoreStateForAttrib(0);
+      // We don't have to restore attrib 0 generic data at the end of this
+      // function even if it is simulated. This is because we will simulate
+      // it in each draw call, and attrib 0 generic data queries use cached
+      // values instead of passing down to the underlying driver.
+      RestoreStateForAttrib(0, false);
     }
   }
   return error::kNoError;
@@ -9716,7 +9726,7 @@ void GLES2DecoderImpl::DoBindVertexArrayOES(GLuint client_id) {
 void GLES2DecoderImpl::EmulateVertexArrayState() {
   // Setup the Vertex attribute state
   for (uint32 vv = 0; vv < group_->max_vertex_attribs(); ++vv) {
-    RestoreStateForAttrib(vv);
+    RestoreStateForAttrib(vv, true);
   }
 
   // Setup the element buffer
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
index 6cee965..f2a6888 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
@@ -27,6 +27,8 @@ using ::gfx::MockGLInterface;
 using ::testing::_;
 using ::testing::DoAll;
 using ::testing::InSequence;
+using ::testing::Invoke;
+using ::testing::InvokeWithoutArgs;
 using ::testing::MatcherCast;
 using ::testing::Pointee;
 using ::testing::Return;
@@ -35,6 +37,7 @@ using ::testing::SetArgPointee;
 using ::testing::SetArgumentPointee;
 using ::testing::StrEq;
 using ::testing::StrictMock;
+using ::testing::WithArg;
 
 namespace {
 
@@ -153,6 +156,8 @@ void GLES2DecoderTestBase::InitDecoderWithCommandLine(
   gl_.reset(new StrictMock<MockGLInterface>());
   ::gfx::MockGLInterface::SetGLInterface(gl_.get());
 
+  SetupMockGLBehaviors();
+
   // Only create stream texture manager if extension is requested.
   std::vector<std::string> list;
   base::SplitString(normalized_init.extensions, ' ', &list);
@@ -1533,12 +1538,6 @@ void GLES2DecoderTestBase::AddExpectationsForSimulatedAttrib0WithError(
     EXPECT_CALL(*gl_, VertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, NULL))
         .Times(1)
         .RetiresOnSaturation();
-    EXPECT_CALL(*gl_, BindBuffer(GL_ARRAY_BUFFER, 0))
-        .Times(1)
-        .RetiresOnSaturation();
-    EXPECT_CALL(*gl_, VertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 0, NULL))
-        .Times(1)
-        .RetiresOnSaturation();
     EXPECT_CALL(*gl_, BindBuffer(GL_ARRAY_BUFFER, buffer_id))
         .Times(1)
         .RetiresOnSaturation();
@@ -1551,6 +1550,21 @@ void GLES2DecoderTestBase::AddExpectationsForSimulatedAttrib0(
       num_vertices, buffer_id, GL_NO_ERROR);
 }
 
+void GLES2DecoderTestBase::SetupMockGLBehaviors() {
+  ON_CALL(*gl_, BindVertexArrayOES(_))
+      .WillByDefault(Invoke(
+          &gl_states_,
+          &GLES2DecoderTestBase::MockGLStates::OnBindVertexArrayOES));
+  ON_CALL(*gl_, BindBuffer(GL_ARRAY_BUFFER, _))
+      .WillByDefault(WithArg<1>(Invoke(
+          &gl_states_,
+          &GLES2DecoderTestBase::MockGLStates::OnBindArrayBuffer)));
+  ON_CALL(*gl_, VertexAttribPointer(_, _, _, _, _, NULL))
+      .WillByDefault(InvokeWithoutArgs(
+          &gl_states_,
+          &GLES2DecoderTestBase::MockGLStates::OnVertexAttribNullPointer));
+}
+
 GLES2DecoderWithShaderTestBase::MockCommandBufferEngine::
 MockCommandBufferEngine() {
 
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
index c7b263a..0107b3e 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
@@ -570,10 +570,47 @@ class GLES2DecoderTestBase : public ::testing::TestWithParam<bool> {
     scoped_refptr<gpu::Buffer> invalid_buffer_;
   };
 
+  // MockGLStates is used to track GL states and emulate driver
+  // behaviors on top of MockGLInterface.
+  class MockGLStates {
+   public:
+    MockGLStates()
+        : bound_array_buffer_object_(0),
+          bound_vertex_array_object_(0) {
+    }
+
+    ~MockGLStates() {
+    }
+
+    void OnBindArrayBuffer(GLuint id) {
+      bound_array_buffer_object_ = id;
+    }
+
+    void OnBindVertexArrayOES(GLuint id) {
+      bound_vertex_array_object_ = id;
+    }
+
+    void OnVertexAttribNullPointer() {
+      // When a vertex array object is bound, some drivers (AMD Linux,
+      // Qualcomm, etc.) have a bug where it incorrectly generates an
+      // GL_INVALID_OPERATION on glVertexAttribPointer() if pointer
+      // is NULL, no buffer is bound on GL_ARRAY_BUFFER.
+      // Make sure we don't trigger this bug.
+      if (bound_vertex_array_object_ != 0)
+        EXPECT_TRUE(bound_array_buffer_object_ != 0);
+    }
+
+   private:
+    GLuint bound_array_buffer_object_;
+    GLuint bound_vertex_array_object_;
+  };  // class MockGLStates
+
   void AddExpectationsForVertexAttribManager();
+  void SetupMockGLBehaviors();
 
   scoped_ptr< ::testing::StrictMock<MockCommandBufferEngine> > engine_;
   scoped_refptr<ContextGroup> group_;
+  MockGLStates gl_states_;
 };
 
 class GLES2DecoderWithShaderTestBase : public GLES2DecoderTestBase {