Make vertex attrib divisor affect non-instanced calls as in spec

In the ANGLE_instanced_arrays spec, drawArrays and drawElements are
redefined in terms of drawArraysOneInstance and drawElementsOneInstance.
Fix the validation checks in the command buffer to reflect this.

BUG=398337
TEST=gpu_unittests, WebGL conformance tests

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

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

4 files changed, 131 insertions, 14 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index f3e5779..7e2c083 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1497,7 +1497,8 @@ class GLES2DecoderImpl : public GLES2Decoder,
 
   // Checks if the current program and vertex attributes are valid for drawing.
   bool IsDrawValid(
-      const char* function_name, GLuint max_vertex_accessed, GLsizei primcount);
+      const char* function_name, GLuint max_vertex_accessed, bool instanced,
+      GLsizei primcount);
 
   // Returns true if successful, simulated will be true if attrib0 was
   // simulated.
@@ -1521,7 +1522,7 @@ class GLES2DecoderImpl : public GLES2Decoder,
   void RestoreStateForSimulatedFixedAttribs();
 
   // Handle DrawArrays and DrawElements for both instanced and non-instanced
-  // cases (primcount is 0 for non-instanced).
+  // cases (primcount is always 1 for non-instanced).
   error::Error DoDrawArrays(
       const char* function_name,
       bool instanced, GLenum mode, GLint first, GLsizei count,
@@ -6218,7 +6219,10 @@ bool GLES2DecoderImpl::ClearUnclearedTextures() {
 }
 
 bool GLES2DecoderImpl::IsDrawValid(
-    const char* function_name, GLuint max_vertex_accessed, GLsizei primcount) {
+    const char* function_name, GLuint max_vertex_accessed, bool instanced,
+    GLsizei primcount) {
+  DCHECK(instanced || primcount == 1);
+
   // NOTE: We specifically do not check current_program->IsValid() because
   // it could never be invalid since glUseProgram would have failed. While
   // glLinkProgram could later mark the program as invalid the previous
@@ -6236,6 +6240,7 @@ bool GLES2DecoderImpl::IsDrawValid(
                          feature_info_.get(),
                          state_.current_program.get(),
                          max_vertex_accessed,
+                         instanced,
                          primcount);
 }
 
@@ -6493,13 +6498,13 @@ error::Error GLES2DecoderImpl::DoDrawArrays(
     return error::kNoError;
   }
 
-  if (count == 0 || (instanced && primcount == 0)) {
+  if (count == 0 || primcount == 0) {
     LOCAL_RENDER_WARNING("Render count or primcount is 0.");
     return error::kNoError;
   }
 
   GLuint max_vertex_accessed = first + count - 1;
-  if (IsDrawValid(function_name, max_vertex_accessed, primcount)) {
+  if (IsDrawValid(function_name, max_vertex_accessed, instanced, primcount)) {
     if (!ClearUnclearedTextures()) {
       LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "out of memory");
       return error::kNoError;
@@ -6546,7 +6551,7 @@ error::Error GLES2DecoderImpl::HandleDrawArrays(
                       static_cast<GLenum>(c.mode),
                       static_cast<GLint>(c.first),
                       static_cast<GLsizei>(c.count),
-                      0);
+                      1);
 }
 
 error::Error GLES2DecoderImpl::HandleDrawArraysInstancedANGLE(
@@ -6607,7 +6612,7 @@ error::Error GLES2DecoderImpl::DoDrawElements(
     return error::kNoError;
   }
 
-  if (count == 0 || (instanced && primcount == 0)) {
+  if (count == 0 || primcount == 0) {
     return error::kNoError;
   }
 
@@ -6622,7 +6627,7 @@ error::Error GLES2DecoderImpl::DoDrawElements(
     return error::kNoError;
   }
 
-  if (IsDrawValid(function_name, max_vertex_accessed, primcount)) {
+  if (IsDrawValid(function_name, max_vertex_accessed, instanced, primcount)) {
     if (!ClearUnclearedTextures()) {
       LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "out of memory");
       return error::kNoError;
@@ -6686,7 +6691,7 @@ error::Error GLES2DecoderImpl::HandleDrawElements(
                         static_cast<GLsizei>(c.count),
                         static_cast<GLenum>(c.type),
                         static_cast<int32>(c.index_offset),
-                        0);
+                        1);
 }
 
 error::Error GLES2DecoderImpl::HandleDrawElementsInstancedANGLE(
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_drawing.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_drawing.cc
index f41f2a9..3c5b72a 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_drawing.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_drawing.cc
@@ -1118,6 +1118,30 @@ TEST_P(GLES2DecoderGeometryInstancingTest,
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
 
+// Regular drawArrays takes the divisor into account
+TEST_P(GLES2DecoderGeometryInstancingTest,
+       DrawArraysWithDivisorSucceeds) {
+  SetupTexture();
+  SetupVertexBuffer();
+  SetupExpectationsForApplyingDefaultDirtyState();
+  DoVertexAttribPointer(1, 2, GL_FLOAT, 0, 0);
+
+  DoEnableVertexAttribArray(0);
+  // Access the data right at the end of the buffer.
+  DoVertexAttribPointer(
+      0, 2, GL_FLOAT, 0, (kNumVertices - 1) * 2 * sizeof(GLfloat));
+  DoVertexAttribDivisorANGLE(0, 1);
+  EXPECT_CALL(
+      *gl_,
+      DrawArrays(GL_TRIANGLES, 0, kNumVertices))
+      .Times(1)
+      .RetiresOnSaturation();
+  DrawArrays cmd;
+  cmd.Init(GL_TRIANGLES, 0, kNumVertices);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
 // Per-instance data is twice as large, but divisor is twice
 TEST_P(GLES2DecoderGeometryInstancingTest,
        DrawArraysInstancedANGLELargeDivisorSucceeds) {
@@ -1208,6 +1232,26 @@ TEST_P(GLES2DecoderGeometryInstancingTest,
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
 
+TEST_P(GLES2DecoderGeometryInstancingTest,
+       DrawArraysNoDivisor0Fails) {
+  SetupTexture();
+  SetupVertexBuffer();
+  DoVertexAttribPointer(1, 2, GL_FLOAT, 0, 0);
+
+  DoEnableVertexAttribArray(0);
+  DoVertexAttribPointer(0, 2, GL_FLOAT, 0, 0);
+  DoVertexAttribDivisorANGLE(0, 1);
+  DoVertexAttribDivisorANGLE(1, 1);
+  EXPECT_CALL(*gl_, DrawArrays(_, _, _))
+      .Times(0)
+      .RetiresOnSaturation();
+  DrawArrays cmd;
+  cmd.Init(GL_TRIANGLES, 0, kNumVertices);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
 TEST_P(GLES2DecoderWithShaderTest, DrawElementsNoAttributesSucceeds) {
   SetupTexture();
   SetupIndexBuffer();
@@ -1589,6 +1633,43 @@ TEST_P(GLES2DecoderGeometryInstancingTest,
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
 
+// Regular drawElements takes the divisor into account
+TEST_P(GLES2DecoderGeometryInstancingTest,
+       DrawElementsWithDivisorSucceeds) {
+  SetupTexture();
+  SetupIndexBuffer();
+  SetupVertexBuffer();
+  SetupExpectationsForApplyingDefaultDirtyState();
+  // Add offset so we're sure we're accessing data near the end of the buffer.
+  DoVertexAttribPointer(
+      1,
+      2,
+      GL_FLOAT,
+      0,
+      (kNumVertices - kMaxValidIndex - 1) * 2 * sizeof(GLfloat));
+
+  DoEnableVertexAttribArray(0);
+  // Access the data right at the end of the buffer.
+  DoVertexAttribPointer(
+      0, 2, GL_FLOAT, 0, (kNumVertices - 1) * 2 * sizeof(GLfloat));
+  DoVertexAttribDivisorANGLE(0, 1);
+  EXPECT_CALL(
+      *gl_,
+      DrawElements(GL_TRIANGLES,
+                   kValidIndexRangeCount,
+                   GL_UNSIGNED_SHORT,
+                   BufferOffset(kValidIndexRangeStart * 2)))
+      .Times(1)
+      .RetiresOnSaturation();
+  DrawElements cmd;
+  cmd.Init(GL_TRIANGLES,
+           kValidIndexRangeCount,
+           GL_UNSIGNED_SHORT,
+           kValidIndexRangeStart * 2);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
 // Per-instance data is twice as large, but divisor is twice
 TEST_P(GLES2DecoderGeometryInstancingTest,
        DrawElementsInstancedANGLELargeDivisorSucceeds) {
@@ -1736,6 +1817,30 @@ TEST_P(GLES2DecoderGeometryInstancingTest,
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 }
 
+TEST_P(GLES2DecoderGeometryInstancingTest,
+       DrawElementsNoDivisor0Fails) {
+  SetupTexture();
+  SetupIndexBuffer();
+  SetupVertexBuffer();
+  DoVertexAttribPointer(1, 2, GL_FLOAT, 0, 0);
+
+  DoEnableVertexAttribArray(0);
+  DoVertexAttribPointer(0, 2, GL_FLOAT, 0, 0);
+  DoVertexAttribDivisorANGLE(0, 1);
+  DoVertexAttribDivisorANGLE(1, 1);
+  EXPECT_CALL(*gl_, DrawElements(_, _, _, _))
+      .Times(0)
+      .RetiresOnSaturation();
+  DrawElements cmd;
+  cmd.Init(GL_TRIANGLES,
+           kValidIndexRangeCount,
+           GL_UNSIGNED_SHORT,
+           kValidIndexRangeStart * 2);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
 TEST_P(GLES2DecoderWithShaderTest, DrawArraysClearsAfterTexImage2DNULL) {
   SetupAllNeededVertexBuffers();
   DoBindTexture(GL_TEXTURE_2D, client_texture_id_, kServiceTextureId);
diff --git a/gpu/command_buffer/service/vertex_attrib_manager.cc b/gpu/command_buffer/service/vertex_attrib_manager.cc
index 5bf4040..8725c4f 100644
--- a/gpu/command_buffer/service/vertex_attrib_manager.cc
+++ b/gpu/command_buffer/service/vertex_attrib_manager.cc
@@ -163,10 +163,13 @@ bool VertexAttribManager::ValidateBindings(
     FeatureInfo* feature_info,
     Program* current_program,
     GLuint max_vertex_accessed,
+    bool instanced,
     GLsizei primcount) {
+  DCHECK(primcount);
   ErrorState* error_state = decoder->GetErrorState();
   // true if any enabled, used divisor is zero
   bool divisor0 = false;
+  bool have_enabled_active_attribs = false;
   const GLuint kInitialBufferId = 0xFFFFFFFFU;
   GLuint current_buffer_id = kInitialBufferId;
   bool use_client_side_arrays_for_stream_buffers = feature_info->workarounds(
@@ -182,6 +185,7 @@ bool VertexAttribManager::ValidateBindings(
         current_program->GetAttribInfoByLocation(attrib->index());
     if (attrib_info) {
       divisor0 |= (attrib->divisor() == 0);
+      have_enabled_active_attribs = true;
       GLuint count = attrib->MaxVertexAccessed(primcount, max_vertex_accessed);
       // This attrib is used in the current program.
       if (!attrib->CanAccess(count)) {
@@ -251,11 +255,14 @@ bool VertexAttribManager::ValidateBindings(
     }
   }
 
-  if (primcount && !divisor0) {
+  // Instanced drawing needs at least one enabled attribute with divisor zero.
+  // Non-instanced drawing is fine with having no attributes at all, but if
+  // there are attributes, at least one should have divisor zero.
+  // (See ANGLE_instanced_arrays spec)
+  if (!divisor0 && (instanced || have_enabled_active_attribs)) {
     ERRORSTATE_SET_GL_ERROR(
         error_state, GL_INVALID_OPERATION, function_name,
-        "attempt instanced render with all attributes having "
-        "non-zero divisors");
+        "attempt to draw with all attributes having non-zero divisors");
     return false;
   }
 
diff --git a/gpu/command_buffer/service/vertex_attrib_manager.h b/gpu/command_buffer/service/vertex_attrib_manager.h
index 8ee8587..73fa480 100644
--- a/gpu/command_buffer/service/vertex_attrib_manager.h
+++ b/gpu/command_buffer/service/vertex_attrib_manager.h
@@ -72,8 +72,7 @@ class GPU_EXPORT VertexAttrib {
   // Find the maximum vertex accessed, accounting for instancing.
   GLuint MaxVertexAccessed(GLsizei primcount,
                            GLuint max_vertex_accessed) const {
-    return (primcount && divisor_) ? ((primcount - 1) / divisor_) :
-                                     max_vertex_accessed;
+    return divisor_ ? ((primcount - 1) / divisor_) : max_vertex_accessed;
   }
 
   bool is_client_side_array() const {
@@ -247,6 +246,7 @@ class GPU_EXPORT VertexAttribManager :
       FeatureInfo* feature_info,
       Program* current_program,
       GLuint max_vertex_accessed,
+      bool instanced,
       GLsizei primcount);
 
  private: