cc: Don't draw and swap if the frame will not change.

cc: Don't draw and swap if the frame will not change.

When there is no visible damage in the root surface, we have no
reason to do CalculateRenderPasses, DrawLayers, or SwapBuffers.

This adds an early out in each of these when there is no damage,
but storing a flag on the FrameData to communicate this state.

When doing a readback, we need to make sure we draw the area
being read back, so we pass a damage rect in to PrepareToDraw
to enforce this. This mechanism can be used to also implement
the "ForceFullFrameDamage" mechanism, so we move the flag to
LayerTreeHostImpl and have DamageTracker take a general rect
instead.

Before:
[ RUN      ] LayerTreeHostPerfTestJsonReader.TenTenSingleThread
*RESULT 10_10_layer_tree: frames= 3089.37 runs/s

After:
[ RUN      ] LayerTreeHostPerfTestJsonReader.TenTenSingleThread
*RESULT 10_10_layer_tree: frames= 4679.13 runs/s

When there's no damage, a full single-threaded commit+composite speeds
up about 50%.

Tests:
DamageTrackerTest.DamageWhenAddedExternally
LayerTreeHostDamageTestNoDamageDoesNotSwap.RunSingleThread
LayerTreeHostDamageTestNoDamageDoesNotSwap.RunMultiThread
LayerTreeHostDamageTestNoDamageReadbackDoesDraw.RunSingleThread
LayerTreeHostDamageTestNoDamageReadbackDoesDraw.RunMultiThread
LayerTreeHostDamageTestForcedFullDamage.RunSingleThread
LayerTreeHostDamageTestForcedFullDamage.RunMultiThread

Adding a new perf test with damage:
LayerTreeHostPerfTestJsonReader.TenTenSingleThread_FullDamageEachFrame
*RESULT 10_10_layer_tree: frames= 3233.98 runs/s


R=nduca
BUG=222915
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192706

[danakj, 2013-03-22 02:33:45]

Bots show up red regardless of their state at the moment :|

[piman, 2013-03-22 02:39:17]

What about CompositeAndReadback? Assuming the previous swap was non-preserving.

[danakj, 2013-03-22 02:42:12]

On 2013/03/22 02:39:17, piman wrote:
> What about CompositeAndReadback? Assuming the previous swap was
non-preserving.

If you do nothing but readback, won't you just read whatever you swapped last
time into the frame buffer?

What do you mean by non-preserving?

[piman, 2013-03-22 02:51:33]

On Thu, Mar 21, 2013 at 7:42 PM, <danakj@chromium.org> wrote:

> On 2013/03/22 02:39:17, piman wrote:
>
>> What about CompositeAndReadback? Assuming the previous swap was
>>
> non-preserving.
>
> If you do nothing but readback, won't you just read whatever you swapped
> last
> time into the frame buffer?
>
> What do you mean by non-preserving?
>

Platform swap (traditionally) flips the backbuffer and the frontbuffer.
I.e. after swap, backbuffer pixels are undefined. On EGL you can specify
the "preserve" bit, but that typically causes an extra copy, and we don't
do it. Without it there's no guarantee.
Take as well the MailboxOutputSurface, it's non-preserving.


>
>
https://codereview.chromium.**org/12662021/<https://codereview.chromium.org/1...
>

[danakj, 2013-03-22 02:56:19]

On Thu, Mar 21, 2013 at 10:51 PM, Antoine Labour <piman@chromium.org> wrote:

> On Thu, Mar 21, 2013 at 7:42 PM, <danakj@chromium.org> wrote:
>
>> On 2013/03/22 02:39:17, piman wrote:
>>
>>> What about CompositeAndReadback? Assuming the previous swap was
>>>
>> non-preserving.
>>
>> If you do nothing but readback, won't you just read whatever you swapped
>> last
>> time into the frame buffer?
>>
>> What do you mean by non-preserving?
>>
>
> Platform swap (traditionally) flips the backbuffer and the frontbuffer.
> I.e. after swap, backbuffer pixels are undefined. On EGL you can specify
> the "preserve" bit, but that typically causes an extra copy, and we don't
> do it. Without it there's no guarantee.
> Take as well the MailboxOutputSurface, it's non-preserving.
>

