cc: Early out of UpdateTiles for resoureceless draw

cc: Early out of UpdateTiles for resoureceless draw

This ensures that software draws with different draw properties do
not affect tiling decisions. This avoids an invalidate-redraw loop
if every invalidate leads to resourceless draw(s) with different
draw properties that cause more tiling updates and invalidates.

BUG=577433
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/99b904adcfc2d25f5a06bcf1e122387ef2c21d7d
Cr-Commit-Position: refs/heads/master@{#371615}

[boliu, 2016-01-15 00:19:21]

Description was changed from

==========
cc: Early out of UpdateTiles for resoureceless draw

WIP tests

BUG=577433
==========

to

==========
cc: Early out of UpdateTiles for resoureceless draw

WIP tests

BUG=577433
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2016-01-15 00:58:18]

danakj@chromium.org changed reviewers:
+ danakj@chromium.org, vmpstr@chromium.org

[danakj, 2016-01-15 00:58:18]

https://codereview.chromium.org/1591513002/diff/1/cc/layers/picture_layer_impl.h
File cc/layers/picture_layer_impl.h (right):

https://codereview.chromium.org/1591513002/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl.h:159: // Any draw properties derived from
|transform|, |viewport|, and |clip|
This comment is no longer relevant?

[boliu, 2016-01-15 23:53:53]

Description was changed from

==========
cc: Early out of UpdateTiles for resoureceless draw

WIP tests

BUG=577433
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Early out of UpdateTiles for resoureceless draw

This ensures that software draws with different draw properties do
not affect tiling decisions. This avoids an invalidate-redraw loop
if every invalidate leads to resourceless draw(s) with different
draw properties that cause more tiling updates and invalidates.

BUG=577433
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[boliu, 2016-01-15 23:54:28]

ptal

just ported the test from the other CL

https://codereview.chromium.org/1591513002/diff/1/cc/layers/picture_layer_impl.h
File cc/layers/picture_layer_impl.h (right):

https://codereview.chromium.org/1591513002/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl.h:159: // Any draw properties derived from
|transform|, |viewport|, and |clip|
On 2016/01/15 00:58:18, danakj wrote:
> This comment is no longer relevant?

Rewrote this comment.

[boliu, 2016-01-19 23:17:17]

ping!

[boliu, 2016-01-21 00:06:18]

pong?

[boliu, 2016-01-25 16:49:57]

monday morning bump :p

[vmpstr, 2016-01-25 23:44:05]

Sorry for the delay, I was out last week.

https://codereview.chromium.org/1591513002/diff/20001/cc/layers/picture_layer...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/1591513002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:433: if (resourceless_software_draw)
It's a bit of an awkward pattern:

f(bool foo) {
  ...
  if (foo)
    return;
  ...
}

It feels like it should be 

f() {
 ...
} 

and 

g() { 
 ...
}

and the caller can just

f();
if (!foo)
 g();


The problem of course is how to name and separate these two functions so that
they still make sense. Other than that, there's only one non-test callsite for
this, so I think the split would make sense. What do you think?

[boliu, 2016-01-26 00:01:03]

https://codereview.chromium.org/1591513002/diff/20001/cc/layers/picture_layer...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/1591513002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:433: if (resourceless_software_draw)
On 2016/01/25 23:44:05, vmpstr wrote:
> It's a bit of an awkward pattern:
> 
> f(bool foo) {
>   ...
>   if (foo)
>     return;
>   ...
> }
> 
> It feels like it should be 
> 
> f() {
>  ...
> } 
> 
> and 
> 
> g() { 
>  ...
> }
> 
> and the caller can just
> 
> f();
> if (!foo)
>  g();
> 
> 
> The problem of course is how to name and separate these two functions so that
> they still make sense. Other than that, there's only one non-test callsite for
> this, so I think the split would make sense. What do you think?

Maybe don't need the split at all, and just skip everything?

Resourceless draw doesn't need to call CleanUpTilingsOnActiveLayer since
resourceless draw does not affect |last_append_quads_tilings_|.

I'm less sure about the !CanHaveTilings() block. According to
SanityCheckTilingState, |tilings_| is supposed to be empty already. Do the ideal
scales matter in that case?

[boliu, 2016-01-26 02:41:29]

ptal

Skipping UpdateTiles altogether

[vmpstr, 2016-01-26 18:24:50]

lgtm thanks

[boliu, 2016-01-26 18:46:41]

dana: any more comments?

[danakj, 2016-01-26 19:12:54]

Nope, I'm happy if vmpstr's happy.

[boliu, 2016-01-26 19:15:59]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2016-01-26 19:17:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1591513002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1591513002/60001

[commit-bot: I haz the power, 2016-01-26 19:37:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-26 19:37:46]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[boliu, 2016-01-26 21:10:29]

doh, didn't compile cc_perftests locally. fixed

[boliu, 2016-01-26 21:10:54]

The CQ bit was checked by boliu@chromium.org

[boliu, 2016-01-26 21:10:55]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/1591513002/#ps80001
(title: "fix perftests")

[commit-bot: I haz the power, 2016-01-26 21:11:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1591513002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1591513002/80001

[commit-bot: I haz the power, 2016-01-26 22:18:11]

Description was changed from

==========
cc: Early out of UpdateTiles for resoureceless draw

This ensures that software draws with different draw properties do
not affect tiling decisions. This avoids an invalidate-redraw loop
if every invalidate leads to resourceless draw(s) with different
draw properties that cause more tiling updates and invalidates.

BUG=577433
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Early out of UpdateTiles for resoureceless draw

This ensures that software draws with different draw properties do
not affect tiling decisions. This avoids an invalidate-redraw loop
if every invalidate leads to resourceless draw(s) with different
draw properties that cause more tiling updates and invalidates.

BUG=577433
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-01-26 22:18:12]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-01-26 22:19:37]

Description was changed from

==========
cc: Early out of UpdateTiles for resoureceless draw

This ensures that software draws with different draw properties do
not affect tiling decisions. This avoids an invalidate-redraw loop
if every invalidate leads to resourceless draw(s) with different
draw properties that cause more tiling updates and invalidates.

BUG=577433
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Early out of UpdateTiles for resoureceless draw

This ensures that software draws with different draw properties do
not affect tiling decisions. This avoids an invalidate-redraw loop
if every invalidate leads to resourceless draw(s) with different
draw properties that cause more tiling updates and invalidates.

BUG=577433
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/99b904adcfc2d25f5a06bcf1e122387ef2c21d7d
Cr-Commit-Position: refs/heads/master@{#371615}
==========

[commit-bot: I haz the power, 2016-01-26 22:19:38]

Patchset 5 (id:??) landed as
https://crrev.com/99b904adcfc2d25f5a06bcf1e122387ef2c21d7d
Cr-Commit-Position: refs/heads/master@{#371615}