ash: Low-latency laser pointer.

ash/laser/laser_pointer_view.cc

Index: ash/laser/laser_pointer_view.cc
diff --git a/ash/laser/laser_pointer_view.cc b/ash/laser/laser_pointer_view.cc
index 5374ce959c59c69e5a25a006ffb861a9394eb6d0..78e171822b51870318d39453a3e037aa3e29b9e2 100644
--- a/ash/laser/laser_pointer_view.cc
+++ b/ash/laser/laser_pointer_view.cc
@@ -4,17 +4,36 @@
 
 #include "ash/laser/laser_pointer_view.h"
 
+#include <GLES2/gl2.h>
+#include <GLES2/gl2ext.h>
+#include <GLES2/gl2extchromium.h>
+
 #include <memory>
 
 #include "ash/laser/laser_pointer_points.h"
 #include "ash/laser/laser_segment_utils.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/timer/timer.h"
+#include "base/trace_event/trace_event.h"
+#include "cc/output/context_provider.h"
+#include "cc/quads/texture_draw_quad.h"
+#include "cc/resources/transferable_resource.h"
+#include "cc/surfaces/surface.h"
+#include "cc/surfaces/surface_manager.h"
+#include "gpu/command_buffer/client/context_support.h"
+#include "gpu/command_buffer/client/gles2_interface.h"
+#include "gpu/command_buffer/client/gpu_memory_buffer_manager.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "third_party/skia/include/core/SkTypes.h"
+#include "ui/aura/env.h"
 #include "ui/aura/window.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
 #include "ui/events/event.h"
 #include "ui/gfx/canvas.h"
