Allow one-copy task tile worker pool to use compressed textures.

cc/raster/one_copy_tile_task_worker_pool.cc

 // Copyright 2014 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 "cc/raster/one_copy_tile_task_worker_pool.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/strings/stringprintf.h"
@@ skipping 163 matching lines @@
 
 // static
 scoped_ptr<TileTaskWorkerPool> OneCopyTileTaskWorkerPool::Create(
     base::SequencedTaskRunner* task_runner,
     TaskGraphRunner* task_graph_runner,
     ContextProvider* context_provider,
     ResourceProvider* resource_provider,
     int max_copy_texture_chromium_size,
     bool use_partial_raster,
     int max_staging_buffer_usage_in_bytes,
-    bool use_rgba_4444_texture_format) {
+    ResourceFormat memory_efficient_format,
+    ResourceFormat memory_efficient_format_with_alpha) {
   return make_scoped_ptr<TileTaskWorkerPool>(new OneCopyTileTaskWorkerPool(
       task_runner, task_graph_runner, resource_provider,
       max_copy_texture_chromium_size, use_partial_raster,
-      max_staging_buffer_usage_in_bytes, use_rgba_4444_texture_format));
+      max_staging_buffer_usage_in_bytes, memory_efficient_format,
+      memory_efficient_format_with_alpha));
 }
 
 OneCopyTileTaskWorkerPool::OneCopyTileTaskWorkerPool(
     base::SequencedTaskRunner* task_runner,
     TaskGraphRunner* task_graph_runner,
     ResourceProvider* resource_provider,
     int max_copy_texture_chromium_size,
     bool use_partial_raster,
     int max_staging_buffer_usage_in_bytes,
-    bool use_rgba_4444_texture_format)
+    ResourceFormat memory_efficient_format,
+    ResourceFormat memory_efficient_format_with_alpha)
     : task_runner_(task_runner),
       task_graph_runner_(task_graph_runner),
       namespace_token_(task_graph_runner->GetNamespaceToken()),
       resource_provider_(resource_provider),
       max_bytes_per_copy_operation_(
           max_copy_texture_chromium_size
               ? std::min(kMaxBytesPerCopyOperation,
                          max_copy_texture_chromium_size)
               : kMaxBytesPerCopyOperation),
       use_partial_raster_(use_partial_raster),
       bytes_scheduled_since_last_flush_(0),
       max_staging_buffer_usage_in_bytes_(max_staging_buffer_usage_in_bytes),
-      use_rgba_4444_texture_format_(use_rgba_4444_texture_format),
+      memory_efficient_format_(memory_efficient_format),
+      memory_efficient_format_with_alpha_(memory_efficient_format_with_alpha),
       staging_buffer_usage_in_bytes_(0),
       free_staging_buffer_usage_in_bytes_(0),
       staging_buffer_expiration_delay_(
           base::TimeDelta::FromMilliseconds(kStagingBufferExpirationDelayMs)),
       reduce_memory_usage_pending_(false),
       weak_ptr_factory_(this),
       task_set_finished_weak_ptr_factory_(this) {
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
       this, "OneCopyTileTaskWorkerPool", base::ThreadTaskRunnerHandle::Get());
   reduce_memory_usage_callback_ =
@@ skipping 112 matching lines @@
     TileTask* task = static_cast<TileTask*>(it->get());
 
     task->WillComplete();
     task->CompleteOnOriginThread(this);
     task->DidComplete();
   }
   completed_tasks_.clear();
 }
 
 ResourceFormat OneCopyTileTaskWorkerPool::GetResourceFormat(
     bool must_support_alpha) const {

[reveman, 2015/11/30 16:41:45]

Can we adjust this interface to match RendererSettings? That would be
GetMemoryEfficientResourceFormat+GetMemoryEfficientResourceFormatWithAlpha in
current patch.

[christiank, 2015/12/01 15:01:07]

Sure! That will leave it to TileManager::DetermineResourceFormat to determine
whether to use a memory efficient format or not. That should work.

-  return use_rgba_4444_texture_format_
-             ? RGBA_4444
-             : resource_provider_->best_texture_format();
+  if (memory_efficient_format_with_alpha_ != RGBA_8888 && must_support_alpha)

[reveman, 2015/11/30 16:41:45]

I think these RGBA_8888 checks will be confusing for someone who is not familiar
with this code. Can memory_efficient_format_ -> tile_resource_format_ and
reflect the value of best_texture_format()?

[christiank, 2015/12/01 15:01:07]

I agree that it's a bit confusing. I don't quite follow what you mean though,
care to elaborate a little? I don't see how we can avoid checking if we should
use a memory efficient format or not. Unless the default memory efficient format
is initialized to best_texture_format() by default which seems tricky.

For example if adjusting the interface like you suggested above, how would
TileManager::DetermineResourceFormat know whether to call
GetMemoryEfficientResourceFormat* or GetResourceFormat?

[reveman, 2015/12/01 17:27:24]

I think it would be much cleaner and easier to understand if the embedder just
had to tell the compositor what tile format to use and that's it. If I
understand this correctly, the problem is that if BGRA (or ETC) can be used
depends on an extension so the embedder is not aware of that.

https://code.google.com/p/chromium/codesearch#chromium/src/cc/resources/platf...

what if we allowed the embedder to specific the preferred tile format (BGRA if
SK_B32_SHIFT is set) but we fall-back to RGBA_8888 if it's not supported? and
that would apply to compressed formats too. Basically, there's no such thing as
memory efficient format anymore, there's just the preferred tile format as
specified by the embedder which we'll use if supported and RGBA_8888 if not.
Wdyt?

[christiank, 2015/12/02 16:18:14]

Now I see what you mean. Sounds good to me, definitely cleaner as you said.

I replaced the memory_efficient_format* settings with a new
preferred_tile_format setting. I decided to keep
PlatformColor::BestTextureFormat in the equation since we need that as a
fallback in case ETC1 is unsuitable (on Android we'd want to fallback to BGRA if
possible).

I hope it's okay with you that I iterate in the review like this rather than
agreeing on a final solution in one go? I think it allows for faster
turn-arounds which is good given the circumstances that I am leaving at the end
of next week :)

