gpu: Implement idle async pixel transfers.

ui/gl/async_pixel_transfer_delegate_idle.cc

+// Copyright (c) 2013 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 "ui/gl/async_pixel_transfer_delegate_idle.h"
+
+#include "base/bind.h"
+#include "base/debug/trace_event.h"
+#include "base/lazy_instance.h"
+#include "base/message_loop.h"
+#include "base/process_util.h"
+#include "base/shared_memory.h"
+#include "build/build_config.h"
+#include "ui/gl/gl_bindings.h"
+#include "ui/gl/gl_context.h"
+#include "ui/gl/gl_surface.h"
+#include "ui/gl/safe_shared_memory_pool.h"
+#include "ui/gl/scoped_make_current.h"
+
+using base::SharedMemory;
+using base::SharedMemoryHandle;
+
+namespace gfx {
+
+namespace {
+
+// Gets the address of the data from shared memory.
+void* GetAddress(SharedMemory* shared_memory, uint32 shm_data_offset) {
+  // Memory bounds have already been validated, so there
+  // is just DCHECKS here.
+  DCHECK(shared_memory);
+  DCHECK(shared_memory->memory());
+  return static_cast<int8*>(shared_memory->memory()) + shm_data_offset;
+}
+
+base::LazyInstance<SafeSharedMemoryPool> g_safe_shared_memory_pool =
+    LAZY_INSTANCE_INITIALIZER;
+
+SafeSharedMemoryPool* safe_shared_memory_pool() {
+  return g_safe_shared_memory_pool.Pointer();
+}
+
+} // namespace
+
+#if !defined(OS_ANDROID)
+scoped_ptr<AsyncPixelTransferDelegate>
+    AsyncPixelTransferDelegate::Create(gfx::GLContext* context) {
+  return AsyncPixelTransferDelegateIdle::Create(context);
+}
+#endif
+
+scoped_ptr<AsyncPixelTransferDelegate>
+      AsyncPixelTransferDelegateIdle::Create(gfx::GLContext* context) {
+  return make_scoped_ptr(
+      static_cast<AsyncPixelTransferDelegate*>(
+          new AsyncPixelTransferDelegateIdle()));
+}
+
+// Class which holds async pixel transfers state.
+class TransferStateInternalIdle
+    : public base::RefCounted<TransferStateInternalIdle> {
+ public:
+  explicit TransferStateInternalIdle(GLuint texture_id)
+      : texture_id_(texture_id),
+        transfer_in_progress_(false) {
+  }
+
+  // Implement AsyncPixelTransferState:
+  bool TransferIsInProgress() {
+    return transfer_in_progress_;
+  }
+
+ protected:
+  friend class base::RefCounted<TransferStateInternalIdle>;
+  friend class AsyncPixelTransferDelegateIdle;
+
+  virtual ~TransferStateInternalIdle() {}
+
+  GLuint texture_id_;
+
+  // Indicates that an async transfer is in progress.
+  bool transfer_in_progress_;
+};
+
+// This just wraps an internal ref-counted state object.
+class AsyncTransferStateIdle : public AsyncPixelTransferState {

[epenner, 2013/03/13 18:58:01]

See my comment on the transfers_ list in the header.

Hmm, on that note, possibly you don't need the 'internal' pattern for this
implementation?  Could you use weak-pointers to the original
AsyncPixelTransferState anywhere that accesses the state? If you do need some
state to persist after the AsyncPixelTransferState is deleted, then the
'internal' pattern is good, so long as you respect the deletion of the
AsyncPixelTransferState where it matters (accessing the texture id, and the
bind_callback).

[reveman, 2013/03/14 01:12:45]

Internal class gone in latest patch.

+ public:
+  explicit AsyncTransferStateIdle(GLuint texture_id)
+      : internal_(new TransferStateInternalIdle(texture_id)) {
+  }
+  virtual ~AsyncTransferStateIdle() {}
+  virtual bool TransferIsInProgress() {
+    return internal_->TransferIsInProgress();
+  }
+  scoped_refptr<TransferStateInternalIdle> internal_;
+};
+
+AsyncPixelTransferDelegateIdle::Transfer::Transfer(
+    TransferStateInternalIdle* state,
+    const base::Closure& task)
+    : state(state),
+      task(task) {
+}
+
+AsyncPixelTransferDelegateIdle::Transfer::~Transfer() {}
+
+AsyncPixelTransferDelegateIdle::AsyncPixelTransferDelegateIdle()
+    : texture_upload_count_(0),
+      texture_dirty_(false) {
+}
+
+AsyncPixelTransferDelegateIdle::~AsyncPixelTransferDelegateIdle() {
+}
+
+AsyncPixelTransferState*
+    AsyncPixelTransferDelegateIdle::CreateRawPixelTransferState(
+        GLuint texture_id,
+        const AsyncTexImage2DParams& define_params) {
+  return static_cast<AsyncPixelTransferState*>(
+      new AsyncTransferStateIdle(texture_id));
+}
+
+bool AsyncPixelTransferDelegateIdle::BindCompletedAsyncTransfers() {
+  bool texture_dirty = texture_dirty_;

[epenner, 2013/03/13 18:58:01]

This feels brittle to me and not implied from the function name etc.

Would it make sense to restore when the actual state is changed? If we _really_
want, we could use a 'restore state' closure to do this anywhere, but if it's
just one more function returning a boolean I still prefer that myself.

[reveman, 2013/03/14 01:12:45]

I made ProcessMorePendingTransfers() have a return value instead. We can do
better and not restore after each idle upload but I think this is good enough
for this patch.

+  texture_dirty_ = false;
+  return texture_dirty;
+}
+
+void AsyncPixelTransferDelegateIdle::AsyncNotifyCompletion(
+    const AsyncMemoryParams& mem_params,
+    const CompletionCallback& callback) {
+  if (transfers_.empty()) {
+    callback.Run(mem_params);
+    return;
+  }
+
+  transfers_.back().notifications.push(
+      base::Bind(
+          &AsyncPixelTransferDelegateIdle::PerformNotifyCompletion,
+          AsWeakPtr(),
+          mem_params,
+          base::Owned(new ScopedSafeSharedMemory(safe_shared_memory_pool(),
+                                                 mem_params.shared_memory,
+                                                 mem_params.shm_size)),
+          callback));
+}
+
+void AsyncPixelTransferDelegateIdle::AsyncTexImage2D(
+    AsyncPixelTransferState* transfer_state,
+    const AsyncTexImage2DParams& tex_params,
+    const AsyncMemoryParams& mem_params,
+    const base::Closure& bind_callback) {
+  scoped_refptr<TransferStateInternalIdle> state =
+      static_cast<AsyncTransferStateIdle*>(transfer_state)->internal_.get();
+  DCHECK(mem_params.shared_memory);
+  DCHECK_LE(mem_params.shm_data_offset + mem_params.shm_data_size,
+            mem_params.shm_size);
+  DCHECK(state);
+  DCHECK(state->texture_id_);
+
+  transfers_.push_back(
+      Transfer(
+          state.get(),
+          base::Bind(
+              &AsyncPixelTransferDelegateIdle::PerformAsyncTexImage2D,
+              AsWeakPtr(),
+              state,
+              tex_params,
+              mem_params,
+              bind_callback,
+              base::Owned(new ScopedSafeSharedMemory(safe_shared_memory_pool(),
+                                                     mem_params.shared_memory,
+                                                     mem_params.shm_size)))));
+
+  state->transfer_in_progress_ = true;
+}
+
+void AsyncPixelTransferDelegateIdle::AsyncTexSubImage2D(
+    AsyncPixelTransferState* transfer_state,
+    const AsyncTexSubImage2DParams& tex_params,
+    const AsyncMemoryParams& mem_params) {
+  scoped_refptr<TransferStateInternalIdle> state =
+      static_cast<AsyncTransferStateIdle*>(transfer_state)->internal_.get();
+  DCHECK(mem_params.shared_memory);
+  DCHECK_LE(mem_params.shm_data_offset + mem_params.shm_data_size,
+            mem_params.shm_size);
+  DCHECK(state);
+  DCHECK(state->texture_id_);
+
+  transfers_.push_back(
+      Transfer(
+          state.get(),
+          base::Bind(
+              &AsyncPixelTransferDelegateIdle::PerformAsyncTexSubImage2D,
+              AsWeakPtr(),
+              state,
+              tex_params,
+              mem_params,
+              base::Owned(new ScopedSafeSharedMemory(safe_shared_memory_pool(),
+                                                     mem_params.shared_memory,
+                                                     mem_params.shm_size)))));
+
+  state->transfer_in_progress_ = true;
+}
+
+void AsyncPixelTransferDelegateIdle::WaitForTransferCompletion(
+    AsyncPixelTransferState* transfer_state) {
+  scoped_refptr<TransferStateInternalIdle> state =
+      static_cast<AsyncTransferStateIdle*>(transfer_state)->internal_.get();
+
+  for (std::list<Transfer>::iterator iter = transfers_.begin();
+       iter != transfers_.end(); ++iter) {
+    if (iter->state.get() != state)
+      continue;
+
+    ProcessTransfer(*iter);
+    transfers_.erase(iter);
+    break;
+  }
+}
+
+uint32 AsyncPixelTransferDelegateIdle::GetTextureUploadCount() {
+  return texture_upload_count_;
+}
+
+base::TimeDelta AsyncPixelTransferDelegateIdle::GetTotalTextureUploadTime() {
+  return total_texture_upload_time_;
+}
+
+void AsyncPixelTransferDelegateIdle::ProcessPendingTransfers() {
+  if (transfers_.empty())
+    return;
+
+  ProcessTransfer(transfers_.front());
+  transfers_.pop_front();
+}
+
+bool AsyncPixelTransferDelegateIdle::NeedsProcessPendingTransfers() {
+  return !transfers_.empty();
+}
+
+void AsyncPixelTransferDelegateIdle::ProcessTransfer(Transfer& transfer) {
+  transfer.task.Run();
+  while (!transfer.notifications.empty()) {
+    transfer.notifications.front().Run();
+    transfer.notifications.pop();
+  }
+}
+
+void AsyncPixelTransferDelegateIdle::PerformNotifyCompletion(
+    AsyncMemoryParams mem_params,
+    ScopedSafeSharedMemory* safe_shared_memory,
+    const CompletionCallback& callback) {
+  TRACE_EVENT0("gpu", "PerformNotifyCompletion");
+  gfx::AsyncMemoryParams safe_mem_params = mem_params;
+  safe_mem_params.shared_memory = safe_shared_memory->shared_memory();
+  callback.Run(safe_mem_params);
+}
+
+void AsyncPixelTransferDelegateIdle::PerformAsyncTexImage2D(
+    scoped_refptr<TransferStateInternalIdle> state,
+    AsyncTexImage2DParams tex_params,
+    AsyncMemoryParams mem_params,
+    const base::Closure& bind_callback,
+    ScopedSafeSharedMemory* safe_shared_memory) {
+  TRACE_EVENT2("gpu", "PerformAsyncTexImage2D",
+               "width", tex_params.width,
+               "height", tex_params.height);
+  DCHECK_EQ(0, tex_params.level);
+
+  void* data = GetAddress(safe_shared_memory->shared_memory(),
+                          mem_params.shm_data_offset);
+
+  glActiveTexture(GL_TEXTURE0);
+  glBindTexture(GL_TEXTURE_2D, state->texture_id_);
+  texture_dirty_ = true;
+
+  {
+    TRACE_EVENT0("gpu", "glTexImage2D");
+    glTexImage2D(
+        tex_params.target,
+        tex_params.level,
+        tex_params.internal_format,
+        tex_params.width,
+        tex_params.height,
+        tex_params.border,
+        tex_params.format,
+        tex_params.type,
+        data);
+  }
+
+  state->transfer_in_progress_ = false;
+
+  // The texture is already fully bound so just call it now.
+  bind_callback.Run();

[epenner, 2013/03/13 18:58:01]

Same comment on possible use-after-free when invoking this callback. Let me know
if you have a better design on how we could protect or better indicate this
restriction. Although it's not completely clear, I do think it makes sense not
to call it after AsyncPixelTrasferState is deleted.

[reveman, 2013/03/14 01:12:45]

Done.

+}
+
+void AsyncPixelTransferDelegateIdle::PerformAsyncTexSubImage2D(
+    scoped_refptr<TransferStateInternalIdle> state,
+    AsyncTexSubImage2DParams tex_params,
+    AsyncMemoryParams mem_params,
+    ScopedSafeSharedMemory* safe_shared_memory) {
+  TRACE_EVENT2("gpu", "PerformAsyncTexSubImage2D",
+               "width", tex_params.width,
+               "height", tex_params.height);
+  DCHECK_EQ(0, tex_params.level);
+
+  void* data = GetAddress(safe_shared_memory->shared_memory(),
+                          mem_params.shm_data_offset);
+
+  base::TimeTicks begin_time(base::TimeTicks::HighResNow());
+  glActiveTexture(GL_TEXTURE0);
+  glBindTexture(GL_TEXTURE_2D, state->texture_id_);
+  texture_dirty_ = true;
+
+  {
+    TRACE_EVENT0("gpu", "glTexSubImage2D");
+    glTexSubImage2D(
+        GL_TEXTURE_2D,
+        tex_params.level,
+        tex_params.xoffset,
+        tex_params.yoffset,
+        tex_params.width,
+        tex_params.height,
+        tex_params.format,
+        tex_params.type,
+        data);
+  }
+
+  texture_upload_count_++;
+  total_texture_upload_time_ += base::TimeTicks::HighResNow() - begin_time;
+
+  state->transfer_in_progress_ = false;
+}
+
+}  // namespace gfx