+#include "ui/gfx/gpu_memory_buffer.h"
 #include "ui/views/widget/widget.h"
 
 namespace ash {
@@ -157,10 +176,40 @@ class LaserSegment {
   DISALLOW_COPY_AND_ASSIGN(LaserSegment);
 };
 
+// This struct contains the resources associated with a laser pointer frame.
+struct LaserResource {
+  LaserResource() {}
+  ~LaserResource() {
+    if (context_provider) {
+      gpu::gles2::GLES2Interface* gles2 = context_provider->ContextGL();
+      if (texture)
+        gles2->DeleteTextures(1, &texture);
+      if (image)
+        gles2->DestroyImageCHROMIUM(image);
+    }
+  }
+  scoped_refptr<cc::ContextProvider> context_provider;
+  uint32_t texture = 0;
+  uint32_t image = 0;
+  gpu::Mailbox mailbox;
+};
+
 // LaserPointerView
 LaserPointerView::LaserPointerView(base::TimeDelta life_duration,
                                    aura::Window* root_window)
-    : laser_points_(life_duration) {
+    : laser_points_(life_duration),
+      frame_sink_id_(aura::Env::GetInstance()
+                         ->context_factory_private()
+                         ->AllocateFrameSinkId()),
+      frame_sink_support_(this,
+                          aura::Env::GetInstance()
+                              ->context_factory_private()
+                              ->GetSurfaceManager(),
+                          frame_sink_id_,
+                          false /* is_root */,
+                          true /* handles_frame_sink_id_invalidation */,
+                          true /* needs_sync_points */),
+      weak_ptr_factory_(this) {
   widget_.reset(new views::Widget);
   views::Widget::InitParams params;
   params.type = views::Widget::InitParams::TYPE_WINDOW_FRAMELESS;
@@ -171,53 +220,168 @@ LaserPointerView::LaserPointerView(base::TimeDelta life_duration,
   params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
   params.parent =
       Shell::GetContainer(root_window, kShellWindowId_OverlayContainer);
+  params.layer_type = ui::LAYER_SOLID_COLOR;
 
   widget_->Init(params);
   widget_->Show();
   widget_->SetContentsView(this);
+  widget_->SetBounds(root_window->GetBoundsInScreen());
   set_owned_by_client();
+
+  scale_factor_ = display::Screen::GetScreen()
+                      ->GetDisplayNearestWindow(widget_->GetNativeView())
+                      .device_scale_factor();
 }
 
-LaserPointerView::~LaserPointerView() {}
+LaserPointerView::~LaserPointerView() {
+  // Make sure GPU memory buffer is unmapped before being destroyed.
+  if (gpu_memory_buffer_)
+    gpu_memory_buffer_->Unmap();
+}
 
 void LaserPointerView::Stop() {
+  buffer_damage_rect_.Union(GetBoundingBox());
   laser_points_.Clear();
-  SchedulePaint();
+  OnPointsUpdated();
 }
 
 void LaserPointerView::AddNewPoint(const gfx::Point& new_point) {
+  buffer_damage_rect_.Union(GetBoundingBox());
   laser_points_.AddPoint(new_point);
+  buffer_damage_rect_.Union(GetBoundingBox());
   OnPointsUpdated();
 }
 
 void LaserPointerView::UpdateTime() {
+  buffer_damage_rect_.Union(GetBoundingBox());
   // Do not add the point but advance the time if the view is in process of
   // fading away.
   laser_points_.MoveForwardToTime(base::Time::Now());
+  buffer_damage_rect_.Union(GetBoundingBox());
   OnPointsUpdated();
 }
 
-void LaserPointerView::OnPointsUpdated() {
-  // The bounding box should be relative to the screen.
-  gfx::Point screen_offset =
-      widget_->GetNativeView()->GetRootWindow()->GetBoundsInScreen().origin();
+void LaserPointerView::SetNeedsBeginFrame(bool needs_begin_frame) {
+  frame_sink_support_.SetNeedsBeginFrame(needs_begin_frame);
+}
+
+void LaserPointerView::SubmitCompositorFrame(
+    const cc::LocalSurfaceId& local_surface_id,
+    cc::CompositorFrame frame) {
+  frame_sink_support_.SubmitCompositorFrame(local_surface_id, std::move(frame));
+}
+
+void LaserPointerView::EvictFrame() {
+  frame_sink_support_.EvictFrame();
+}
+
+void LaserPointerView::Require(const cc::LocalSurfaceId& local_surface_id,
+                               const cc::SurfaceSequence& sequence) {
+  frame_sink_support_.Require(local_surface_id, sequence);
+}
 
+void LaserPointerView::Satisfy(const cc::SurfaceSequence& sequence) {
+  frame_sink_support_.Satisfy(sequence);
+}
+
+void LaserPointerView::ReclaimResources(
+    const cc::ReturnedResourceArray& resources) {
+  DCHECK_EQ(resources.size(), 1u);
+
+  auto it = resources_.find(resources.front().id);
+  DCHECK(it != resources_.end());
+  std::unique_ptr<LaserResource> resource = std::move(it->second);
+  resources_.erase(it);
+
+  gpu::gles2::GLES2Interface* gles2 = resource->context_provider->ContextGL();
+  if (resources.front().sync_token.HasData())
+    gles2->WaitSyncTokenCHROMIUM(resources.front().sync_token.GetConstData());
+
+  if (!resources.front().lost)
+    returned_resources_.push_back(std::move(resource));
+}
+
+void LaserPointerView::WillDrawSurface(
+    const cc::LocalSurfaceId& local_surface_id,
+    const gfx::Rect& damage_rect) {

[jbauman, 2017/03/01 01:12:01]

On think this should be triggered by DidReceiveCompositorFrameAck instead.

[reveman, 2017/03/01 18:57:15]

Done. Will throttle frames more aggressively. This is good in the HW overlay
case where each frame end up as a no-op. Might have negative impact when HW
overlays are not supported but I can't tell a difference from testing so let's
start with this.

+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&LaserPointerView::OnDidDrawSurface,
+                            weak_ptr_factory_.GetWeakPtr()));
+  ;
+}
+
+gfx::Rect LaserPointerView::GetBoundingBox() {
   // Expand the bounding box so that it includes the radius of the points on the
-  // edges.
-  gfx::Rect bounding_box;
-  bounding_box = laser_points_.GetBoundingBox();
-  bounding_box.Offset(-kPointInitialRadius, -kPointInitialRadius);
-  bounding_box.Offset(screen_offset.x(), screen_offset.y());
-  bounding_box.set_width(bounding_box.width() + (kPointInitialRadius * 2));
-  bounding_box.set_height(bounding_box.height() + (kPointInitialRadius * 2));
-  widget_->SetBounds(bounding_box);
-  SchedulePaint();
+  // edges and antialiasing.
+  gfx::Rect bounding_box = laser_points_.GetBoundingBox();
+  const int kOutsetForAntialiasing = 1;
+  int outset = kPointInitialRadius + kOutsetForAntialiasing;
+  bounding_box.Inset(-outset, -outset);
+  return bounding_box;
 }
 
-void LaserPointerView::OnPaint(gfx::Canvas* canvas) {
-  if (laser_points_.IsEmpty())
+void LaserPointerView::OnPointsUpdated() {
+  if (pending_update_buffer_)
     return;
 
+  pending_update_buffer_ = true;
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&LaserPointerView::UpdateBuffer,
+                            weak_ptr_factory_.GetWeakPtr()));
+}
+
+void LaserPointerView::UpdateBuffer() {
+  TRACE_EVENT2("ui", "LaserPointerView::UpdatedBuffer", "damage",
+               buffer_damage_rect_.ToString(), "points",
+               laser_points_.GetNumberOfPoints());
+
+  DCHECK(pending_update_buffer_);
+  pending_update_buffer_ = false;
+
+  gfx::Rect screen_bounds = widget_->GetNativeView()->GetBoundsInScreen();
+  gfx::Rect update_rect = buffer_damage_rect_;
+  buffer_damage_rect_ = gfx::Rect();
+
+  // Create and map a single GPU memory buffer. The laser pointer will be
+  // written into this buffer without any buffering. The result is that we
+  // might be modifying the buffer while it's being displayed. This provides
+  // minimal latency but potential tearing. Note that we have to draw into
+  // a temporary surface and copy it into GPU memory buffer to avoid flicker.
+  if (!gpu_memory_buffer_) {
+    gpu_memory_buffer_ =
+        aura::Env::GetInstance()
+            ->context_factory()
+            ->GetGpuMemoryBufferManager()

[jbauman, 2017/03/01 01:12:01]

I think we need to listen to ContextFactoryObserver::OnLostResources to know
when to recreate this.

[reveman, 2017/03/01 18:57:15]

GpuMemoryBuffers survive GPU process crashes. The textures and the GLImage
doesn't but the idea with the current code is that if that happens it's fine
that a stylus up event (will destroy the instance of this class) and another
stylus down is needed to get output again. That keeps the code simple. I added a
comment about this to latest patch.

+            ->CreateGpuMemoryBuffer(
+                gfx::ScaleToCeiledSize(screen_bounds.size(), scale_factor_),
+                SK_B32_SHIFT ? gfx::BufferFormat::RGBA_8888
+                             : gfx::BufferFormat::BGRA_8888,
+                gfx::BufferUsage::SCANOUT_CPU_READ_WRITE,
+                gpu::kNullSurfaceHandle);
+    if (!gpu_memory_buffer_) {
+      LOG(ERROR) << "Failed to allocate GPU memory buffer";
+      return;
+    }
+
+    // Map buffer and keep it mapped until destroyed.
+    bool rv = gpu_memory_buffer_->Map();
+    if (!rv) {
+      LOG(ERROR) << "Failed to map GPU memory buffer";
+      return;
+    }
+
+    // Make sure the first update rectangle covers the whole buffer.
+    update_rect = gfx::Rect(screen_bounds.size());
+  }
+
+  // Constrain update rectangle to buffer size and early out if empty.
+  update_rect.Intersect(gfx::Rect(screen_bounds.size()));
+  if (update_rect.IsEmpty())
+    return;
+
+  // Create a temporary canvas for update rectangle.
+  gfx::Canvas canvas(update_rect.size(), scale_factor_, false);

[jbauman, 2017/03/01 01:12:01]

You might want to have a cache for this to avoid the cost of allocating and
clearing it.

[reveman, 2017/03/01 18:57:15]

I considered that but with a good system allocator it might not make much
difference in allocation cost. Minimizing the clear is tricky as this is already
reduced to only the size that was damaged. Anyhow, there might be room for
improvement here and I added a comment to the code about this in latest patch.

+
   cc::PaintFlags flags;
   flags.setStyle(cc::PaintFlags::kFill_Style);
   flags.setAntiAlias(true);
@@ -228,50 +392,210 @@ void LaserPointerView::OnPaint(gfx::Canvas* canvas) {
       widget_->GetNativeView()->GetBoundsInRootWindow().origin().y());
 
   int num_points = laser_points_.GetNumberOfPoints();
-  DCHECK(num_points > 0);
-  LaserPointerPoints::LaserPoint previous_point = laser_points_.GetOldest();
-  previous_point.location -= widget_offset;
-  LaserPointerPoints::LaserPoint current_point;
-  std::vector<gfx::PointF> previous_segment_points;
-  float previous_radius;
-  int current_opacity;
-
-  for (int i = 0; i < num_points; ++i) {
-    current_point = laser_points_.laser_points()[i];
-    current_point.location -= widget_offset;
-
-    // Set the radius and opacity based on the distance.
-    float current_radius = LinearInterpolate(
-        kPointInitialRadius, kPointFinalRadius, current_point.age);
-    current_opacity = int{LinearInterpolate(
-        kPointInitialOpacity, kPointFinalOpacity, current_point.age)};
-
-    // If we draw laser_points_ that are within a stroke width of each other,
-    // the result will be very jagged, unless we are on the last point, then we
-    // draw regardless.
-    float distance_threshold = current_radius * 2.0f;
-    if (DistanceBetweenPoints(previous_point.location,
-                              current_point.location) <= distance_threshold &&
-        i != num_points - 1) {
-      continue;
+  if (num_points) {
+    LaserPointerPoints::LaserPoint previous_point = laser_points_.GetOldest();
+    previous_point.location -= widget_offset + update_rect.OffsetFromOrigin();

[jbauman, 2017/03/01 01:12:01]

Might be easier to just translate the canvas rather than change all the point
locations.

[reveman, 2017/03/01 18:57:15]

Good idea. I'll keep it as is in current patch to minimize the delta but will
follow up with another patch that improves this.

+    LaserPointerPoints::LaserPoint current_point;
+    std::vector<gfx::PointF> previous_segment_points;
+    float previous_radius;
+    int current_opacity;
+
+    for (int i = 0; i < num_points; ++i) {
+      current_point = laser_points_.laser_points()[i];
+      current_point.location -= widget_offset + update_rect.OffsetFromOrigin();
+
+      // Set the radius and opacity based on the distance.
+      float current_radius = LinearInterpolate(
+          kPointInitialRadius, kPointFinalRadius, current_point.age);
+      current_opacity = int{LinearInterpolate(
+          kPointInitialOpacity, kPointFinalOpacity, current_point.age)};
+
+      // If we draw laser_points_ that are within a stroke width of each other,
+      // the result will be very jagged, unless we are on the last point, then
+      // we draw regardless.
+      float distance_threshold = current_radius * 2.0f;
+      if (DistanceBetweenPoints(previous_point.location,
+                                current_point.location) <= distance_threshold &&
+          i != num_points - 1) {
+        continue;
+      }
+
+      LaserSegment current_segment(
+          previous_segment_points, gfx::PointF(previous_point.location),
+          gfx::PointF(current_point.location), previous_radius, current_radius,
+          i == num_points - 1);
+
+      SkPath path = current_segment.path();
+      flags.setColor(SkColorSetA(kPointColor, current_opacity));
+      canvas.DrawPath(path, flags);
+
+      previous_segment_points = current_segment.path_points();
+      previous_radius = current_radius;
+      previous_point = current_point;
     }
 
-    LaserSegment current_segment(
-        previous_segment_points, gfx::PointF(previous_point.location),
-        gfx::PointF(current_point.location), previous_radius, current_radius,
-        i == num_points - 1);
-
-    SkPath path = current_segment.path();
+    // Draw the last point as a circle.
     flags.setColor(SkColorSetA(kPointColor, current_opacity));
-    canvas->DrawPath(path, flags);
+    flags.setStyle(cc::PaintFlags::kFill_Style);
+    canvas.DrawCircle(current_point.location, kPointInitialRadius, flags);
+  }
 
-    previous_segment_points = current_segment.path_points();
-    previous_radius = current_radius;
-    previous_point = current_point;
+  // Copy result to GPU memory buffer. This is effectiely a memcpy and unlike
+  // drawing to the buffer directly this ensures that the buffer is never in a
+  // state that would result in flicker.
+  {
+    TRACE_EVENT0("ui", "LaserPointerView::OnPointsUpdated::Copy");
+
+    // Convert update rectangle to pixel coordinates.
+    gfx::Rect pixel_rect =
+        gfx::ScaleToEnclosingRect(update_rect, scale_factor_);
+    uint8_t* data = static_cast<uint8_t*>(gpu_memory_buffer_->memory(0));
+    int stride = gpu_memory_buffer_->stride(0);
+    canvas.sk_canvas()->readPixels(
+        SkImageInfo::MakeN32Premul(pixel_rect.width(), pixel_rect.height()),
+        data + pixel_rect.y() * stride + pixel_rect.x() * 4, stride, 0, 0);
   }
-  // Draw the last point as a circle.
-  flags.setColor(SkColorSetA(kPointColor, current_opacity));
-  flags.setStyle(cc::PaintFlags::kFill_Style);
-  canvas->DrawCircle(current_point.location, kPointInitialRadius, flags);
+
+  // Update surface damage rectangle.
+  surface_damage_rect_.Union(update_rect);
+
+  needs_update_surface_ = true;
+
+  // Early out if waiting for last surface update to be drawn.
+  if (pending_draw_surface_)
+    return;
+
+  UpdateSurface();
+}
+
+void LaserPointerView::UpdateSurface() {

[jbauman, 2017/03/01 01:12:01]

We'd normally want to trigger this from OnBeginFrame instead so it'd be
rate-limited. I don't know if that would hurt latency too much, though.

[reveman, 2017/03/01 18:57:15]

That's another good follow up that I also considered including in this patch but
decided to save for later to keep the change small.

+  TRACE_EVENT1("ui", "LaserPointerView::UpdatedSurface", "damage",
+               surface_damage_rect_.ToString());
+
+  DCHECK(needs_update_surface_);
+  needs_update_surface_ = false;
+
+  std::unique_ptr<LaserResource> resource;
+  // Reuse returned resource if available.
+  if (!returned_resources_.empty()) {
+    resource = std::move(returned_resources_.front());
+    returned_resources_.pop_front();
+  }
+
+  // Create new resource if needed.
+  if (!resource)
+    resource = base::MakeUnique<LaserResource>();
+
+  // Acquire context provider for resource if needed.
+  if (!resource->context_provider) {

[jbauman, 2017/03/01 01:12:01]

Check if ContextGL()->GetGraphicsResetStatusKHR() is GL_NO_ERROR and if not
throw this resource away and create a new one.

[reveman, 2017/03/01 18:57:15]

As I mentioned in the other comment. We don't worry about this as this class is
considered short-lived. Added a comment about this here.

+    resource->context_provider = aura::Env::GetInstance()
+                                     ->context_factory()
+                                     ->SharedMainThreadContextProvider();
+    if (!resource->context_provider) {
+      LOG(ERROR) << "Failed to acquire a context provider";
+      return;
+    }
+  }
+
+  gpu::gles2::GLES2Interface* gles2 = resource->context_provider->ContextGL();
+
+  if (resource->texture) {
+    gles2->ActiveTexture(GL_TEXTURE0);
+    gles2->BindTexture(GL_TEXTURE_2D, resource->texture);
+  } else {
+    gles2->GenTextures(1, &resource->texture);
+    gles2->ActiveTexture(GL_TEXTURE0);
+    gles2->BindTexture(GL_TEXTURE_2D, resource->texture);
+    gles2->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    gles2->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    gles2->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    gles2->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    gles2->GenMailboxCHROMIUM(resource->mailbox.name);
+    gles2->ProduceTextureCHROMIUM(GL_TEXTURE_2D, resource->mailbox.name);
+  }
+
+  gfx::Size buffer_size = gpu_memory_buffer_->GetSize();
+
+  if (resource->image) {
+    gles2->ReleaseTexImage2DCHROMIUM(GL_TEXTURE_2D, resource->image);
+  } else {
+    resource->image = gles2->CreateImageCHROMIUM(
+        gpu_memory_buffer_->AsClientBuffer(), buffer_size.width(),
+        buffer_size.height(), SK_B32_SHIFT ? GL_RGBA : GL_BGRA_EXT);
+    if (!resource->image) {
+      LOG(ERROR) << "Failed to create image";
+      return;
+    }
+  }
+  gles2->BindTexImage2DCHROMIUM(GL_TEXTURE_2D, resource->image);
+
+  gpu::SyncToken sync_token;
+  uint64_t fence_sync = gles2->InsertFenceSyncCHROMIUM();
+  gles2->OrderingBarrierCHROMIUM();
+  gles2->GenUnverifiedSyncTokenCHROMIUM(fence_sync, sync_token.GetData());
+
+  cc::TransferableResource transferable_resource;
+  transferable_resource.id = next_resource_id_++;
+  transferable_resource.format = cc::RGBA_8888;
+  transferable_resource.filter = GL_LINEAR;
+  transferable_resource.size = buffer_size;
+  transferable_resource.mailbox_holder =
+      gpu::MailboxHolder(resource->mailbox, sync_token, GL_TEXTURE_2D);
+  transferable_resource.is_overlay_candidate = true;
+
+  gfx::Rect quad_rect(widget_->GetNativeView()->GetBoundsInScreen().size());
+
+  const int kRenderPassId = 1;
+  std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create();
+  render_pass->SetNew(kRenderPassId, quad_rect, surface_damage_rect_,
+                      gfx::Transform());
+  surface_damage_rect_ = gfx::Rect();
+
+  cc::SharedQuadState* quad_state =
+      render_pass->CreateAndAppendSharedQuadState();
+  quad_state->quad_layer_bounds = quad_rect.size();
+  quad_state->visible_quad_layer_rect = quad_rect;
+  quad_state->opacity = 1.0f;
+
+  cc::CompositorFrame frame;
+  cc::TextureDrawQuad* texture_quad =
+      render_pass->CreateAndAppendDrawQuad<cc::TextureDrawQuad>();
+  float vertex_opacity[4] = {1.0, 1.0, 1.0, 1.0};
+  gfx::PointF uv_top_left(0.f, 0.f);
+  gfx::PointF uv_bottom_right(1.f, 1.f);
+  texture_quad->SetNew(quad_state, quad_rect, gfx::Rect(), quad_rect,
+                       transferable_resource.id, true, uv_top_left,
+                       uv_bottom_right, SK_ColorTRANSPARENT, vertex_opacity,
+                       false, false, false);
+  texture_quad->set_resource_size_in_pixels(transferable_resource.size);
+  frame.resource_list.push_back(transferable_resource);
+  frame.render_pass_list.push_back(std::move(render_pass));
+
+  // Set layer surface if this is the initial frame.
+  if (!local_surface_id_.is_valid()) {
+    local_surface_id_ = id_allocator_.GenerateId();
+    widget_->GetNativeView()->layer()->SetShowSurface(
+        cc::SurfaceInfo(cc::SurfaceId(frame_sink_id_, local_surface_id_), 1.0f,
+                        quad_rect.size()),
+        aura::Env::GetInstance()
+            ->context_factory_private()
+            ->GetSurfaceManager()
+            ->reference_factory());
+    widget_->GetNativeView()->layer()->SetFillsBoundsOpaquely(false);
+  }
+
+  SubmitCompositorFrame(local_surface_id_, std::move(frame));
+
+  resources_[transferable_resource.id] = std::move(resource);
+
+  DCHECK(!pending_draw_surface_);
+  pending_draw_surface_ = true;
+}
+
+void LaserPointerView::OnDidDrawSurface() {
+  pending_draw_surface_ = false;
+  if (needs_update_surface_)
+    UpdateSurface();
 }
+
 }  // namespace ash