[WebGL 2] primitive restart should be always enabled

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 16 matching lines @@
 
 BufferManager::BufferManager(MemoryTracker* memory_tracker,
                              FeatureInfo* feature_info)
     : memory_type_tracker_(
           new MemoryTypeTracker(memory_tracker)),
       memory_tracker_(memory_tracker),
       feature_info_(feature_info),
       allow_buffers_on_multiple_targets_(false),
       allow_fixed_attribs_(false),
       buffer_count_(0),
+      primitive_restart_enabled_(false),
       have_context_(true),
       use_client_side_arrays_for_stream_buffers_(
           feature_info
               ? feature_info->workarounds()
                     .use_client_side_arrays_for_stream_buffers
               : 0) {
   // When created from InProcessCommandBuffer, we won't have a |memory_tracker_|
   // so don't register a dump provider.
   if (memory_tracker_) {
     base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
@@ skipping 145 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) {
+  bool index_buffer =
+      this->initial_target() == GL_ELEMENT_ARRAY_BUFFER ? true : false;
+  GLuint primitive_restart_index = 0;
+  if (index_buffer && manager_->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);

[Ken Russell (switch to Gerrit), 2016/03/16 16:18:07]

The suggestion is to add the "primitive restart enabled" bit to the Range class,
and use that as part of the key during lookup in the RangeToMaxValueMap. Then
the map can cache the results for the same range both for having primitive
restart enabled as well as disabled. The lookup code can optimistically try to
populate the version with primitive restart enabled from the version with
primitive restart disabled, if the value isn't the primitive restart index.
Otherwise it needs to compute it from scratch.

[yunchao, 2016/03/17 00:19:02]

Ken, I tried this method at first. But the first element in std::map should be
constant. Range is the first element in RangeToMaxValueMap. So I follow the idea
you suggested and get this one: use primitive_restart_flipped (I also tried
primitive_restart_enabled, it is OK too), and use a struct to cache the status
together with max_value in the second element in RangeToMaxValue. It has the
same optimization as bajones@ suggested, it can reuse the max_value when it
flips from primitive restart disabled to primitive restart enabled if possible.
Moreover, if it flips with a even number, say 2, 4 and so forth. The untouched
ranges those are not called by GexMaxValueForRange() during flipping will keep
the same. they don't need to recalculate from scratch!

Could you have a look of this one, I think it is correct, and it may be better.

[Zhenyao Mo, 2016/03/18 20:28:44]

Yunchao, I think you misunderstood.  The suggestion is to make the key range +
primitive_restart_enabled, and the value is max_value.

So for each range, you might have two entries, (range, false) -> max_value, and
(range, true) -> max_value.

   RangeToMaxValueMap::iterator it = range_set_.find(range);
   if (it != range_set_.end()) {
-    *max_value = it->second;
-    return true;
+    if (!index_buffer || !it->second.primitive_restart_flipped_ ||
+        (manager_->primitive_restart_enabled_ &&
+        it->second.max_value_ < primitive_restart_index)) {
+      *max_value = it->second.max_value_;
+      it->second.primitive_restart_flipped_ = false;
+      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));
+  if (it != range_set_.end()) {
+    it->second.max_value_ = max_v;
+    it->second.primitive_restart_flipped_ = false;
+  } else {
+    struct MaxValue value(max_v);
+    range_set_.insert(std::make_pair(range, value));
+  }
   *max_value = max_v;
   return true;
 }
 
 bool BufferManager::GetClientId(GLuint service_id, GLuint* client_id) const {
   // This doesn't need to be fast. It's only used during slow queries.
   for (BufferMap::const_iterator it = buffers_.begin();
        it != buffers_.end(); ++it) {
     if (it->second->service_id() == service_id) {
       *client_id = it->first;
@@ skipping 252 matching lines @@
     case GL_TRANSFORM_FEEDBACK_BUFFER:
       return state->bound_transform_feedback_buffer.get();
     case GL_UNIFORM_BUFFER:
       return state->bound_uniform_buffer.get();
     default:
       NOTREACHED();
       return nullptr;
   }
 }
 
+void BufferManager::SetPrimitiveRestartState(bool enabled) {
+  if (primitive_restart_enabled_ != enabled) {
+    primitive_restart_enabled_ = enabled;
+
+    for (BufferMap::const_iterator it = buffers_.begin();
+        it != buffers_.end(); ++it) {
+      if (it->second->initial_target() == GL_ELEMENT_ARRAY_BUFFER) {
+        Buffer::RangeToMaxValueMap::iterator range_set_it;
+        for (range_set_it = it->second->range_set_.begin();
+            range_set_it != it->second->range_set_.end(); ++range_set_it) {
+          range_set_it->second.primitive_restart_flipped_ =
+              !range_set_it->second.primitive_restart_flipped_;
+        }
+      }
+    }
+  }
+}
+
 bool BufferManager::OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
                                  base::trace_event::ProcessMemoryDump* pmd) {
   const int client_id = memory_tracker_->ClientId();
   for (const auto& buffer_entry : buffers_) {
     const auto& client_buffer_id = buffer_entry.first;
     const auto& buffer = buffer_entry.second;
 
     std::string dump_name = base::StringPrintf(
         "gpu/gl/buffers/client_%d/buffer_%d", client_id, client_buffer_id);
     base::trace_event::MemoryAllocatorDump* dump =
         pmd->CreateAllocatorDump(dump_name);
     dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
                     base::trace_event::MemoryAllocatorDump::kUnitsBytes,
                     static_cast<uint64_t>(buffer->size()));
 
     auto guid = gfx::GetGLBufferGUIDForTracing(
         memory_tracker_->ShareGroupTracingGUID(), client_buffer_id);
     pmd->CreateSharedGlobalAllocatorDump(guid);
     pmd->AddOwnershipEdge(dump->guid(), guid);
   }
   return true;
 }
 
 }  // namespace gles2
 }  // namespace gpu