+    return memory_efficient_format_with_alpha_;
+
+  if (memory_efficient_format_ != RGBA_8888 && !must_support_alpha)
+    return memory_efficient_format_;
+
+  return resource_provider_->best_texture_format();
 }
 
 bool OneCopyTileTaskWorkerPool::GetResourceRequiresSwizzle(
     bool must_support_alpha) const {
   return !PlatformColor::SameComponentOrder(
       GetResourceFormat(must_support_alpha));
 }
 
 scoped_ptr<RasterBuffer> OneCopyTileTaskWorkerPool::AcquireBufferForRaster(
     const Resource* resource,
@@ skipping 123 matching lines @@
       // TODO(reveman): This avoids a performance problem on some ChromeOS
       // devices. This needs to be removed to support native GpuMemoryBuffer
       // implementations. crbug.com/436314
       gl->BeginQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM, staging_buffer->query_id);
 #else
       gl->BeginQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM,
                         staging_buffer->query_id);
 #endif
     }
 
-    int bytes_per_row =
-        (BitsPerPixel(resource->format()) * resource->size().width()) / 8;
-    int chunk_size_in_rows =
-        std::max(1, max_bytes_per_copy_operation_ / bytes_per_row);
-    // Align chunk size to 4. Required to support compressed texture formats.
-    chunk_size_in_rows = MathUtil::UncheckedRoundUp(chunk_size_in_rows, 4);
-    int y = 0;
-    int height = resource->size().height();
-    while (y < height) {
-      // Copy at most |chunk_size_in_rows|.
-      int rows_to_copy = std::min(chunk_size_in_rows, height - y);
-      DCHECK_GT(rows_to_copy, 0);
+    // Since compressed texture's cannot be pre-allocated we might have an
+    // unallocated resource in which case we need to perform a full size copy.
+    if (IsResourceFormatCompressed(resource->format())) {
+      gl->CompressedCopyTextureCHROMIUM(GL_TEXTURE_2D,
+                                        staging_buffer->texture_id,
+                                        resource_lock->texture_id());
+    } else {
+      int bytes_per_row =
+          (BitsPerPixel(resource->format()) * resource->size().width()) / 8;
+      int chunk_size_in_rows =
+          std::max(1, max_bytes_per_copy_operation_ / bytes_per_row);
+      // Align chunk size to 4. Required to support compressed texture formats.
+      chunk_size_in_rows = MathUtil::UncheckedRoundUp(chunk_size_in_rows, 4);
+      int y = 0;
+      int height = resource->size().height();
+      while (y < height) {
+        // Copy at most |chunk_size_in_rows|.
+        int rows_to_copy = std::min(chunk_size_in_rows, height - y);
+        DCHECK_GT(rows_to_copy, 0);
 
-      gl->CopySubTextureCHROMIUM(GL_TEXTURE_2D, staging_buffer->texture_id,
-                                 resource_lock->texture_id(), 0, y, 0, y,
-                                 resource->size().width(), rows_to_copy, false,
-                                 false, false);
-      y += rows_to_copy;
+        if (IsResourceFormatCompressed(resource->format())) {

[reveman, 2015/11/30 16:41:45]

This is never going to be true thanks to the check above. remove it?

[christiank, 2015/12/01 15:01:07]

Oh, that's sloppy of me. You're absolutely right. Fixed.

+          gl->CompressedCopySubTextureCHROMIUM(
+              GL_TEXTURE_2D, staging_buffer->texture_id,
+              resource_lock->texture_id(), 0, y, 0, y, resource->size().width(),
+              rows_to_copy);
+        } else {
+          gl->CopySubTextureCHROMIUM(GL_TEXTURE_2D, staging_buffer->texture_id,
+                                     resource_lock->texture_id(), 0, y, 0, y,
+                                     resource->size().width(), rows_to_copy,
+                                     false, false, false);
+        }
+        y += rows_to_copy;
 
-      // Increment |bytes_scheduled_since_last_flush_| by the amount of memory
-      // used for this copy operation.
-      bytes_scheduled_since_last_flush_ += rows_to_copy * bytes_per_row;
+        // Increment |bytes_scheduled_since_last_flush_| by the amount of memory
+        // used for this copy operation.
+        bytes_scheduled_since_last_flush_ += rows_to_copy * bytes_per_row;
 
-      if (bytes_scheduled_since_last_flush_ >= max_bytes_per_copy_operation_) {
-        gl->ShallowFlushCHROMIUM();
-        bytes_scheduled_since_last_flush_ = 0;
+        if (bytes_scheduled_since_last_flush_ >=
+            max_bytes_per_copy_operation_) {
+          gl->ShallowFlushCHROMIUM();
+          bytes_scheduled_since_last_flush_ = 0;
+        }
       }
     }
 
     if (resource_provider_->use_sync_query()) {
 #if defined(OS_CHROMEOS)
       gl->EndQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM);
 #else
       gl->EndQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM);
 #endif
     }
@@ skipping 296 matching lines @@
 
   staging_state->SetInteger("staging_buffer_count",
                             static_cast<int>(buffers_.size()));
   staging_state->SetInteger("busy_count",
                             static_cast<int>(busy_buffers_.size()));
   staging_state->SetInteger("free_count",
                             static_cast<int>(free_buffers_.size()));
 }
 
 }  // namespace cc