Gpu Image Decode Controller

cc/tiles/gpu_image_decode_controller.cc

 // Copyright 2016 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/tiles/gpu_image_decode_controller.h"
 
+#include "base/memory/discardable_memory_allocator.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/stringprintf.h"
+#include "base/thread_task_runner_handle.h"
 #include "cc/debug/devtools_instrumentation.h"
+#include "cc/output/context_provider.h"
 #include "cc/raster/tile_task_runner.h"
+#include "gpu/command_buffer/client/context_support.h"
+#include "gpu/command_buffer/client/gles2_interface.h"
+#include "gpu_image_decode_controller.h"
 #include "skia/ext/refptr.h"
+#include "skia/ext/texture_handle.h"
+#include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkSurface.h"
+#include "third_party/skia/include/gpu/GrContext.h"
+#include "third_party/skia/include/gpu/GrTexture.h"
+#include "ui/gfx/skia_util.h"
+#include "ui/gl/trace_util.h"
 
 namespace cc {
+namespace {
 
+static const int kMaxGpuImageBytes = 1024 * 1024 * 96;
+static const int kMaxDiscardableItems = 1000;
+
+// Returns true if an image would not be drawn and should therefore be
+// skipped rather than decoded.
+bool SkipImage(const DrawImage& draw_image) {
+  if (!SkIRect::Intersects(draw_image.src_rect(), draw_image.image()->bounds()))
+    return true;
+  if (draw_image.matrix_is_decomposable() &&
+      (draw_image.scale().width() == 0 || draw_image.scale().height() == 0))

[vmpstr, 2016/03/25 22:37:30]

width() and height() are floats, so maybe check if std::abs(width) < epsilon?

[ericrk, 2016/03/28 22:10:52]

Done.

+    return true;
+  return false;
+}
+
+SkImage::DeferredTextureImageUsageParams ParamsFromDrawImage(
+    const DrawImage& draw_image) {
+  SkImage::DeferredTextureImageUsageParams params;
+  params.fMatrix = draw_image.matrix();
+  params.fQuality = draw_image.filter_quality();
+
+  return params;
+}
+
+SkImageInfo ImageInfoFromDrawImage(const DrawImage& draw_image) {
+  return SkImageInfo::MakeN32Premul(draw_image.image()->width(),

[vmpstr, 2016/03/25 22:37:29]

The software idc changed to use different info depending on the desired format.
I don't know if that applies to GPU, but just fyi.

[ericrk, 2016/03/28 22:10:52]

refactored both to use a new function in resource_format.h

+                                    draw_image.image()->height());
+}
+
+}  // namespace
+
+// Task which decodes an image and stores the result in discardable memory.
+// This task does not use GPU resources and can be run on any thread.
 class ImageDecodeTaskImpl : public ImageDecodeTask {
  public:
   ImageDecodeTaskImpl(GpuImageDecodeController* controller,
-                      const DrawImage& image,
+                      const DrawImage& draw_image,
                       uint64_t source_prepare_tiles_id)
       : controller_(controller),
-        image_(image),
-        image_ref_(skia::SharePtr(image.image())),
+        image_(draw_image),
+        image_ref_(skia::SharePtr(draw_image.image())),
         source_prepare_tiles_id_(source_prepare_tiles_id) {}
 
   // Overridden from Task:
   void RunOnWorkerThread() override {
     TRACE_EVENT2("cc", "ImageDecodeTaskImpl::RunOnWorkerThread", "mode", "gpu",
                  "source_prepare_tiles_id", source_prepare_tiles_id_);
-    devtools_instrumentation::ScopedImageDecodeTask image_decode_task(
-        image_ref_.get());
     controller_->DecodeImage(image_);
   }
 
   // Overridden from TileTask:
   void ScheduleOnOriginThread(TileTaskClient* client) override {}
   void CompleteOnOriginThread(TileTaskClient* client) override {
-    controller_->RemovePendingTaskForImage(image_);
+    controller_->RemovePendingDecodeTaskForImage(image_);
   }
 
  protected:
   ~ImageDecodeTaskImpl() override {}
 
  private:
   GpuImageDecodeController* controller_;
   DrawImage image_;
   skia::RefPtr<const SkImage> image_ref_;
+  const uint64_t source_prepare_tiles_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageDecodeTaskImpl);
+};
+
+// Task which creates an image from decoded data. Typically this involves
+// uploading data to the GPU, which requires this task be run on the non-
+// concurrent thread.
+class ImageUploadTaskImpl : public ImageDecodeTask {
+ public:
+  ImageUploadTaskImpl(GpuImageDecodeController* controller,
+                      const DrawImage& draw_image,
+                      uint64_t source_prepare_tiles_id)
+      : ImageDecodeTask(controller->GetImageDecodeTaskAndRefLocked(
+            draw_image,
+            source_prepare_tiles_id)),
+        controller_(controller),
+        image_(draw_image),
+        image_ref_(skia::SharePtr(draw_image.image())),
+        source_prepare_tiles_id_(source_prepare_tiles_id) {}
+
+  // Override from Task:
+  void RunOnWorkerThread() override {
+    TRACE_EVENT2("cc", "ImageUploadTaskImpl::RunOnWorkerThread", "mode", "gpu",
+                 "source_prepare_tiles_id", source_prepare_tiles_id_);
+    controller_->UploadImage(image_);
+  }
+
+  void ScheduleOnOriginThread(TileTaskClient* client) override {}
+  void CompleteOnOriginThread(TileTaskClient* client) override {
+    // We're done with the image decode, so un-ref it.
+    controller_->UnrefImageDecode(image_);
+    controller_->RemovePendingUploadTaskForImage(image_);
+  }
+
+  // Override from ImageDecodeTask:
+  bool SupportsConcurrentExecution() const override { return false; }
+
+ protected:
+  ~ImageUploadTaskImpl() override {}
+
+ private:
+  GpuImageDecodeController* controller_;
+  DrawImage image_;
+  skia::RefPtr<const SkImage> image_ref_;
   uint64_t source_prepare_tiles_id_;
 
-  DISALLOW_COPY_AND_ASSIGN(ImageDecodeTaskImpl);
+  DISALLOW_COPY_AND_ASSIGN(ImageUploadTaskImpl);
 };
 
