cc: Move draw params from SetExternalDrawConstraints to OnDraw

cc/trees/layer_tree_host_impl.cc

 // Copyright 2011 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/trees/layer_tree_host_impl.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <set>
 
+#include "base/auto_reset.h"
 #include "base/basictypes.h"
 #include "base/containers/hash_tables.h"
 #include "base/containers/small_map.h"
 #include "base/json/json_writer.h"
 #include "base/metrics/histogram.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "cc/animation/animation_host.h"
@@ skipping 1228 matching lines @@
   pending_tree_ = nullptr;
   if (recycle_tree_)
     recycle_tree_->DetachLayerTree();
   recycle_tree_ = nullptr;
 }
 
 size_t LayerTreeHostImpl::SourceAnimationFrameNumberForTesting() const {
   return fps_counter_->current_frame_number();
 }
 
+void LayerTreeHostImpl::SetExternalViewportForTesting(
+    const gfx::Rect& external_viewport) {
+  external_viewport_ = external_viewport;
+}
+
+void LayerTreeHostImpl::SetResourcelessSoftwareDrawForTesting(
+    bool resourceless_software_draw) {
+  resourceless_software_draw_ = resourceless_software_draw;
+  active_tree_->set_needs_update_draw_properties();
+}
+
 void LayerTreeHostImpl::UpdateTileManagerMemoryPolicy(
     const ManagedMemoryPolicy& policy) {
   if (!resource_pool_)
     return;
 
   global_tile_state_.hard_memory_limit_in_bytes = 0;
   global_tile_state_.soft_memory_limit_in_bytes = 0;
   if (visible_ && policy.bytes_limit_when_visible > 0) {
     global_tile_state_.hard_memory_limit_in_bytes =
         policy.bytes_limit_when_visible;
@@ skipping 176 matching lines @@
       old_policy.bytes_limit_when_visible >= max_memory_needed_bytes_ &&
       actual_policy.priority_cutoff_when_visible ==
           old_policy.priority_cutoff_when_visible) {
     needs_commit = false;
   }
 
   if (needs_commit)
     client_->SetNeedsCommitOnImplThread();
 }
 
