RasterInterface plumbing for image decode acceleration.

This CL adds a ScheduleImageDecode() method to RasterInterface that
locks a transfer cache entry and sends an image decode request. This is
done so that it's convenient for the GpuImageDecodeCache to eventually
request image decodes.

Note that a new interface was introduced:
ImageDecodeAcceleratorInterface. This is necessary to allow
RasterImplementation to use ImageDecodeAcceleratorProxy without breaking
dependencies.

Bug: 868400
Change-Id: Ib24ffa9f5855cc843e1bf83ef4cd2ebb039c04d8
Reviewed-on: https://chromium-review.googlesource.com/c/1313671Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611304}

gpu/command_buffer/client/BUILD.gn

@@ -72,6 +72,7 @@ jumbo_source_set("client_sources") {
     "gpu_control.h",
     "gpu_memory_buffer_manager.cc",
     "gpu_memory_buffer_manager.h",
+    "image_decode_accelerator_interface.h",
     "mapped_memory.cc",
     "mapped_memory.h",
     "ring_buffer.cc",
@@ -184,6 +185,7 @@ source_set("raster_interface") {
   deps = [
     "//base",
     "//components/viz/common:resource_format",
+    "//gpu/command_buffer/common",
     "//ui/gfx:buffer_types",
   ]
 }

gpu/command_buffer/client/client_transfer_cache.cc

@@ -86,11 +86,14 @@ void ClientTransferCache::StartTransferCacheEntry(
   EntryKey key(type, id);
 
   base::AutoLock hold(lock_);
+  auto handle = CreateDiscardableHandle(key);
+  if (!handle.IsValid())
+    return;
 
   // Call |create_entry_cb| while |lock_| is held so that in case another thread
   // tries to lock the cache entry later, it can assume that the creation of the
   // service-side cache entry has been triggered.
-  std::move(create_entry_cb).Run(CreateDiscardableHandle(key));
+  std::move(create_entry_cb).Run(handle);
 }
 
 ClientDiscardableHandle ClientTransferCache::CreateDiscardableHandle(

gpu/command_buffer/client/client_transfer_cache.h

@@ -89,7 +89,8 @@ class GLES2_IMPL_EXPORT ClientTransferCache {
   // message. This external mechanism should guarantee that it is safe for
   // command buffer commands to reference the cache entry after
   // |create_entry_cb| returns. Note that this function calls |create_entry_cb|
-  // before returning.
+  // before returning. |create_entry_cb| is not called if the
+  // ClientDiscardableHandle could not be created.
   void StartTransferCacheEntry(
       uint32_t type,
       uint32_t id,

gpu/command_buffer/client/image_decode_accelerator_interface.h

+// Copyright 2018 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.
+
+#ifndef GPU_COMMAND_BUFFER_CLIENT_IMAGE_DECODE_ACCELERATOR_INTERFACE_H_
+#define GPU_COMMAND_BUFFER_CLIENT_IMAGE_DECODE_ACCELERATOR_INTERFACE_H_
+
+#include "base/containers/span.h"
+#include "gpu/command_buffer/common/command_buffer_id.h"
+#include "gpu/command_buffer/common/sync_token.h"
+
+namespace gfx {
+class ColorSpace;
+class Size;
+}  // namespace gfx
+
+namespace gpu {
+
+// TODO(andrescj): move API documentation from ImageDecodeAcceleratorProxy to
+// here.
+class ImageDecodeAcceleratorInterface {
+ public:
+  virtual ~ImageDecodeAcceleratorInterface() {}
+
+  virtual SyncToken ScheduleImageDecode(
+      base::span<const uint8_t> encoded_data,
+      const gfx::Size& output_size,
+      CommandBufferId raster_decoder_command_buffer_id,
+      uint32_t transfer_cache_entry_id,
+      int32_t discardable_handle_shm_id,
+      uint32_t discardable_handle_shm_offset,
+      const gfx::ColorSpace& target_color_space,
+      bool needs_mips) = 0;
+};
+
+}  // namespace gpu
+
+#endif  // GPU_COMMAND_BUFFER_CLIENT_IMAGE_DECODE_ACCELERATOR_INTERFACE_H_
\ No newline at end of file

gpu/command_buffer/client/raster_implementation.cc

@@ -18,6 +18,7 @@
 #include <string>
 
 #include "base/atomic_sequence_num.h"
+#include "base/bind.h"
 #include "base/bits.h"
 #include "base/compiler_specific.h"
 #include "base/no_destructor.h"
@@ -36,11 +37,11 @@
 #include "cc/paint/transfer_cache_entry.h"
 #include "cc/paint/transfer_cache_serialize_helper.h"
 #include "gpu/command_buffer/client/gpu_control.h"
+#include "gpu/command_buffer/client/image_decode_accelerator_interface.h"
 #include "gpu/command_buffer/client/query_tracker.h"
 #include "gpu/command_buffer/client/raster_cmd_helper.h"
 #include "gpu/command_buffer/client/shared_memory_limits.h"
 #include "gpu/command_buffer/client/transfer_buffer.h"
-#include "gpu/command_buffer/common/sync_token.h"
 #include "ui/gfx/color_space.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_f.h"
@@ -281,7 +282,8 @@ RasterImplementation::RasterImplementation(
     TransferBufferInterface* transfer_buffer,
     bool bind_generates_resource,
     bool lose_context_when_out_of_memory,
-    GpuControl* gpu_control)
+    GpuControl* gpu_control,
+    ImageDecodeAcceleratorInterface* image_decode_accelerator)
     : ImplementationBase(helper, transfer_buffer, gpu_control),
       helper_(helper),
       active_texture_unit_(0),
@@ -293,7 +295,8 @@ RasterImplementation::RasterImplementation(
       font_manager_(this, helper->command_buffer()),
       lost_(false),
       max_inlined_entry_size_(kMaxTransferCacheEntrySizeForTransferBuffer),
-      transfer_cache_(this) {
+      transfer_cache_(this),
+      image_decode_accelerator_(image_decode_accelerator) {
   DCHECK(helper);
   DCHECK(transfer_buffer);
   DCHECK(gpu_control);
@@ -1183,6 +1186,44 @@ void RasterImplementation::EndRasterCHROMIUM() {
     FlushPaintCachePurgedEntries();
 }
 
+SyncToken RasterImplementation::ScheduleImageDecode(
+    base::span<const uint8_t> encoded_data,
+    const gfx::Size& output_size,
+    uint32_t transfer_cache_entry_id,
+    const gfx::ColorSpace& target_color_space,
+    bool needs_mips) {
+  // It's safe to use base::Unretained(this) here because
+  // StartTransferCacheEntry() will call the callback before returning.
+  SyncToken decode_sync_token;
+  transfer_cache_.StartTransferCacheEntry(
+      static_cast<uint32_t>(cc::TransferCacheEntryType::kImage),
+      transfer_cache_entry_id,
+      base::BindOnce(&RasterImplementation::IssueImageDecodeCacheEntryCreation,
+                     base::Unretained(this), encoded_data, output_size,
+                     transfer_cache_entry_id, target_color_space, needs_mips,
+                     &decode_sync_token));
+  return decode_sync_token;
+}
+
+void RasterImplementation::IssueImageDecodeCacheEntryCreation(
+    base::span<const uint8_t> encoded_data,
+    const gfx::Size& output_size,
+    uint32_t transfer_cache_entry_id,
+    const gfx::ColorSpace& target_color_space,
+    bool needs_mips,
+    SyncToken* decode_sync_token,
+    ClientDiscardableHandle handle) {
+  DCHECK(gpu_control_);
+  DCHECK(image_decode_accelerator_);
+  DCHECK(handle.IsValid());
+
+  // Send the decode request to the service.
+  *decode_sync_token = image_decode_accelerator_->ScheduleImageDecode(
+      encoded_data, output_size, gpu_control_->GetCommandBufferID(),
+      transfer_cache_entry_id, handle.shm_id(), handle.byte_offset(),
+      target_color_space, needs_mips);
+}
+
 void RasterImplementation::BeginGpuRaster() {
   NOTREACHED();
 }

gpu/command_buffer/client/raster_implementation.h

@@ -27,6 +27,7 @@
 #include "gpu/command_buffer/client/transfer_buffer.h"
 #include "gpu/command_buffer/common/context_result.h"
 #include "gpu/command_buffer/common/debug_marker_manager.h"
+#include "gpu/command_buffer/common/discardable_handle.h"
 #include "gpu/command_buffer/common/id_allocator.h"
 #include "gpu/command_buffer/common/raster_cmd_format.h"
 #include "gpu/raster_export.h"
@@ -39,6 +40,7 @@ class TransferCacheSerializeHelper;
 namespace gpu {
 
 class GpuControl;
+class ImageDecodeAcceleratorInterface;
 struct SharedMemoryLimits;
 
 namespace raster {
@@ -54,11 +56,13 @@ class RASTER_EXPORT RasterImplementation : public RasterInterface,
                                            public gles2::QueryTrackerClient,
                                            public ClientFontManager::Client {
  public:
-  RasterImplementation(RasterCmdHelper* helper,
-                       TransferBufferInterface* transfer_buffer,
-                       bool bind_generates_resource,
-                       bool lose_context_when_out_of_memory,
-                       GpuControl* gpu_control);
+  RasterImplementation(
+      RasterCmdHelper* helper,
+      TransferBufferInterface* transfer_buffer,
+      bool bind_generates_resource,
+      bool lose_context_when_out_of_memory,
+      GpuControl* gpu_control,
+      ImageDecodeAcceleratorInterface* image_decode_accelerator);
 
   ~RasterImplementation() override;
 
@@ -121,6 +125,11 @@ class RASTER_EXPORT RasterImplementation : public RasterInterface,
                       const gfx::Vector2dF& post_translate,
                       GLfloat post_scale,
                       bool requires_clear) override;
+  SyncToken ScheduleImageDecode(base::span<const uint8_t> encoded_data,
+                                const gfx::Size& output_size,
+                                uint32_t transfer_cache_entry_id,
+                                const gfx::ColorSpace& target_color_space,
+                                bool needs_mips) override;
   void BeginGpuRaster() override;
   void EndGpuRaster() override;
 
@@ -253,6 +262,15 @@ class RASTER_EXPORT RasterImplementation : public RasterInterface,
 
   const std::string& GetLogPrefix() const;
 
+  void IssueImageDecodeCacheEntryCreation(
+      base::span<const uint8_t> encoded_data,
+      const gfx::Size& output_size,
+      uint32_t transfer_cache_entry_id,
+      const gfx::ColorSpace& target_color_space,
+      bool needs_mips,
+      SyncToken* decode_sync_token,
+      ClientDiscardableHandle handle);
+
 // Set to 1 to have the client fail when a GL error is generated.
 // This helps find bugs in the renderer since the debugger stops on the error.
 #if DCHECK_IS_ON()
@@ -332,6 +350,8 @@ class RASTER_EXPORT RasterImplementation : public RasterInterface,
   cc::ClientPaintCache::PurgedData temp_paint_cache_purged_data_;
   std::unique_ptr<cc::ClientPaintCache> paint_cache_;
 
+  ImageDecodeAcceleratorInterface* image_decode_accelerator_;
+
   // Tracing helpers.
   int raster_chromium_id_ = 0;
 

gpu/command_buffer/client/raster_implementation_gles.cc

@@ -209,6 +209,16 @@ void RasterImplementationGLES::EndRasterCHROMIUM() {
   NOTREACHED();
 }
 
+SyncToken RasterImplementationGLES::ScheduleImageDecode(
+    base::span<const uint8_t> encoded_data,
+    const gfx::Size& output_size,
+    uint32_t transfer_cache_entry_id,
+    const gfx::ColorSpace& target_color_space,
+    bool needs_mips) {
+  NOTREACHED();
+  return SyncToken();
+}
+
 void RasterImplementationGLES::BeginGpuRaster() {
   // Using push/pop functions directly incurs cost to evaluate function
   // arguments even when tracing is disabled.

gpu/command_buffer/client/raster_implementation_gles.h

@@ -90,6 +90,13 @@ class RASTER_EXPORT RasterImplementationGLES : public RasterInterface {
                       bool requires_clear) override;
   void EndRasterCHROMIUM() override;
 
+  // Image decode acceleration.
+  SyncToken ScheduleImageDecode(base::span<const uint8_t> encoded_data,
+                                const gfx::Size& output_size,
+                                uint32_t transfer_cache_entry_id,
+                                const gfx::ColorSpace& target_color_space,
+                                bool needs_mips) override;
+
   // Raster via GrContext.
   void BeginGpuRaster() override;
   void EndGpuRaster() override;

gpu/command_buffer/client/raster_implementation_unittest.cc

@@ -124,7 +124,7 @@ class RasterImplementationTest : public testing::Test {
         gl_.reset(new RasterImplementation(
             helper_.get(), transfer_buffer_.get(),
             bind_generates_resource_client, lose_context_when_out_of_memory,
-            gpu_control_.get()));
+            gpu_control_.get(), nullptr /* image_decode_accelerator */));
       }
 
       // The client should be set to something non-null.

gpu/command_buffer/client/raster_interface.h

@@ -7,7 +7,9 @@
 
 #include <GLES2/gl2.h>
 #include "base/compiler_specific.h"
+#include "base/containers/span.h"
 #include "components/viz/common/resources/resource_format.h"
+#include "gpu/command_buffer/common/sync_token.h"
 
 namespace cc {
 class DisplayItemList;
@@ -16,6 +18,7 @@ struct RasterColorSpace;
 }  // namespace cc
 
 namespace gfx {
+class ColorSpace;
 class Rect;
 class Size;
 class Vector2dF;
@@ -52,6 +55,16 @@ class RasterInterface {
                               GLfloat post_scale,
                               bool requires_clear) = 0;
 
+  // Schedules a hardware-accelerated image decode and a sync token that's
+  // released when the image decode is complete. If the decode could not be
+  // scheduled, an empty sync token is returned.
+  virtual SyncToken ScheduleImageDecode(
+      base::span<const uint8_t> encoded_data,
+      const gfx::Size& output_size,
+      uint32_t transfer_cache_entry_id,
+      const gfx::ColorSpace& target_color_space,
+      bool needs_mips) = 0;
+
   // Raster via GrContext.
   virtual void BeginGpuRaster() = 0;
   virtual void EndGpuRaster() = 0;

gpu/ipc/client/image_decode_accelerator_proxy.cc

@@ -4,6 +4,8 @@
 
 #include "gpu/ipc/client/image_decode_accelerator_proxy.h"
 
+#include <vector>
+
 #include "gpu/command_buffer/common/constants.h"
 #include "gpu/ipc/client/gpu_channel_host.h"
 #include "gpu/ipc/common/command_buffer_id.h"
@@ -20,18 +22,24 @@ ImageDecodeAcceleratorProxy::ImageDecodeAcceleratorProxy(GpuChannelHost* host,
 ImageDecodeAcceleratorProxy::~ImageDecodeAcceleratorProxy() {}
 
 SyncToken ImageDecodeAcceleratorProxy::ScheduleImageDecode(
-    const std::vector<uint8_t>& encoded_data,
+    base::span<const uint8_t> encoded_data,
     const gfx::Size& output_size,
-    int32_t raster_decoder_route_id,
+    CommandBufferId raster_decoder_command_buffer_id,
     uint32_t transfer_cache_entry_id,
     int32_t discardable_handle_shm_id,
     uint32_t discardable_handle_shm_offset,
     const gfx::ColorSpace& target_color_space,
     bool needs_mips) {
+  DCHECK(host_);
+  DCHECK_EQ(host_->channel_id(),
+            ChannelIdFromCommandBufferId(raster_decoder_command_buffer_id));
+
   GpuChannelMsg_ScheduleImageDecode_Params params;
-  params.encoded_data = encoded_data;
+  params.encoded_data =
+      std::vector<uint8_t>(encoded_data.cbegin(), encoded_data.cend());
   params.output_size = output_size;
-  params.raster_decoder_route_id = raster_decoder_route_id;
+  params.raster_decoder_route_id =
+      RouteIdFromCommandBufferId(raster_decoder_command_buffer_id);
   params.transfer_cache_entry_id = transfer_cache_entry_id;
   params.discardable_handle_shm_id = discardable_handle_shm_id;
   params.discardable_handle_shm_offset = discardable_handle_shm_offset;
@@ -42,8 +50,8 @@ SyncToken ImageDecodeAcceleratorProxy::ScheduleImageDecode(
   uint64_t release_count = ++next_release_count_;
   // Note: we send the message under the lock to guarantee monotonicity of the
   // release counts as seen by the service.
-  host_->Send(
-      new GpuChannelMsg_ScheduleImageDecode(route_id_, params, release_count));
+  host_->Send(new GpuChannelMsg_ScheduleImageDecode(
+      route_id_, std::move(params), release_count));
   return SyncToken(
       CommandBufferNamespace::GPU_IO,
       CommandBufferIdFromChannelAndRoute(host_->channel_id(), route_id_),

gpu/ipc/client/image_decode_accelerator_proxy.h

@@ -5,17 +5,10 @@
 #ifndef GPU_IPC_CLIENT_IMAGE_DECODE_ACCELERATOR_PROXY_H_
 #define GPU_IPC_CLIENT_IMAGE_DECODE_ACCELERATOR_PROXY_H_
 
-#include <vector>
-
 #include "base/macros.h"
 #include "base/synchronization/lock.h"
 #include "base/thread_annotations.h"
-#include "gpu/command_buffer/common/sync_token.h"
-
-namespace gfx {
-class ColorSpace;
-class Size;
-}  // namespace gfx
+#include "gpu/command_buffer/client/image_decode_accelerator_interface.h"
 
 namespace gpu {
 class GpuChannelHost;
@@ -47,27 +40,28 @@ class GpuChannelHost;
 // Objects of this class are thread-safe.
 //
 // TODO(andrescj): actually put the decoder's capabilities in GpuInfo.
-class ImageDecodeAcceleratorProxy {
+class ImageDecodeAcceleratorProxy : public ImageDecodeAcceleratorInterface {
  public:
   ImageDecodeAcceleratorProxy(GpuChannelHost* host, int32_t route_id);
-  ~ImageDecodeAcceleratorProxy();
+  ~ImageDecodeAcceleratorProxy() override;
 
   // Schedules a hardware-accelerated image decode on the GPU process. The image
   // in |encoded_data| is decoded and scaled to |output_size|. Upon completion,
   // a service-side transfer cache entry will be created with the decoded data
   // using |transfer_cache_entry_id|, |discardable_handle_shm_id|, and
-  // |discardable_handle_shm_offset|. The |raster_decoder_route_id| is used to
-  // look up the appropriate command buffer and create the transfer cache entry
-  // correctly. Returns a sync token that will be released after the decode is
-  // done and the service-side transfer cache entry is created.
-  SyncToken ScheduleImageDecode(const std::vector<uint8_t>& encoded_data,
-                                const gfx::Size& output_size,
-                                int32_t raster_decoder_route_id,
-                                uint32_t transfer_cache_entry_id,
-                                int32_t discardable_handle_shm_id,
-                                uint32_t discardable_handle_shm_offset,
-                                const gfx::ColorSpace& target_color_space,
-                                bool needs_mips);
+  // |discardable_handle_shm_offset|. The |raster_decoder_command_buffer_id| is
+  // used to look up the appropriate command buffer and create the transfer
+  // cache entry correctly. Returns a sync token that will be released after the
+  // decode is done and the service-side transfer cache entry is created.
+  SyncToken ScheduleImageDecode(
+      base::span<const uint8_t> encoded_data,
+      const gfx::Size& output_size,
+      CommandBufferId raster_decoder_command_buffer_id,
+      uint32_t transfer_cache_entry_id,
+      int32_t discardable_handle_shm_id,
+      uint32_t discardable_handle_shm_offset,
+      const gfx::ColorSpace& target_color_space,
+      bool needs_mips) override;
 
  private:
   GpuChannelHost* const host_;

gpu/ipc/common/command_buffer_id.h

@@ -26,6 +26,11 @@ inline int ChannelIdFromCommandBufferId(
   return static_cast<int>(command_buffer_id.GetUnsafeValue() >> 32);
 }
 
+inline int32_t RouteIdFromCommandBufferId(
+    gpu::CommandBufferId command_buffer_id) {
+  return 0xffffffff & command_buffer_id.GetUnsafeValue();
+}
+
 }  // namespace gpu
 
 #endif  // GPU_IPC_COMMON_COMMAND_BUFFER_ID_H_

gpu/ipc/raster_in_process_context.cc

@@ -92,7 +92,8 @@ ContextResult RasterInProcessContext::Initialize(
 
   raster_implementation_ = std::make_unique<raster::RasterImplementation>(
       raster_helper.get(), transfer_buffer_.get(), bind_generates_resource,
-      attribs.lose_context_when_out_of_memory, command_buffer_.get());
+      attribs.lose_context_when_out_of_memory, command_buffer_.get(),
+      nullptr /* image_decode_accelerator */);
   result = raster_implementation_->Initialize(memory_limits);
   raster_implementation_->SetLostContextCallback(base::BindOnce(
       []() { EXPECT_TRUE(false) << "Unexpected lost context."; }));

services/ws/public/cpp/gpu/context_provider_command_buffer.cc

@@ -133,7 +133,7 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentThread() {
   // This command buffer is a client-side proxy to the command buffer in the
   // GPU process.
   command_buffer_ = std::make_unique<gpu::CommandBufferProxyImpl>(
-      std::move(channel_), gpu_memory_buffer_manager_, stream_id_, task_runner);
+      channel_, gpu_memory_buffer_manager_, stream_id_, task_runner);
   bind_result_ = command_buffer_->Initialize(
       surface_handle_, /*shared_command_buffer=*/nullptr, stream_priority_,
       attributes_, active_url_);
@@ -191,10 +191,12 @@ gpu::ContextResult ContextProviderCommandBuffer::BindToCurrentThread() {
 
     // The RasterImplementation exposes the RasterInterface, as well as the
     // gpu::ContextSupport interface.
+    DCHECK(channel_);
     auto raster_impl = std::make_unique<gpu::raster::RasterImplementation>(
         raster_helper.get(), transfer_buffer_.get(),
         attributes_.bind_generates_resource,
-        attributes_.lose_context_when_out_of_memory, command_buffer_.get());
+        attributes_.lose_context_when_out_of_memory, command_buffer_.get(),
+        channel_->image_decode_accelerator_proxy());
     bind_result_ = raster_impl->Initialize(memory_limits_);
     if (bind_result_ != gpu::ContextResult::kSuccess) {
       DLOG(ERROR) << "Failed to initialize RasterImplementation.";