cc: Move draw params from SetExternalDrawConstraints to OnDraw

cc/trees/layer_tree_host_impl.cc

Index: cc/trees/layer_tree_host_impl.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index 0590b357569574b48df9fc8ee1c17b4d828fd70f..f3b2e65595cdccd5af198a38e6005608d4f1d39d 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -9,6 +9,7 @@
 #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"
@@ -1227,6 +1228,17 @@ size_t LayerTreeHostImpl::SourceAnimationFrameNumberForTesting() const {
   return fps_counter_->current_frame_number();
 }
 
+void LayerTreeHostImpl::SetExternalViewporForTesting(

[danakj, 2015/11/24 21:17:31]

Why aren't callers of this just using SetExternalDrawConstraints?

[boliu, 2015/11/24 23:27:38]

external_viewport_ is set in OnDraw now, so this is needed for tests that calls
CalcDrawProps directly

[danakj, 2015/12/10 22:33:54]

If you call CalcDrawProps you can pass whatever viewport you want, no?

+    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_)
@@ -1424,59 +1436,29 @@ void LayerTreeHostImpl::SetManagedMemoryPolicy(
 }
 
 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) {
+    const gfx::Transform& transform_for_tile_priority) {
   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_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);
   }
 
-  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();
     SetFullRootLayerDamage();

[danakj, 2015/11/24 21:17:31]

Can you explain why this and SetNeedsRedraw() still belong in here? Maybe with a
comment too?

[boliu, 2015/11/24 23:27:38]

Essentially this replaces SetNeedsRedrawRect from SyncOutputSurface.

     SetNeedsRedraw();
   }
-
-  if (resourceless_software_draw_changed) {
-    client_->OnResourcelessSoftareDrawStateChanged(resourceless_software_draw);
-    client_->OnCanDrawStateChanged(CanDraw());
-  }
 }
 
 void LayerTreeHostImpl::SetNeedsRedrawRect(const gfx::Rect& damage_rect) {
@@ -1526,8 +1508,53 @@ void LayerTreeHostImpl::ReclaimResources(const CompositorFrameAck* ack) {
   }
 }
 
-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 all changes to draw

[danakj, 2015/11/24 21:17:30]

all changes => any changes

[boliu, 2015/11/24 23:27:38]

Done.

+    // 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) {
+    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() {