-GpuImageDecodeController::GpuImageDecodeController() {}
+GpuImageDecodeController::ImageData::ImageData(DecodedDataMode mode,
+                                               size_t size)
+    : mode(mode),
+      size(size),
+      decoded_data_ref_count(0),
+      decoded_data_is_locked(false),
+      image_pending(false),
+      image_ref_count(0),
+      is_at_raster(false) {}
+
+GpuImageDecodeController::ImageData::~ImageData() = default;
+
+GpuImageDecodeController::GpuImageDecodeController(ContextProvider* context)
+    : image_data_(decltype(image_data_)::NO_AUTO_EVICT),

[vmpstr, 2016/03/25 22:37:29]

Can you typedef the map in the header instead :)

[ericrk, 2016/03/28 22:10:52]

haha, forgot I had this :P - sure.

+      cached_items_limit_(kMaxDiscardableItems),
+      cached_bytes_limit_(kMaxGpuImageBytes),
+      bytes_used_(0),
+      context_(context),
+      context_threadsafe_proxy_(context->GrContext()->threadSafeProxy()) {}
 
 GpuImageDecodeController::~GpuImageDecodeController() {}
 
 bool GpuImageDecodeController::GetTaskForImageAndRef(
-    const DrawImage& image,
+    const DrawImage& draw_image,
     uint64_t prepare_tiles_id,
     scoped_refptr<ImageDecodeTask>* task) {
-  auto image_id = image.image()->uniqueID();
-  base::AutoLock lock(lock_);
-  if (prerolled_images_.count(image_id) != 0) {
+  if (SkipImage(draw_image)) {
     *task = nullptr;
     return false;
   }
 
+  base::AutoLock lock(lock_);
+
+  const auto image_id = draw_image.image()->uniqueID();
+
+  auto found = image_data_.Get(image_id);
+  if (found != image_data_.end() && found->second->uploaded_image) {
+    // The image is already uploaded. If the image is not at raster, all we have
+    // to do is ref and return it. If the image is at raster, we should
+    // convert it to a non-at-raster image if possible. Otherwise we will
+    // continue using it in at-raster mode.
+    if (found->second->is_at_raster) {
+      if (EnsureCapacityLocked(found->second->size)) {
+        // Convert to non-at-raster, we have room now.
+        found->second->is_at_raster = false;
+      } else {
+        // Image won't fit in memory, Do at-raster decode.
+        *task = nullptr;
+        return false;
+      }
+    }
+    RefImageLocked(draw_image);
+    *task = nullptr;
+    return true;
+  }
+
+  // We didn't have a pre-uploaded image. Create or find the upload task.
   scoped_refptr<ImageDecodeTask>& existing_task =
-      pending_image_tasks_[image_id];
+      pending_image_upload_tasks_[image_id];
+  if (!existing_task) {

[vmpstr, 2016/03/25 22:37:29]

It might be less indentation to do

if (existing_task) {
  RefImageLocked(existing_task);
  *task = existing_task;
  return true;
}

/* else code here */

[ericrk, 2016/03/28 22:10:52]

sure... less indentation is good.

+    // We will be decoding and uploading image data. Create a placeholder
+    // ImageData to hold the result.
+    scoped_ptr<ImageData> new_data;
+    ImageData* data;
+    if (found == image_data_.end()) {
+      new_data = NewImageData(draw_image);
+      data = new_data.get();
+    } else {
+      data = found->second.get();
+    }
+
+    if (!EnsureCapacityLocked(data->size)) {
+      // Image won't fit in memory. Do at-raster decode.
+      *task = nullptr;
+      return false;
+    }
+
+    if (new_data)
+      image_data_.Put(image_id, std::move(new_data));
+
+    bytes_used_ += data->size;
+    data->image_pending = true;
+    existing_task = make_scoped_refptr(
+        new ImageUploadTaskImpl(this, draw_image, prepare_tiles_id));
+  }
+
+  RefImageLocked(draw_image);
+  *task = existing_task;
+  return true;
+}
+
+void GpuImageDecodeController::UnrefImage(const DrawImage& draw_image) {
+  base::AutoLock lock(lock_);
+  UnrefImageLocked(draw_image);

[vmpstr, 2016/03/25 22:37:30]

There's two different uses of Locked it seems. One is for the context lock and
one is for |lock_|. Is it possible to rename the functions to make it more clear
which lock is needed?

[ericrk, 2016/03/28 22:10:52]

Ok... let me know how the new names are.

+}
+
+DecodedDrawImage GpuImageDecodeController::GetDecodedImageForDraw(
+    const DrawImage& draw_image) {
+  TRACE_EVENT0("cc", "GpuImageDecodeController::GetDecodedImageForDraw");
+  if (SkipImage(draw_image))
+    return DecodedDrawImage(nullptr, draw_image.filter_quality());
+
+  const uint32_t unique_id = draw_image.image()->uniqueID();
+
+  context_->GetLock()->AssertAcquired();
+  base::AutoLock lock(lock_);
+  auto found = image_data_.Peek(unique_id);
+  if (found == image_data_.end()) {
+    auto data = NewImageData(draw_image);
+    if (!EnsureCapacityLocked(data->size)) {

[vmpstr, 2016/03/25 22:37:30]

When would it be the case that we get to GetDecodedImageForDraw, we don't have
the image prepared yet, and it fits into memory. Wouldn't this have been done by
the decode/upload task? (aka, should data->is_at_raster always be set to true
here?)

[ericrk, 2016/03/28 22:10:52]

Likely not possible. Converting to a DCHECK, will test a bit more, but even if
we hit this it's not terrible.

+      // Image won't fit in memory. Mark it as at-raster so it will not be
+      // retained longer than necessary.
+      data->is_at_raster = true;
+    }
+    bytes_used_ += data->size;
+    data->image_pending = true;
+    found = image_data_.Put(unique_id, std::move(data));
+  }
+  RefImageLocked(draw_image);
+
+  // We may or may not need to decode and upload the image we've found, the
+  // following functions early-out to if we already decoded.
+  DecodeImageIfNecessaryLocked(draw_image, found->second.get());
+  UploadImageIfNecessaryLocked(draw_image, found->second.get());
+  SkImage* image = found->second->uploaded_image.get();

[vmpstr, 2016/03/25 22:37:29]

DCHECK(image)? or can it be null here?

[ericrk, 2016/03/28 22:10:52]

Done.

+
+  DecodedDrawImage decoded_draw_image(image, draw_image.filter_quality());
+  decoded_draw_image.set_at_raster_decode(found->second->is_at_raster);
+  return decoded_draw_image;
+}
+
+void GpuImageDecodeController::DrawWithImageFinished(
+    const DrawImage& draw_image,
+    const DecodedDrawImage& decoded_draw_image) {
+  if (SkipImage(draw_image))
+    return;
+
+  UnrefImage(draw_image);
+
+  // We are mid-draw and holding the context lock, ensure we clean up any
+  // textures (especially at-raster), which may have just been marked for
+  // deletion by UnrefImage.
+  DeletePendingImagesLocked();
+}
+
+void GpuImageDecodeController::ReduceCacheUsage() {
+  base::AutoLock lock(lock_);
+  EnsureCapacityLocked(0);
+}
+
+void GpuImageDecodeController::DecodeImage(const DrawImage& draw_image) {
+  base::AutoLock lock(lock_);
+  auto found = image_data_.Peek(draw_image.image()->uniqueID());
+  if (found == image_data_.end() ||
+      found->second->decoded_data_ref_count == 0) {
+    // All references to this image decode were released before the task had a
+    // chance
+    // to run. Early out.
+    return;
+  }
+
+  // Take a ref on the image to ensure it stays alive while decode is in
+  // progress.
+  RefImageDecodeLocked(draw_image);
+  DecodeImageIfNecessaryLocked(draw_image, found->second.get());
+
+  // Remove the pending task before unreffing the image to ensure that
+  // the image is still alive while any caller might try to re-use this
+  // task.
+  RemovePendingDecodeTaskForImageLocked(draw_image);
+  UnrefImageDecodeLocked(draw_image);
+}
+
+void GpuImageDecodeController::UploadImage(const DrawImage& draw_image) {
+  ContextProvider::ScopedContextLock context_lock(context_);
+  base::AutoLock lock(lock_);
+  auto found = image_data_.Peek(draw_image.image()->uniqueID());
+  if (found == image_data_.end() || found->second->image_ref_count == 0) {
+    // All references to this image were released before the task had a chance
+    // to run. Early out.
+    return;
+  }
+  DCHECK(found != image_data_.end());
+
+  // We are about to upload a new image and are holding the context lock.
+  // Ensure that any images which have been marked for deletion are actually
+  // cleaned up so we don't exceed our memory limit during this upload.
+  DeletePendingImagesLocked();
+
+  // Take a ref on the image to ensure it stays alive while upload is in
+  // progress.
+  RefImageLocked(draw_image);
+  UploadImageIfNecessaryLocked(draw_image, found->second.get());
+
+  // Remove the pending task before unreffing the image to ensure that
+  // the image is still alive while any caller might try to re-use this
+  // task.
+  RemovePendingUploadTaskForImageLocked(draw_image);
+  UnrefImageLocked(draw_image);
+}
+
+void GpuImageDecodeController::RemovePendingDecodeTaskForImage(
+    const DrawImage& draw_image) {
+  base::AutoLock lock(lock_);
+  RemovePendingDecodeTaskForImageLocked(draw_image);
+}
+
+void GpuImageDecodeController::RemovePendingUploadTaskForImage(
+    const DrawImage& draw_image) {
+  base::AutoLock lock(lock_);
+  RemovePendingUploadTaskForImageLocked(draw_image);
+}
+
+scoped_refptr<ImageDecodeTask>
+GpuImageDecodeController::GetImageDecodeTaskAndRefLocked(
+    const DrawImage& draw_image,
+    uint64_t prepare_tiles_id) {
+  lock_.AssertAcquired();
+  uint32_t image_id = draw_image.image()->uniqueID();
+  RefImageDecodeLocked(draw_image);
+
+  auto found = image_data_.Peek(image_id);
+  if (found != image_data_.end() && found->second->decoded_data_is_locked) {
+    // We should never be creating a decode task for an at raster image.
+    DCHECK(!found->second->is_at_raster);
+    return nullptr;
+  }
+
+  scoped_refptr<ImageDecodeTask>& existing_task =
+      pending_image_decode_tasks_[image_id];
   if (!existing_task) {
     existing_task = make_scoped_refptr(
-        new ImageDecodeTaskImpl(this, image, prepare_tiles_id));
-  }
-  *task = existing_task;
+        new ImageDecodeTaskImpl(this, draw_image, prepare_tiles_id));
+  }
+  return existing_task;
+}
+
+void GpuImageDecodeController::UnrefImageDecode(const DrawImage& draw_image) {
+  base::AutoLock lock(lock_);
+  UnrefImageDecodeLocked(draw_image);
+}
+
+void GpuImageDecodeController::RefImageDecodeLocked(
+    const DrawImage& draw_image) {
+  lock_.AssertAcquired();
+  auto found = image_data_.Peek(draw_image.image()->uniqueID());
+  DCHECK(found != image_data_.end());
+  // At raster never uses Image Decode refs.
+  DCHECK(!found->second->is_at_raster);
+  ++found->second->decoded_data_ref_count;

[vmpstr, 2016/03/25 22:37:30]

DCHECK that the memory is locked maybe?

[ericrk, 2016/03/28 22:10:52]

We ref the image when we create a decode task - if that task hasn't run yet, we
may have a ref but not have data or have unlocked data (in the case that decode
is just an lock).

+}
+
+void GpuImageDecodeController::UnrefImageDecodeLocked(
+    const DrawImage& draw_image) {
+  lock_.AssertAcquired();
+  auto found = image_data_.Peek(draw_image.image()->uniqueID());
+  DCHECK(found != image_data_.end());
+  DCHECK_GT(found->second->decoded_data_ref_count, 0u);
+  // At raster never uses Image Decode refs.
+  DCHECK(!found->second->is_at_raster);
+  --found->second->decoded_data_ref_count;
+  if (found->second->decoded_data_ref_count == 0) {
+    if (found->second->decoded_data_is_locked) {
+      found->second->decoded_data_is_locked = false;
+      found->second->decoded_data->Unlock();
+    }
+    EnsureCapacityLocked(0);
+  }
+}
+
+void GpuImageDecodeController::RefImageLocked(const DrawImage& draw_image) {
+  lock_.AssertAcquired();
+  auto image_id = draw_image.image()->uniqueID();
+  auto found = image_data_.Peek(image_id);
+  DCHECK(found != image_data_.end());
+  ++found->second->image_ref_count;
+  if (found->second->image_ref_count == 1) {

[vmpstr, 2016/03/25 22:37:29]

Dead code?

[ericrk, 2016/03/28 22:10:52]

Done.

+  }
+}
+
+void GpuImageDecodeController::UnrefImageLocked(const DrawImage& draw_image) {
+  lock_.AssertAcquired();
+  auto image_id = draw_image.image()->uniqueID();
+  auto found = image_data_.Peek(image_id);
+  DCHECK(found != image_data_.end());
+  DCHECK_GT(found->second->image_ref_count, 0u);
+  uint32_t count = --found->second->image_ref_count;
+  if (count == 0) {
+    // Covert this image to not-at-raster. Its ref count is 0, so it will be

[vmpstr, 2016/03/25 22:37:30]

Why are we converting this to not-at-raster?

[ericrk, 2016/03/28 22:10:52]

Updated the comment.

+    // cleaned up in the next step if it doesn't fit anyway.
+    found->second->is_at_raster = false;
+    if (found->second->image_pending) {
+      found->second->image_pending = false;

[vmpstr, 2016/03/25 22:37:29]

This is kind of weird for me. If the task is pending, we just set the bool to
false and remove the memory. Is the task still pending though?

[ericrk, 2016/03/28 22:10:52]

This was meant to mean that the image was budgeted, but not uploaded yet. In
this case the task has been cancelled, so we just need to un-budget the memory.

+      bytes_used_ -= found->second->size;
+    }
+    EnsureCapacityLocked(0);
+  }
+}
+
+bool GpuImageDecodeController::EnsureCapacityLocked(size_t new_size) {
+  lock_.AssertAcquired();
+  const size_t new_elements = new_size ? 1 : 0;
+  if (CanFitSize(new_size) && CanFitCount(new_elements))
+    return true;
+
+  // While we are over memory or item capacity, we iterate through our set of
+  // cached image data in LRU order. For each image, we can do two things:
+  // 1) We can free the uploaded image, reducing the memory usage of the cache
+  // and 2) we can remove the entry entirely, reducing the count of elements in
+  // the cache.
+  for (auto it = image_data_.rbegin(); it != image_data_.rend();) {
+    if (it->second->decoded_data_ref_count == 0 &&

[vmpstr, 2016/03/25 22:37:29]

nit: Can you structure this as

if (... != 0 || ... != 0) {
 ++it;
 continue;
}

...

[ericrk, 2016/03/28 22:10:52]

Done.

[ericrk, 2016/03/28 22:10:52]

Done.

+        it->second->image_ref_count == 0) {
+      if (it->second->uploaded_image) {

[vmpstr, 2016/03/25 22:37:30]

uploaded_image implies that upload_pending is false right? Can you dcheck that
in here

[ericrk, 2016/03/28 22:10:52]

Done.

[ericrk, 2016/03/28 22:10:52]

Done.

+        DCHECK_GE(bytes_used_, it->second->size);
+        bytes_used_ -= it->second->size;
+        images_pending_deletion_.push_back(
+            std::move(it->second->uploaded_image));
+        it->second->uploaded_image = nullptr;
+      }
+      if (!CanFitCount(new_elements)) {
+        it = image_data_.Erase(it);
+      } else {
+        ++it;
+      }
+
+      if (CanFitSize(new_size) && CanFitCount(new_elements))
+        return true;
+    } else {
+      ++it;
+    }
+  }
+
+  // We couldn't reach a state where we are under our memory / count limites.
   return false;
 }
 
