Fix PRIMITIVE_RESTART_FIXED_INDEX handling in command buffer and WebGL 2.0.

gpu/command_buffer/service/buffer_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/buffer_manager.h"
 
 #include <stdint.h>
 
 #include <limits>
 
@@ skipping 181 matching lines @@
     return NULL;
   }
   return shadow_.get() + offset;
 }
 
 void Buffer::ClearCache() {
   range_set_.clear();
 }
 
 template <typename T>
-GLuint GetMaxValue(const void* data, GLuint offset, GLsizei count) {
+GLuint GetMaxValue(const void* data, GLuint offset, GLsizei count,
+    GLuint primitive_restart_index) {
   GLuint max_value = 0;
   const T* element =
       reinterpret_cast<const T*>(static_cast<const int8_t*>(data) + offset);
   const T* end = element + count;
   for (; element < end; ++element) {
     if (*element > max_value) {
+      if (*element == primitive_restart_index) {
+        continue;
+      }
       max_value = *element;
     }
   }
   return max_value;
 }
 
 bool Buffer::GetMaxValueForRange(
-    GLuint offset, GLsizei count, GLenum type, GLuint* max_value) {
-  Range range(offset, count, type);
+    GLuint offset, GLsizei count, GLenum type, bool primitive_restart_enabled,
+    GLuint* max_value) {
+  bool index_buffer =
+      this->initial_target() == GL_ELEMENT_ARRAY_BUFFER ? true : false;

[piman, 2016/03/19 00:56:11]

nit: "? true : false" is redundant

[piman, 2016/03/19 00:56:11]

What is this check for? This is only used for things that will be used as
indices, it doesn't really matter which target this was originally created for -
we still would want to avoid false-negatives if primitive restart is enabled.

[Ken Russell (switch to Gerrit), 2016/03/19 04:06:52]

Good point -- I copied this code from the original CL without thinking it
through. It seems unnecessary. Removed and re-tested; all tests in
deqp/functional/gles3/primitiverestart.html still pass.

[yunchao, 2016/03/21 15:11:35]

I am not familiar with GetMaxValueInBufferCHROMIUM. 
We expose GetMaxValueInBufferCHROMIUM in command buffer, which calls into
GetMaxValueForRange. Does this API only access to index buffer? Is it valid to
access to other buffers? Please considering this situation, we have 0xFF in
color buffer, and try to get the max value in that buffer in WebGL 2 (primitive
restart is always enabled in WebGL 2). 

[Ken Russell (switch to Gerrit), 2016/03/21 17:41:46]

GetMaxValueInBufferCHROMIUM is used to emulate client-side vertex arrays. It
returns the largest index accessed by the index buffer for a given draw call so
the client side can upload just those vertices into a buffer object behind the
scenes. It won't affect the fetching of values from non-element-array buffer
objects.

+  GLuint primitive_restart_index = 0;
+  if (index_buffer && primitive_restart_enabled) {
+    switch (type) {
+      case GL_UNSIGNED_BYTE:
+        primitive_restart_index = 0xFF;
+        break;
+      case GL_UNSIGNED_SHORT:
+        primitive_restart_index = 0xFFFF;
+        break;
+      case GL_UNSIGNED_INT:
+        primitive_restart_index = 0xFFFFFFFF;
+        break;
+      default:
+        NOTREACHED();  // should never get here by validation.
+        break;
+    }
+  }
+
+  Range range(offset, count, type, primitive_restart_enabled);
   RangeToMaxValueMap::iterator it = range_set_.find(range);
   if (it != range_set_.end()) {
     *max_value = it->second;
     return true;
   }
+  // Optimization. If:
+  //  - primitive restart is enabled
+  //  - we don't have an entry in the range set for these parameters
+  //    for the situation when primitive restart is enabled
+  //  - we do have an entry in the range set for these parameters for
+  //    the situation when primitive restart is disabled
+  //  - this entry is less than the primitive restart index
+  // Then we can repurpose this entry for the situation when primitive
+  // restart is enabled. Otherwise, we need to compute the max index
+  // from scratch.
+  if (primitive_restart_enabled) {
+    Range disabled_range(offset, count, type, false);
+    RangeToMaxValueMap::iterator it = range_set_.find(disabled_range);
+    if (it != range_set_.end() && it->second < primitive_restart_index) {
+      // This reuses the max value for the case where primitive
+      // restart is enabled.
+      range_set_.insert(std::make_pair(range, it->second));
+      *max_value = it->second;
+      return true;
+    }
+  }
 
   uint32_t size;
   if (!SafeMultiplyUint32(
       count, GLES2Util::GetGLTypeSizeForBuffers(type), &size)) {
     return false;
   }
 
   if (!SafeAddUint32(offset, size, &size)) {
     return false;
   }
 
   if (size > static_cast<uint32_t>(size_)) {
     return false;
   }
 
   if (!shadowed_) {
     return false;
   }
 
   // Scan the range for the max value and store
   GLuint max_v = 0;
   switch (type) {
     case GL_UNSIGNED_BYTE:
-      max_v = GetMaxValue<uint8_t>(shadow_.get(), offset, count);
+      max_v = GetMaxValue<uint8_t>(shadow_.get(), offset, count,
+          primitive_restart_index);
       break;
     case GL_UNSIGNED_SHORT:
       // Check we are not accessing an odd byte for a 2 byte value.
       if ((offset & 1) != 0) {
         return false;
       }
-      max_v = GetMaxValue<uint16_t>(shadow_.get(), offset, count);
+      max_v = GetMaxValue<uint16_t>(shadow_.get(), offset, count,
+          primitive_restart_index);
       break;
     case GL_UNSIGNED_INT:
       // Check we are not accessing a non aligned address for a 4 byte value.
       if ((offset & 3) != 0) {
         return false;
       }
-      max_v = GetMaxValue<uint32_t>(shadow_.get(), offset, count);
+      max_v = GetMaxValue<uint32_t>(shadow_.get(), offset, count,
+          primitive_restart_index);
       break;
     default:
       NOTREACHED();  // should never get here by validation.
       break;
   }
   range_set_.insert(std::make_pair(range, max_v));
   *max_value = max_v;
   return true;
 }
 
@@ skipping 283 matching lines @@
     auto guid = gfx::GetGLBufferGUIDForTracing(
         memory_tracker_->ShareGroupTracingGUID(), client_buffer_id);
     pmd->CreateSharedGlobalAllocatorDump(guid);
     pmd->AddOwnershipEdge(dump->guid(), guid);
   }
   return true;
 }
 
 }  // namespace gles2
 }  // namespace gpu