-void LayerTreeHostImpl::SetExternalDrawConstraints(
-    const gfx::Transform& transform,
-    const gfx::Rect& viewport,
-    const gfx::Rect& clip,
-    const gfx::Rect& viewport_rect_for_tile_priority,
-    const gfx::Transform& transform_for_tile_priority,
-    bool resourceless_software_draw) {
+void LayerTreeHostImpl::SetExternalTilePriorityConstraints(
+    const gfx::Rect& viewport_rect,
+    const gfx::Transform& transform) {

[brianderson, 2015/12/14 20:51:26]

Would it make sense to DCHECK(using_synchronous_renderer_compositor) here? (And
in LayerTreeHostImpl::OnDraw.)

[boliu, 2015/12/16 19:06:41]

This works in production, and so probably good idea long term. But this is
causing a bunch of tests to fail. In theory these don't require
using_synchronous_renderer_compositor, since a bunch of tests has its own
LTHIClient implementations. This DCHECK already exists for
Scheduler::OnDrawForOutputSurface so at least tests using real scheduler is ok.
This patch is already pretty big as it is, so didn't change this.

   gfx::Rect viewport_rect_for_tile_priority_in_view_space;
-  if (!resourceless_software_draw) {
-    gfx::Transform screen_to_view(gfx::Transform::kSkipInitialization);
-    if (transform_for_tile_priority.GetInverse(&screen_to_view)) {
-      // Convert from screen space to view space.
-      viewport_rect_for_tile_priority_in_view_space =
-          MathUtil::ProjectEnclosingClippedRect(
-              screen_to_view, viewport_rect_for_tile_priority);
-    }
+  gfx::Transform screen_to_view(gfx::Transform::kSkipInitialization);
+  if (transform.GetInverse(&screen_to_view)) {
+    // Convert from screen space to view space.
+    viewport_rect_for_tile_priority_in_view_space =
+        MathUtil::ProjectEnclosingClippedRect(screen_to_view, viewport_rect);
   }
 
-  const bool transform_changed = external_transform_ != transform;
-  const bool viewport_changed = external_viewport_ != viewport;
-  const bool clip_changed = external_clip_ != clip;
-  const bool resourceless_software_draw_changed =
-      resourceless_software_draw_ != resourceless_software_draw;
   const bool tile_priority_params_changed =
       viewport_rect_for_tile_priority_ !=
       viewport_rect_for_tile_priority_in_view_space;
 
-  // UpdateDrawProperties does not depend on clip.
-  if (transform_changed || viewport_changed ||
-      resourceless_software_draw_changed || tile_priority_params_changed) {
-    active_tree_->set_needs_update_draw_properties();
-  }
-
-  external_transform_ = transform;
-  external_viewport_ = viewport;
-  external_clip_ = clip;
   viewport_rect_for_tile_priority_ =
       viewport_rect_for_tile_priority_in_view_space;
-  resourceless_software_draw_ = resourceless_software_draw;
 
-  // When not toggling resourceless software draw, need to set redraw for
-  // all changes to draw parameters. Damage will be set externally by Android
-  // WebView for resourceless software draw toggles, so ignored here.
-  const bool draw_params_changed = transform_changed || viewport_changed ||
-                                   clip_changed || tile_priority_params_changed;
-  if (!resourceless_software_draw_changed && draw_params_changed) {
+  if (tile_priority_params_changed) {
+    active_tree_->set_needs_update_draw_properties();
+
+    // Compositor, not OutputSurface, is responsible for setting damage and
+    // triggering redraw for constraint changes.
     SetFullRootLayerDamage();
     SetNeedsRedraw();
   }
-
-  if (resourceless_software_draw_changed) {
-    client_->OnResourcelessSoftareDrawStateChanged(resourceless_software_draw);
-    client_->OnCanDrawStateChanged(CanDraw());
-  }
 }
 
 void LayerTreeHostImpl::SetNeedsRedrawRect(const gfx::Rect& damage_rect) {
   if (damage_rect.IsEmpty())
     return;
   NotifySwapPromiseMonitorsOfSetNeedsRedraw();
   client_->SetNeedsRedrawRectOnImplThread(damage_rect);
 }
 
 void LayerTreeHostImpl::DidSwapBuffers() {
@@ skipping 29 matching lines @@
   }
 
   // If we're not visible, we likely released resources, so we want to
   // aggressively flush here to make sure those DeleteTextures make it to the
   // GPU process to free up the memory.
   if (output_surface_->context_provider() && !visible_) {
     output_surface_->context_provider()->ContextGL()->ShallowFlushCHROMIUM();
   }
 }
 
-void LayerTreeHostImpl::OnDraw() {
-  client_->OnDrawForOutputSurface();
+void LayerTreeHostImpl::OnDraw(const gfx::Transform& transform,
+                               const gfx::Rect& viewport,
+                               const gfx::Rect& clip,
+                               bool resourceless_software_draw) {
+  DCHECK(!resourceless_software_draw_);
+  const bool transform_changed = external_transform_ != transform;
+  const bool viewport_changed = external_viewport_ != viewport;
+  const bool clip_changed = external_clip_ != clip;
+
+  external_transform_ = transform;
+  external_viewport_ = viewport;
+  external_clip_ = clip;
+
+  {
+    base::AutoReset<bool> resourceless_software_draw_reset(
+        &resourceless_software_draw_, resourceless_software_draw);
+
+    // For resourceless software draw, always set full damage to ensure they
+    // always swap. Otherwise, need to set redraw for any changes to draw
+    // parameters.
+    const bool draw_params_changed =
+        transform_changed || viewport_changed || clip_changed;
+    if (resourceless_software_draw_ || draw_params_changed) {
+      SetFullRootLayerDamage();
+      SetNeedsRedraw();
+    }
+
+    // UpdateDrawProperties does not depend on clip.
+    if (transform_changed || viewport_changed || resourceless_software_draw_) {
+      active_tree_->set_needs_update_draw_properties();
+    }
+
+    if (resourceless_software_draw) {
+      client_->OnCanDrawStateChanged(CanDraw());
+    }
+
+    client_->OnDrawForOutputSurface(resourceless_software_draw_);
+  }
+
+  if (resourceless_software_draw) {

[brianderson, 2015/12/14 20:51:26]

I think it would be easier to understand the flow of this method if there was a
higher level if(resourceless_software_draw) {} else {} block. wdyt?

[boliu, 2015/12/16 19:06:41]

Tried this, result below in this comment. Really dislike duplicating a few
lines, so didn't go with this.

void LayerTreeHostImpl::OnDraw(const gfx::Transform& transform,
                               const gfx::Rect& viewport,
                               const gfx::Rect& clip,
                               bool resourceless_software_draw) {
  DCHECK(!resourceless_software_draw_);
  const bool transform_changed = external_transform_ != transform;
  const bool viewport_changed = external_viewport_ != viewport;
  const bool clip_changed = external_clip_ != clip;

  external_transform_ = transform;
  external_viewport_ = viewport;
  external_clip_ = clip;

  if (!resourceless_software_draw) {
    // Set damage viewport if draw parameters changed.
    if (transform_changed || viewport_changed || clip_changed) {
      SetFullRootLayerDamage();
      SetNeedsRedraw();
    }

    // UpdateDrawProperties does not depend on clip.
    if (transform_changed || viewport_changed) {
      active_tree_->set_needs_update_draw_properties();
    }

    client_->OnDrawForOutputSurface(resourceless_software_draw_);
    return;
  }

  {
    base::AutoReset<bool> resourceless_software_draw_reset(
        &resourceless_software_draw_, resourceless_software_draw);

    // For resourceless software draw, always set full damage to ensure draw
    // always swap.
    SetFullRootLayerDamage();
    SetNeedsRedraw();
    active_tree_->set_needs_update_draw_properties();
    client_->OnCanDrawStateChanged(CanDraw());

    client_->OnDrawForOutputSurface(resourceless_software_draw_);
  }

  active_tree_->set_needs_update_draw_properties();
  client_->OnCanDrawStateChanged(CanDraw());
  // This draw may have reset all damage, which would lead to subsequent
  // incorrect hardware draw, so explicitly set damage for next hardware
  // draw as well.
  SetFullRootLayerDamage();
}

+    active_tree_->set_needs_update_draw_properties();
+    client_->OnCanDrawStateChanged(CanDraw());
+    // This draw may have reset all damage, which would lead to subsequent
+    // incorrect hardware draw, so explicitly set damage for next hardware
+    // draw as well.
+    SetFullRootLayerDamage();
+  }
 }
 
 void LayerTreeHostImpl::OnCanDrawStateChangedForTree() {
   client_->OnCanDrawStateChanged(CanDraw());
 }
 
 CompositorFrameMetadata LayerTreeHostImpl::MakeCompositorFrameMetadata() const {
   CompositorFrameMetadata metadata;
   metadata.device_scale_factor = active_tree_->painted_device_scale_factor() *
                                  active_tree_->device_scale_factor();
@@ skipping 2186 matching lines @@
   return task_runner_provider_->HasImplThread();
 }
 
 bool LayerTreeHostImpl::CommitToActiveTree() const {
   // In single threaded mode we skip the pending tree and commit directly to the
   // active tree.
   return !task_runner_provider_->HasImplThread();
 }
 
 }  // namespace cc