Oh, we're going to readback from the backbuffer.. somehow I was thinking
we'd read the frontbuffer. Thanks! I'll fix and add a test tomorrow.


>
>
>>
>>
https://codereview.chromium.**org/12662021/<https://codereview.chromium.org/1...
>>
>
>

[danakj, 2013-03-22 19:02:01]

+shawnsingh

[danakj, 2013-03-22 19:02:10]

+shawnsingh for real

[danakj, 2013-03-22 19:02:24]

I've added support for readback with tests. PTAL

[nduca, 2013-03-23 01:02:53]

(as a followup, would be good to add a synthetic example of this to
tools/perf/page_sets/tough_animation_cases that would be smooth with this change
and horrible without it...)

[danakj, 2013-03-23 01:06:20]

On Fri, Mar 22, 2013 at 9:02 PM, <nduca@chromium.org> wrote:

> (as a followup, would be good to add a synthetic example of this to
> tools/perf/page_sets/tough_**animation_cases that would be smooth with
> this change
> and horrible without it...)
>

Hm, not sure what that would look like. You'd need animating thing on the
screen in order to be "smooth". Do you have an idea what such an example
would look like?

Btw, offscreen things still cause commits to be happening constantly at
100% cpu as well, this is only part of the picture. Not sure if that
affects "smoothness" though, depending what you mean to measure.


>
>
https://codereview.chromium.**org/12662021/<https://codereview.chromium.org/1...
>

[shawnsingh, 2013-03-23 01:08:14]

Looks great for damage tracker changes.  It does seem like a possibly cleaner
concept to "force damage" outside of the damage tracker logic.

[nduca, 2013-03-23 01:32:20]

> Hm, not sure what that would look like. You'd need animating thing on the
> screen in order to be "smooth". Do you have an idea what such an example
> would look like?

Not sure. Just reminding folks on the thread that we should be thinking about
perf bot coverage when it makes sense. I agree that in this case its awkward
because success is a frame rate of zero, which is le-awk.

[danakj, 2013-03-23 03:22:57]

On 2013/03/23 01:32:20, nduca wrote:
> > Hm, not sure what that would look like. You'd need animating thing on the
> > screen in order to be "smooth". Do you have an idea what such an example
> > would look like?
> 
> Not sure. Just reminding folks on the thread that we should be thinking about
> perf bot coverage when it makes sense. I agree that in this case its awkward
> because success is a frame rate of zero, which is le-awk.

Not that they run on bots yet, but I should see what this does to cc_perftests.
It probably does some crazy boss type improvements.

But should also add a test that damages every frame so we can measure improving
raf there.

[nduca, 2013-03-23 03:51:35]

Something to keep on our radar. This might be fine as is since its a pretty
clear pass/fail situation. Sitatuion X should not draw. Done. :) Maybe just keep
this general topic in mind as we do stuff perf related now that we do have the
capability. :)

[danakj, 2013-03-26 01:23:34]

Here's some perftest results.

Before: 
[ RUN ] LayerTreeHostPerfTestJsonReader.TenTenSingleThread 
*RESULT 10_10_layer_tree: frames= 3089.37 runs/s 

After: 
[ RUN ] LayerTreeHostPerfTestJsonReader.TenTenSingleThread 
*RESULT 10_10_layer_tree: frames= 4679.13 runs/s 

When there's no damage, a full single-threaded commit+composite speeds 
up about 50%. 

I added a new perf test that damages and draws every frame so we can measure
CalcDrawProps and CalcRenderPasses inprovements still:
LayerTreeHostPerfTestJsonReader.TenTenSingleThread_FullDamageEachFrame

[danakj, 2013-03-26 01:24:23]

Also rebased this on GTFO changes. PTAL.

+enne

[enne (OOO), 2013-03-26 01:42:07]

In general, this is great!

nduca: Please correct me if I'm wrong, but I thought there were
scheduler-related issues in the past where if CanDraw() was true then any
request to draw needed to swap or badness would ensue.  Am I misremembering or
is that no longer the case or was that only the case for readback and this patch
addresses that?

https://codereview.chromium.org/12662021/diff/26001/cc/test/layer_tree_test.cc
File cc/test/layer_tree_test.cc (right):