-void GpuImageDecodeController::UnrefImage(const DrawImage& image) {
-  NOTREACHED();
-}
-
-DecodedDrawImage GpuImageDecodeController::GetDecodedImageForDraw(
-    const DrawImage& draw_image) {
-  return DecodedDrawImage(draw_image.image(), draw_image.filter_quality());
-}
-
-void GpuImageDecodeController::DrawWithImageFinished(
-    const DrawImage& image,
-    const DecodedDrawImage& decoded_image) {}
-
-void GpuImageDecodeController::ReduceCacheUsage() {}
-
-void GpuImageDecodeController::DecodeImage(const DrawImage& image) {
-  image.image()->preroll();
-  base::AutoLock lock(lock_);
-  prerolled_images_.insert(image.image()->uniqueID());
-}
-
-void GpuImageDecodeController::RemovePendingTaskForImage(
-    const DrawImage& image) {
-  base::AutoLock lock(lock_);
-  pending_image_tasks_.erase(image.image()->uniqueID());
+bool GpuImageDecodeController::CanFitSize(size_t size) const {
+  return bytes_used_ + size <= cached_bytes_limit_;
+}
+
+bool GpuImageDecodeController::CanFitCount(size_t num_elements) const {
+  return image_data_.size() + num_elements <= cached_items_limit_;
+}
+
+void GpuImageDecodeController::DecodeImageIfNecessaryLocked(
+    const DrawImage& draw_image,
+    ImageData* image_data) {
+  lock_.AssertAcquired();
+  if (image_data->uploaded_image) {
+    // We already have an uploaded image, no reason to decode.
+    return;
+  }
+  if (image_data->decoded_data && (image_data->decoded_data_is_locked ||
+                                   image_data->decoded_data->Lock())) {
+    // We already decoded this, or we just needed to lock, early out.
+    image_data->decoded_data_is_locked = true;
+    return;
+  }
+
+  TRACE_EVENT0("cc", "GpuImageDecodeController::DecodeImage");
+  scoped_ptr<base::DiscardableMemory> backing_memory;
+  {
+    base::AutoUnlock unlock(lock_);
+    if (image_data->mode == DECODED_DATA_MODE_CPU) {

[vmpstr, 2016/03/25 22:37:29]

Can you make this a switch, so the "else" case is clear too.

[ericrk, 2016/03/28 22:10:52]

Done.

+      backing_memory = base::DiscardableMemoryAllocator::GetInstance()
+                           ->AllocateLockedDiscardableMemory(image_data->size);
+      memset(backing_memory->data(), 0, image_data->size);

[vmpstr, 2016/03/25 22:37:30]

Why the clear?

[ericrk, 2016/03/28 22:10:52]

This shouldn't be here.

+      SkImageInfo image_info = ImageInfoFromDrawImage(draw_image);
+      bool result = draw_image.image()->readPixels(
+          image_info, backing_memory->data(), image_info.minRowBytes(), 0, 0,
+          SkImage::kDisallow_CachingHint);
+      if (!result) {
+        backing_memory->Unlock();
+        return;
+      }
+    } else {
+      backing_memory = base::DiscardableMemoryAllocator::GetInstance()
+                           ->AllocateLockedDiscardableMemory(image_data->size);
+      memset(backing_memory->data(), 0, image_data->size);
+      auto params = ParamsFromDrawImage(draw_image);
+      size_t data_size = draw_image.image()->getDeferredTextureImageData(
+          *context_threadsafe_proxy_, &params, 1, backing_memory->data());
+
+      if (!data_size) {
+        backing_memory->Unlock();
+        return;
+      }
+    }
+  }
+  image_data->decoded_data = std::move(backing_memory);
+  image_data->decoded_data_is_locked = true;
+}
+
+void GpuImageDecodeController::UploadImageIfNecessaryLocked(
+    const DrawImage& draw_image,
+    ImageData* image_data) {
+  lock_.AssertAcquired();
+  if (image_data->uploaded_image) {
+    // Someone has uploaded this image before us (at raster).
+    return;
+  }
+  TRACE_EVENT0("cc", "GpuImageDecodeController::UploadImage");
+  DCHECK(image_data->decoded_data_is_locked);
+
+  skia::RefPtr<SkImage> uploaded_image;
+  {
+    base::AutoUnlock unlock(lock_);
+    if (image_data->mode == DECODED_DATA_MODE_CPU) {

[vmpstr, 2016/03/25 22:37:29]

switch here too please

[ericrk, 2016/03/28 22:10:52]

Done.

+      SkImageInfo image_info = ImageInfoFromDrawImage(draw_image);
+      uploaded_image = skia::AdoptRef(SkImage::NewFromRaster(
+          image_info, image_data->decoded_data->data(),
+          image_info.minRowBytes(), [](const void*, void*) {}, nullptr));
+    } else {
+      uploaded_image = skia::AdoptRef(SkImage::NewFromDeferredTextureImageData(
+          context_->GrContext(), image_data->decoded_data->data(),
+          SkBudgeted::kNo));
+    }
+  }
+
+  image_data->uploaded_image = uploaded_image;
+  image_data->image_pending = false;
+}
+
+scoped_ptr<GpuImageDecodeController::ImageData>
+GpuImageDecodeController::NewImageData(const DrawImage& draw_image) {

[vmpstr, 2016/03/25 22:37:30]

maybe CreateImageData?

[ericrk, 2016/03/28 22:10:52]

sure

+  DecodedDataMode mode;
+  SkImageInfo info = ImageInfoFromDrawImage(draw_image);
+  auto params = ParamsFromDrawImage(draw_image);

[vmpstr, 2016/03/25 22:37:30]

SkImageInfo info ...
auto params ...

why auto? Is the type horrible? :)

[ericrk, 2016/03/28 22:10:52]

I guess not :P

+  size_t data_size = draw_image.image()->getDeferredTextureImageData(
+      *context_threadsafe_proxy_, &params, 1, nullptr);
+
+  if (data_size == 0) {

[vmpstr, 2016/03/25 22:37:30]

0 is returned in case of general failure, not just too large right?

[ericrk, 2016/03/28 22:10:52]

Done.

+    // Can't upload image, too large. Use SW fallback.
+    data_size = info.getSafeSize(info.minRowBytes());
+    mode = GpuImageDecodeController::DECODED_DATA_MODE_CPU;
+  } else {
+    mode = GpuImageDecodeController::DECODED_DATA_MODE_GPU;

[vmpstr, 2016/03/25 22:37:29]

s/GpuImageDecodeController:://

[ericrk, 2016/03/28 22:10:52]

Done.

+  }
+
+  return make_scoped_ptr(new ImageData(mode, data_size));
+}
+
+void GpuImageDecodeController::RemovePendingDecodeTaskForImageLocked(
+    const DrawImage& draw_image) {
+  lock_.AssertAcquired();
+  pending_image_decode_tasks_.erase(draw_image.image()->uniqueID());
+}
+
+void GpuImageDecodeController::RemovePendingUploadTaskForImageLocked(
+    const DrawImage& draw_image) {
+  lock_.AssertAcquired();
+  pending_image_upload_tasks_.erase(draw_image.image()->uniqueID());
+}
+
+void GpuImageDecodeController::DeletePendingImagesLocked() {
+  context_->GetLock()->AssertAcquired();
+  lock_.AssertAcquired();
+  images_pending_deletion_.clear();
 }
 
 }  // namespace cc