cc: Remove Display::SetExternalViewport.

content/renderer/android/synchronous_compositor_frame_sink.cc

Index: content/renderer/android/synchronous_compositor_frame_sink.cc
diff --git a/content/renderer/android/synchronous_compositor_frame_sink.cc b/content/renderer/android/synchronous_compositor_frame_sink.cc
index 3ba0cf799f49e73070c197c8a1c2952feb08871a..70202ca7895fd132acf9030ea51238ee3a2d260b 100644
--- a/content/renderer/android/synchronous_compositor_frame_sink.cc
+++ b/content/renderer/android/synchronous_compositor_frame_sink.cc
@@ -86,7 +86,7 @@ class SynchronousCompositorFrameSink::SoftwareOutputSurface
                float scale_factor,
                const gfx::ColorSpace& color_space,
                bool has_alpha) override {
-    // Intentional no-op. Surface size controlled by embedder.
+    surface_size_ = size;
   }
   uint32_t GetFramebufferCopyTextureFormat() override { return 0; }
   cc::OverlayCandidateValidator* GetOverlayCandidateValidator() const override {
@@ -97,10 +97,6 @@ class SynchronousCompositorFrameSink::SoftwareOutputSurface
   bool SurfaceIsSuspendForRecycle() const override { return false; }
   bool HasExternalStencilTest() const override { return false; }
   void ApplyExternalStencil() override {}
-
-  void SetSurfaceSize(const gfx::Size surface_size) {
-    surface_size_ = surface_size;
-  }
 };
 
 SynchronousCompositorFrameSink::SynchronousCompositorFrameSink(
@@ -173,10 +169,9 @@ bool SynchronousCompositorFrameSink::BindToClient(
 
   cc::RendererSettings software_renderer_settings;
 
-  std::unique_ptr<SoftwareOutputSurface> compositor_frame_sink(
-      new SoftwareOutputSurface(
-          base::MakeUnique<SoftwareDevice>(&current_sw_canvas_)));
-  software_compositor_frame_sink_ = compositor_frame_sink.get();
+  auto output_surface = base::MakeUnique<SoftwareOutputSurface>(
+      base::MakeUnique<SoftwareDevice>(&current_sw_canvas_));
+  software_output_surface_ = output_surface.get();
 
   // The shared_bitmap_manager and gpu_memory_buffer_manager here are null as
   // this Display is only used for resourcesless software draws, where no
@@ -185,7 +180,7 @@ bool SynchronousCompositorFrameSink::BindToClient(
   display_.reset(new cc::Display(
       nullptr /* shared_bitmap_manager */,
       nullptr /* gpu_memory_buffer_manager */, software_renderer_settings,
-      nullptr /* begin_frame_source */, std::move(compositor_frame_sink),
+      nullptr /* begin_frame_source */, std::move(output_surface),
       nullptr /* scheduler */, nullptr /* texture_mailbox_deleter */));
   display_->Initialize(&display_client_, surface_manager_.get(),
                        surface_id_allocator_->client_id());
@@ -207,7 +202,7 @@ void SynchronousCompositorFrameSink::DetachFromClient() {
       surface_id_allocator_->client_id());
   surface_manager_->InvalidateSurfaceClientId(
       surface_id_allocator_->client_id());
-  software_compositor_frame_sink_ = nullptr;
+  software_output_surface_ = nullptr;
   display_ = nullptr;
   surface_factory_ = nullptr;
   surface_id_allocator_ = nullptr;
@@ -245,9 +240,12 @@ void SynchronousCompositorFrameSink::SwapBuffers(cc::CompositorFrame frame) {
     display_->SetSurfaceId(delegated_surface_id_,
                            frame.metadata.device_scale_factor);
 
-    gfx::Size frame_size =
-        frame.delegated_frame_data->render_pass_list.back()->output_rect.size();
-    display_->Resize(frame_size);
+    // This size covers the entire external clip given to DemandDrawSw(). The
+    // CompositorFrame here could be either smaller than this size (we're
+    // drawing an enlarged viewport that extends beyond the layer compositor's
+    // concept of the output size) or larger than this size (we're only drawing
+    // a portion of the layer compositor's usual output size).
+    display_->Resize(sw_display_size_for_current_draw_);
 
     surface_factory_->SubmitCompositorFrame(
         delegated_surface_id_, std::move(frame), base::Bind(&NoOpDrawCallback));
@@ -326,20 +324,22 @@ void SynchronousCompositorFrameSink::DemandDrawSw(SkCanvas* canvas) {
   transform.matrix() = canvas->getTotalMatrix();  // Converts 3x3 matrix to 4x4.
 
   base::AutoReset<bool> set_in_software_draw(&in_software_draw_, true);
-  display_->SetExternalViewport(viewport);
+
+  // We will resize the Display to ensure it draws the entire |viewport| given
+  // here instead of only the layer compositor's idea of what the viewport is.
+  sw_display_size_for_current_draw_ =
+      gfx::Size(viewport.right(), viewport.bottom());
+  // Though the canvas already has a clip on it the Display can destroy it so we
+  // need to tell it about the clip.
   display_->SetExternalClip(viewport);
-  software_compositor_frame_sink_->SetSurfaceSize(
-      gfx::SkISizeToSize(canvas->getBaseLayerSize()));
   InvokeComposite(transform, viewport);
 }
 
 void SynchronousCompositorFrameSink::InvokeComposite(
     const gfx::Transform& transform,
     const gfx::Rect& viewport) {
-  gfx::Transform adjusted_transform = transform;
-  adjusted_transform.matrix().postTranslate(-viewport.x(), -viewport.y(), 0);
   did_swap_ = false;
-  client_->OnDraw(adjusted_transform, viewport, in_software_draw_);

[danakj, 2016/09/30 18:58:24]

Or it's probably related to this change..

[danakj, 2016/09/30 19:10:23]

So LTHI::UpdateDrawProperties would use this transform to figure out where to
draw things. You said it includes the viewport shift. So before what would
happen..

Say your viewport is 5,5,100,100. And you're drawing a dot at 10,10.

The transform would be +5,+5. We would adjust it by the viewport to make it
identity. Now LTHI draws with identity putting the dot at 10,10 and clipping
around 5,5,100,100. So its world origin is at 0,0 in the canvas.

After this change, the transform stays +5,+5. So we draw the dot at 15,15. But
we're clipping 5,5,100,100, so bad things occur and we clip the bottom 5px in
LTHI.


I'm trying to explain to myself the relationship of the Display tho now that
it's viewport origin is 0,0 instead of 5,5.

+  client_->OnDraw(transform, viewport, in_software_draw_);
 
   if (did_swap_) {
     // This must happen after unwinding the stack and leaving the compositor.