https://codereview.chromium.org/12662021/diff/26001/cc/test/layer_tree_test.c...
cc/test/layer_tree_test.cc:424: return;
ಠ_ృ

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:507:
gfx::IntersectRects(root_surface->damage_tracker()->current_damage_rect(),
I think you're looking for gfx::Rect::Intersects

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:849: if (active_tree_->root_layer()) {
Why only do this if there's a root layer? If there's not a root layer, then
adding layers the next time we draw will cause damage.

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl.h (right):

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.h:128: bool has_no_damage;
Weren't you just saying FrameData had too many members?

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_unittest_damage.cc (right):

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_unittest_damage.cc:19: class
LayerTreeHostDamageTestNoDamageDoesNotSwap : public LayerTreeHostDamageTest {
80 col.

[danakj, 2013-03-26 02:00:10]

Also fixed some bad rebasing in the unit tests, they pass again now.

https://codereview.chromium.org/12662021/diff/26001/cc/test/layer_tree_test.cc
File cc/test/layer_tree_test.cc (right):

https://codereview.chromium.org/12662021/diff/26001/cc/test/layer_tree_test.c...
cc/test/layer_tree_test.cc:424: return;
On 2013/03/26 01:42:07, enne wrote:
> ಠ_ృ

Haha.. Oops, was gdbing. We need a command line flag to disable this. Thanks for
spotting.

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:507:
gfx::IntersectRects(root_surface->damage_tracker()->current_damage_rect(),
On 2013/03/26 01:42:07, enne wrote:
> I think you're looking for gfx::Rect::Intersects

So I am!

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:849: if (active_tree_->root_layer()) {
On 2013/03/26 01:42:07, enne wrote:
> Why only do this if there's a root layer? If there's not a root layer, then
> adding layers the next time we draw will cause damage.

That is true, but it may not be a full frame if the root layer doesn't cover the
viewport. Until you add a root layer, we don't draw, so we don't clear damage,
so this seemed right to me.

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl.h (right):

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.h:128: bool has_no_damage;
On 2013/03/26 01:42:07, enne wrote:
> Weren't you just saying FrameData had too many members?

Yes, this is bad and I feel bad.

I tried to do this differently, but you can't just decide based on the
|render_passes| being empty, or on the damage being empty, because these are
always true once you get to SwapBuffers(). So.. yeh.

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_unittest_damage.cc (right):

https://codereview.chromium.org/12662021/diff/26001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_unittest_damage.cc:19: class
LayerTreeHostDamageTestNoDamageDoesNotSwap : public LayerTreeHostDamageTest {
On 2013/03/26 01:42:07, enne wrote:
> 80 col.

ty

[danakj, 2013-03-26 02:26:05]

https://codereview.chromium.org/12662021/diff/26001/cc/test/layer_tree_test.cc
File cc/test/layer_tree_test.cc (right):

https://codereview.chromium.org/12662021/diff/26001/cc/test/layer_tree_test.c...
cc/test/layer_tree_test.cc:424: return;
 2013/03/26 02:00:10, danakj wrote:
> On 2013/03/26 01:42:07, enne wrote:
> > ಠ_ృ
> 
> Haha.. Oops, was gdbing. We need a command line flag to disable this. Thanks
for
> spotting.

This will keep it from happening again:
https://codereview.chromium.org/12907011/

[nduca, 2013-03-26 20:09:47]

I think the CanDraw is meant to protect against white outs, but it doesn't
protect against "its not safe to draw" cases. Iirw, DrawLayers basically has to
always succeed.

[danakj, 2013-03-26 20:25:36]

On Tue, Mar 26, 2013 at 4:09 PM, <nduca@chromium.org> wrote:

> I think the CanDraw is meant to protect against white outs, but it doesn't
> protect against "its not safe to draw" cases. Iirw, DrawLayers basically
> has to
> always succeed.
>

In this case it succeeds because it has nothing to draw. It does not update
the debug info etc since there was no frame produced. I'm not clear if
you're saying this is OK or not?


>
>
https://codereview.chromium.**org/12662021/<https://codereview.chromium.org/1...
>

[nduca, 2013-03-26 23:19:49]

Use your discretion I guess. :) Worst case, the universe explodes and we learn
something! I think its fine.

[danakj, 2013-03-29 16:48:28]

enne: ping. should we go ahead with this? anything you want me to experiment
with first?

[enne (OOO), 2013-03-29 16:54:20]

lgtm

I think this is probably ok.  I have this faint memory where not swapping would
cause problems, but I wonder if the ScheduledActionDrawAndSwapResult::did_swap
has that covered these days.  Also, you're careful about always setting a damage
rect during readback, so that should be fine too.

[commit-bot: I haz the power, 2013-03-29 17:11:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/38002

[commit-bot: I haz the power, 2013-03-29 17:29:35]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-03-29 17:36:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/52003

[commit-bot: I haz the power, 2013-03-29 18:43:48]

Failed to trigger a try job on linux_chromeos
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-03-29 18:43:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/65002

[commit-bot: I haz the power, 2013-03-29 19:11:32]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-03-29 19:45:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/65002

[commit-bot: I haz the power, 2013-03-29 21:01:59]

Failed to trigger a try job on linux_chromeos
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-03-29 21:02:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/70001

[commit-bot: I haz the power, 2013-03-29 22:47:27]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-03-29 23:30:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/70001

[commit-bot: I haz the power, 2013-03-30 00:39:33]

Retried try job too often on linux_chromeos for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-03-30 03:24:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/70001

[commit-bot: I haz the power, 2013-03-30 04:26:30]

Retried try job too often on linux_chromeos for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-03-30 22:29:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/70001

[commit-bot: I haz the power, 2013-03-30 22:29:50]

Failed to apply patch for cc/trees/layer_tree_host_impl.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/trees/layer_tree_host_impl.h
  Hunk #4 FAILED at 459.
  Hunk #5 succeeded at 463 (offset -7 lines).
  1 out of 5 hunks FAILED -- saving rejects to file
cc/trees/layer_tree_host_impl.h.rej

Patch:       cc/trees/layer_tree_host_impl.h
Index: cc/trees/layer_tree_host_impl.h
diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
index
08ade52442b3044bd7a24a1a5c4dce154fa9ae78..cef6e234e7e015b07d2d5d6cbc3327b19c8ba1dd
100644
--- a/cc/trees/layer_tree_host_impl.h
+++ b/cc/trees/layer_tree_host_impl.h
@@ -131,6 +131,7 @@ class CC_EXPORT LayerTreeHostImpl : public
InputHandlerClient,
     const LayerImplList* render_surface_layer_list;
     LayerImplList will_draw_layers;
     bool contains_incomplete_tile;
+    bool has_no_damage;
 
     // RenderPassSink implementation.
     virtual void AppendRenderPass(scoped_ptr<RenderPass> render_pass) OVERRIDE;
@@ -148,7 +149,8 @@ class CC_EXPORT LayerTreeHostImpl : public
InputHandlerClient,
   // to avoid displaying the frame. If PrepareToDraw is called,
DidDrawAllLayers
   // must also be called, regardless of whether DrawLayers is called between
the
   // two.
-  virtual bool PrepareToDraw(FrameData* frame);
+  virtual bool PrepareToDraw(FrameData* frame,
+                             gfx::Rect device_viewport_damage_rect);
   virtual void DrawLayers(FrameData* frame, base::TimeTicks frame_begin_time);
   // Must be called if and only if PrepareToDraw was called.
   void DidDrawAllLayers(const FrameData& frame);
@@ -205,7 +207,7 @@ class CC_EXPORT LayerTreeHostImpl : public
InputHandlerClient,
   Renderer* renderer() { return renderer_.get(); }
   const RendererCapabilities& GetRendererCapabilities() const;
 
-  virtual bool SwapBuffers();
+  virtual bool SwapBuffers(const FrameData& frame);
   void EnableVSyncNotification(bool enable);
 
   void Readback(void* pixels, gfx::Rect rect_in_device_viewport);
@@ -457,7 +459,6 @@ class CC_EXPORT LayerTreeHostImpl : public
InputHandlerClient,
   int64 num_main_thread_scrolls_;
 
   int64 cumulative_num_layers_drawn_;
-
   int64 cumulative_num_missing_tiles_;
 
   // The maximum memory that would be used by the prioritized resource
@@ -468,6 +469,8 @@ class CC_EXPORT LayerTreeHostImpl : public
InputHandlerClient,
   size_t last_sent_memory_visible_and_nearby_bytes_;
   size_t last_sent_memory_use_bytes_;
 
+  bool next_frame_damages_full_device_viewport_;
+
   base::TimeTicks current_frame_time_;
 
   scoped_ptr<AnimationRegistrar> animation_registrar_;

[commit-bot: I haz the power, 2013-03-30 23:21:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/101001

[commit-bot: I haz the power, 2013-03-31 01:15:11]

Retried try job too often on linux_chromeos for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[danakj, 2013-03-31 03:53:01]

This definitely breaks a test. I'll have to figure out why and what to do about
it.

[danakj, 2013-04-04 03:49:45]

The unit test that is failing is
WebContentsViewAuraTest.QuickOverscrollDirectionChange.

It failed because the message loop never became empty because we are constantly
requesting to commit/composite, because there is an animation that is never
cleaned up since we don't tick the animations if we aren't drawing.

This is all fixed up in https://codereview.chromium.org/13613003/ and the unit
test passes with those changes.

[commit-bot: I haz the power, 2013-04-06 00:36:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/101001

[commit-bot: I haz the power, 2013-04-06 00:37:03]

Failed to apply patch for cc/trees/single_thread_proxy.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/trees/single_thread_proxy.cc
  Hunk #3 succeeded at 335 (offset 5 lines).
  Hunk #4 succeeded at 370 (offset 5 lines).
  Hunk #5 FAILED at 392.
  Hunk #6 succeeded at 431 with fuzz 2 (offset 11 lines).
  1 out of 6 hunks FAILED -- saving rejects to file
cc/trees/single_thread_proxy.cc.rej

Patch:       cc/trees/single_thread_proxy.cc
Index: cc/trees/single_thread_proxy.cc
diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc
index
84c24e056246016c950c3479e58a3d781204ca01..88ba4f6ea159d3f08599ff20a257643daa9ebfa4
100644
--- a/cc/trees/single_thread_proxy.cc
+++ b/cc/trees/single_thread_proxy.cc
@@ -54,7 +54,12 @@ bool SingleThreadProxy::CompositeAndReadback(void* pixels,
gfx::Rect rect) {
   TRACE_EVENT0("cc", "SingleThreadProxy::CompositeAndReadback");
   DCHECK(Proxy::IsMainThread());
 
-  if (!CommitAndComposite(base::TimeTicks::Now()))
+  gfx::Rect device_viewport_damage_rect = rect;
+
+  LayerTreeHostImpl::FrameData frame;
+  if (!CommitAndComposite(base::TimeTicks::Now(),
+                          device_viewport_damage_rect,
+                          &frame))
     return false;
 
   {
@@ -64,7 +69,7 @@ bool SingleThreadProxy::CompositeAndReadback(void* pixels,
gfx::Rect rect) {
     if (layer_tree_host_impl_->IsContextLost())
       return false;
 
-    layer_tree_host_impl_->SwapBuffers();
+    layer_tree_host_impl_->SwapBuffers(frame);
   }
   DidSwapFrame();
 
@@ -325,8 +330,13 @@ void SingleThreadProxy::DidLoseOutputSurfaceOnImplThread()
{
 // Called by the legacy scheduling path (e.g. where render_widget does the
 // scheduling)
 void SingleThreadProxy::CompositeImmediately(base::TimeTicks frame_begin_time)
{
-  if (CommitAndComposite(frame_begin_time)) {
-    layer_tree_host_impl_->SwapBuffers();
+  gfx::Rect device_viewport_damage_rect;
+
+  LayerTreeHostImpl::FrameData frame;
+  if (CommitAndComposite(frame_begin_time,
+                         device_viewport_damage_rect,
+                         &frame)) {
+    layer_tree_host_impl_->SwapBuffers(frame);
     DidSwapFrame();
   }
 }
@@ -355,7 +365,10 @@ void SingleThreadProxy::ForceSerializeOnSwapBuffers() {
 
 void SingleThreadProxy::OnSwapBuffersCompleteOnImplThread() { NOTREACHED(); }
 
-bool SingleThreadProxy::CommitAndComposite(base::TimeTicks frame_begin_time) {
+bool SingleThreadProxy::CommitAndComposite(
+    base::TimeTicks frame_begin_time,
+    gfx::Rect device_viewport_damage_rect,
+    LayerTreeHostImpl::FrameData* frame) {
   DCHECK(Proxy::IsMainThread());
 
   if (!layer_tree_host_->InitializeRendererIfNeeded())
@@ -379,14 +392,19 @@ bool SingleThreadProxy::CommitAndComposite(base::TimeTicks
frame_begin_time) {
 
   layer_tree_host_->WillCommit();
   DoCommit(queue.Pass());
-  bool result = DoComposite(offscreen_context_provider, frame_begin_time);
+  bool result = DoComposite(offscreen_context_provider,
+                            frame_begin_time,
+                            device_viewport_damage_rect,
+                            frame);
   layer_tree_host_->DidBeginFrame();
   return result;
 }
 
 bool SingleThreadProxy::DoComposite(
     scoped_refptr<cc::ContextProvider> offscreen_context_provider,
-    base::TimeTicks frame_begin_time) {
+    base::TimeTicks frame_begin_time,
+    gfx::Rect device_viewport_damage_rect,
+    LayerTreeHostImpl::FrameData* frame) {
   DCHECK(!output_surface_lost_);
   {
     DebugScopedSetImplThread impl(this);
@@ -407,10 +425,9 @@ bool SingleThreadProxy::DoComposite(
     if (!layer_tree_host_impl_->CanDraw())
       return false;
 
-    LayerTreeHostImpl::FrameData frame;
-    layer_tree_host_impl_->PrepareToDraw(&frame);
-    layer_tree_host_impl_->DrawLayers(&frame, frame_begin_time);
-    layer_tree_host_impl_->DidDrawAllLayers(frame);
+    layer_tree_host_impl_->PrepareToDraw(frame, device_viewport_damage_rect);
+    layer_tree_host_impl_->DrawLayers(frame, frame_begin_time);
+    layer_tree_host_impl_->DidDrawAllLayers(*frame);
     output_surface_lost_ = layer_tree_host_impl_->IsContextLost();
 
     layer_tree_host_impl_->BeginNextFrame();

[commit-bot: I haz the power, 2013-04-06 00:38:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12662021/110001

[commit-bot: I haz the power, 2013-04-06 05:01:24]

Change committed as 192706

[Tom Hudson, 2013-04-08 16:15:41]

Universe may not have exploded, but Clank is broken by this CL. I presume a side
effect of our private reliance on the compositor.

[danakj, 2013-04-08 16:19:34]

On Mon, Apr 8, 2013 at 12:15 PM, <tomhudson@chromium.org> wrote:

> Universe may not have exploded, but Clank is broken by this CL. I presume
> a side
> effect of our private reliance on the compositor.
>

Broken how?


>
>
https://chromiumcodereview.**appspot.com/12662021/<https://chromiumcodereview...
>

[danakj, 2013-04-08 16:20:01]

On Mon, Apr 8, 2013 at 12:19 PM, Dana Jansens <danakj@chromium.org> wrote:

> On Mon, Apr 8, 2013 at 12:15 PM, <tomhudson@chromium.org> wrote:
>
>> Universe may not have exploded, but Clank is broken by this CL. I presume
>> a side
>> effect of our private reliance on the compositor.
>>
>
> Broken how?
>

Actually, can you file a bug about it rather than discussing here?

Thanks!
- Dana


>
>
>>
>>
https://chromiumcodereview.**appspot.com/12662021/<https://chromiumcodereview...
>>
>
>

[no sievers, 2013-04-08 19:10:53]

On 2013/04/08 16:20:01, danakj wrote:
> On Mon, Apr 8, 2013 at 12:19 PM, Dana Jansens <mailto:danakj@chromium.org>
wrote:
> 
> > On Mon, Apr 8, 2013 at 12:15 PM, <mailto:tomhudson@chromium.org> wrote:
> >
> >> Universe may not have exploded, but Clank is broken by this CL. I presume
> >> a side
> >> effect of our private reliance on the compositor.
> >>
> >
> > Broken how?
> >
> 
> Actually, can you file a bug about it rather than discussing here?
> 
> Thanks!
> - Dana
> 
> 
> >
> >
> >>
> >>
>
https://chromiumcodereview.**appspot.com/12662021/%3Chttps://chromiumcoderevi...>
> >>
> >
> >

crbug.com/228978 filed. The problem is with the app expecting swap callbacks,
which are now sometimes skipped.