EarlyOut_AnimatedTooRecently should only be done in software mode

EarlyOut_AnimatedTooRecently should only be done in software mode

BUG=106786
R=jamesr@chromium.org,jbates@chromium.org


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114041

[jbates, 2011-12-09 20:33:43]

Boo Ya. LGTM

[jamesr, 2011-12-09 20:38:14]

http://codereview.chromium.org/8898010/diff/2001/content/renderer/render_widg...
File content/renderer/render_widget.cc (right):

http://codereview.chromium.org/8898010/diff/2001/content/renderer/render_widg...
content/renderer/render_widget.cc:654: if (now >= animation_floor_time_ ||
is_accelerated_compositing_active_) {
shouldn't this be checking using_asynchronous_swapbuffers_ ? if it doesn't need
to, why do we have that bool?

[nduca, 2011-12-09 22:00:08]

using_async_swapbuffers_ is a unrelated system. That is used to control *when*
to run DoDeferredUpdate. The question is really --- should we ever be allowed to
submit a compositor frame that doesn't have the contents of animate() in it.

On the hypothetical system that has no async_swapbuffers [someone in
--in-process-gpu, I think], the compositor will basically draw as fast as it
can. In that case, we have two options:
1 - interleave animates at 16ms intervals
2 - put content in every draw

I think 2 is more correct.

[jamesr, 2011-12-09 22:11:48]

That's the exact same behavior as the current software path (paints as fast as
it can). The timer is set up so that in this case the RAF rate is still
something reasonable. I think this is better for authors and users in general
since the callback rate is close to the rate in other rendering modes and it
keeps CPU use at a reasonable rate.

I don't see why we would want to behave differently in software path w/out
framerate limiting vs compositing path w/out framerate limiting.

[nduca, 2011-12-09 22:29:02]

I disagree. Lets chat in person.

[jamesr, 2011-12-09 22:35:25]

On 2011/12/09 22:29:02, nduca wrote:
> I disagree. Lets chat in person.

OK. Let me try to understand what Antoine's patch did here before we do.

[jamesr, 2011-12-09 23:49:37]

On 2011/12/09 22:35:25, jamesr wrote:
> On 2011/12/09 22:29:02, nduca wrote:
> > I disagree. Lets chat in person.
> 
> OK. Let me try to understand what Antoine's patch did here before we do.

Aha! I was under the mistaken impression that there wasn't any rate limiting in
the case where we were compositing but we weren't using the async swap buffers.
Nat reminded me that there's also rate limiting in the command buffer itself to
hold us to only one pending ack, so the average framerate still can't exceed the
display rate.

With that knowledge this patch looks great. LGTM

[commit-bot: I haz the power, 2011-12-09 23:51:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nduca@chromium.org/8898010/2001

[commit-bot: I haz the power, 2011-12-10 02:04:52]

Try job failure for 8898010-2001 (retry) on win_rel for step "ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2011-12-12 16:38:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nduca@chromium.org/8898010/2001

[commit-bot: I haz the power, 2011-12-12 18:15:07]

Change committed as 114041