Add --disable-low-res-tiling. Disable low res tiling to save power.

Add --disable-low-res-tiling. Disable low res tiling to save power.

Generating low resolution tiles uses about 10% of the power used by
pages that have gif animations (measured on Nexus 4). These are the
most common pages on which chrome consumes above baseline power.

It is unclear whether low resolutions are still valuable. It is
likely that having low resolution tiles is more useful on older
devices. For the moment providing this as a flag, so that it
is easier to investigate whether dropping low resolution tiles
is reasonable. Default behavior is unchanged, low resolution tiles
are enabled.

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257790

[Anton, 2014-03-12 15:53:25]

Hi,

Also interested in your opinion of whether low res tiling is still a useful
feature. In particular, do you know of any website, device, use case combination
where user experience benefits from low resolution tiles.

Thanks,
Anton

[enne (OOO), 2014-03-12 17:52:07]

This experimentation seems reasonable to me.

I think there's a lot of contention here, and I go back and forth on this.  I
personally think low res tilings are useful during pinch and scrolling scenarios
(when the CPU can't keep up with the scroll/pinch velocity).  Right now we
create low res tilings at all times, but maybe there's some way to create them
only at different times.

Right now, TileManager is the arbiter of what tiles should be rastered and
layers produce low res tilings to give TileManager the option to use them. 
Right now, it probably chooses to raster them far too frequently.  In a more
short/medium-term sense, I'm hoping that vmpstr's changes to tile prioritization
will help here.  This will move some of the decision making to layers, where
they can examine coverage and produce low res tile raster candidates only when
it makes sense to and it's more known that the CPU isn't keeping up, rather than
all the time.  See:
https://docs.google.com/a/chromium.org/document/d/1tkwOlSlXiR320dFufuA_M-RF9L...

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

https://codereview.chromium.org/196473007/diff/1/cc/layers/picture_layer_impl...
cc/layers/picture_layer_impl.h:93: return !should_use_gpu_rasterization() &&
should_use_low_res_tiling_;
Can you make this just should_use_low_res_tiling and have the could that enables
should_use_gpu_rasterization set that to true?

https://codereview.chromium.org/196473007/diff/1/cc/trees/layer_tree_settings.h
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/196473007/diff/1/cc/trees/layer_tree_settings...
cc/trees/layer_tree_settings.h:34: bool low_res_tiling;
bikeshed: should_create_low_res_tilings?

[Anton, 2014-03-12 18:37:31]

On 2014/03/12 17:52:07, enne wrote:
> This experimentation seems reasonable to me.
> 
> I think there's a lot of contention here, and I go back and forth on this.  I
> personally think low res tilings are useful during pinch and scrolling
scenarios
> (when the CPU can't keep up with the scroll/pinch velocity).  Right now we
> create low res tilings at all times, but maybe there's some way to create them
> only at different times.

It is already the case that we don't generate low resolution tiles when pinching
(see the code in picture_layer_impl.cc:965), so hopefully pinching isn't a case 
when they are useful. I thought fast scrolling might be a case, but although I 
sometimes see low res tiles when scrolling, I don't see checkboarding or white 
tiles when they are turned off (i.e. I don't see checkboarding or white tiles). 
So my experience is that scrolling is better without low res tiles, though it
may be
different on a different device or with a particular tough page (examples?).
Also I am not sure that a human will perceive the presence of a white tile for
1/60th
of a second (it could easily be that the brain is more tolerant of that than it
is
of low res tiles, which it does see because they look blurred).

> Right now, TileManager is the arbiter of what tiles should be rastered and
> layers produce low res tilings to give TileManager the option to use them. 
> Right now, it probably chooses to raster them far too frequently.  In a more
> short/medium-term sense, I'm hoping that vmpstr's changes to tile
prioritization
> will help here.  This will move some of the decision making to layers, where
> they can examine coverage and produce low res tile raster candidates only when
> it makes sense to and it's more known that the CPU isn't keeping up, rather
than
> all the time.  See:
>
https://docs.google.com/a/chromium.org/document/d/1tkwOlSlXiR320dFufuA_M-RF9L...

Thanks for the link. I spent a while investigating this code, but couldn't see
a way to teach it enough for it to make a better decision.

> 
>
https://codereview.chromium.org/196473007/diff/1/cc/layers/picture_layer_impl.h
> File cc/layers/picture_layer_impl.h (right):
> 
>
https://codereview.chromium.org/196473007/diff/1/cc/layers/picture_layer_impl...
> cc/layers/picture_layer_impl.h:93: return !should_use_gpu_rasterization() &&
> should_use_low_res_tiling_;
> Can you make this just should_use_low_res_tiling and have the could that
enables
> should_use_gpu_rasterization set that to true?
> 
>
https://codereview.chromium.org/196473007/diff/1/cc/trees/layer_tree_settings.h
> File cc/trees/layer_tree_settings.h (right):
> 
>
https://codereview.chromium.org/196473007/diff/1/cc/trees/layer_tree_settings...
> cc/trees/layer_tree_settings.h:34: bool low_res_tiling;
> bikeshed: should_create_low_res_tilings?

[piman, 2014-03-13 00:51:58]

https://codereview.chromium.org/196473007/diff/20001/cc/base/switches.cc
File cc/base/switches.cc (right):

https://codereview.chromium.org/196473007/diff/20001/cc/base/switches.cc#newc...
cc/base/switches.cc:126: bool IsLowResTilingEnabled() {
Nit: this is only used in the renderer. Could this be a bool on
RenderThreadImpl, similar to other related state? Like e.g.
RenderThreadImpl::is_impl_side_painting_enabled()
That way we don't have to parse strings every time we create a compositor. Also
maybe one day we won't have to pass a gigantic command line to the renderer.

https://codereview.chromium.org/196473007/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/196473007/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.cc:1131:
cc::switches::kEnableLowResTiling,
Can you also add those to
chrome/browser/chromeos/login/chrome_restart_request.cc so that it also applies
to guest mode?

[Sami, 2014-03-13 13:30:31]

Drive by comment: good sites to test this on are ones where we have historically
had trouble with long raster times (e.g., desktop theverge.com, plus.google.com,
engadget.com).

[Anton, 2014-03-13 14:43:37]

I do see white tiles for theverge.com, but I see them for both with and
without low resolution tiles turn on, at the same frequency. Tried both on
Nexus 7 (1st Gen) and Nexus 4. On Nexus 7 it seems like at the end of
scroll after we have finished painting the screen we over paint with white,
this seems like a bug, though it is possible it is something in the design
of the page. Anyway not see anything of significance for low res tiles here.



On Thu, Mar 13, 2014 at 1:30 PM, <skyostil@chromium.org> wrote:

> Drive by comment: good sites to test this on are ones where we have
> historically
> had trouble with long raster times (e.g., desktop theverge.com,
> plus.google.com,
> engadget.com).
>
> https://codereview.chromium.org/196473007/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Anton, 2014-03-13 18:03:24]

Uploaded patch #3 which should address the code level comments.

Thanks,
Anton

On 2014/03/13 14:43:37, Anton wrote:
> I do see white tiles for http://theverge.com, but I see them for both with and
> without low resolution tiles turn on, at the same frequency. Tried both on
> Nexus 7 (1st Gen) and Nexus 4. On Nexus 7 it seems like at the end of
> scroll after we have finished painting the screen we over paint with white,
> this seems like a bug, though it is possible it is something in the design
> of the page. Anyway not see anything of significance for low res tiles here.
> 
> 
> 
> On Thu, Mar 13, 2014 at 1:30 PM, <mailto:skyostil@chromium.org> wrote:
> 
> > Drive by comment: good sites to test this on are ones where we have
> > historically
> > had trouble with long raster times (e.g., desktop http://theverge.com,
> > http://plus.google.com,
> > http://engadget.com).
> >
> > https://codereview.chromium.org/196473007/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Anton, 2014-03-13 18:06:52]

BTW the whitespace change was made by "git cl format", so hopefully that is ok!

Thanks,
Anton

On 2014/03/13 18:03:24, Anton wrote:
> Uploaded patch #3 which should address the code level comments.
> 
> Thanks,
> Anton
> 
> On 2014/03/13 14:43:37, Anton wrote:
> > I do see white tiles for http://theverge.com, but I see them for both with
and
> > without low resolution tiles turn on, at the same frequency. Tried both on
> > Nexus 7 (1st Gen) and Nexus 4. On Nexus 7 it seems like at the end of
> > scroll after we have finished painting the screen we over paint with white,
> > this seems like a bug, though it is possible it is something in the design
> > of the page. Anyway not see anything of significance for low res tiles here.
> > 
> > 
> > 
> > On Thu, Mar 13, 2014 at 1:30 PM, <mailto:skyostil@chromium.org> wrote:
> > 
> > > Drive by comment: good sites to test this on are ones where we have
> > > historically
> > > had trouble with long raster times (e.g., desktop http://theverge.com,
> > > http://plus.google.com,
> > > http://engadget.com).
> > >
> > > https://codereview.chromium.org/196473007/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.

[enne (OOO), 2014-03-13 18:23:28]

lgtm

https://codereview.chromium.org/196473007/diff/40001/cc/trees/layer_tree_sett...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/196473007/diff/40001/cc/trees/layer_tree_sett...
cc/trees/layer_tree_settings.h:38: bool low_res_tiling;
Can this be should_create_low_res_tiling or low_res_tiling_enabled?
low_res_tiling on its own isn't all that descriptive, especially if somebody
else adds another low_res_tiling in the future.

[piman, 2014-03-13 18:26:35]

LGTM, thanks!

[Anton, 2014-03-13 18:33:34]

On 2014/03/13 18:23:28, enne wrote:
> lgtm
> 
>
https://codereview.chromium.org/196473007/diff/40001/cc/trees/layer_tree_sett...
> File cc/trees/layer_tree_settings.h (right):
> 
>
https://codereview.chromium.org/196473007/diff/40001/cc/trees/layer_tree_sett...
> cc/trees/layer_tree_settings.h:38: bool low_res_tiling;
> Can this be should_create_low_res_tiling or low_res_tiling_enabled?
> low_res_tiling on its own isn't all that descriptive, especially if somebody
> else adds another low_res_tiling in the future.

Patch 4
I am dropping the "should" since it is dropped from the other settings in this
file. 
I am adding the "create" which seems like the right compromise.

Anton

[enne (OOO), 2014-03-13 18:36:04]

lgtm, thanks!

[Anton, 2014-03-17 18:14:49]

The CQ bit was checked by anton@chromium.org

[commit-bot: I haz the power, 2014-03-17 18:15:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/anton@chromium.org/196473007/120001

[commit-bot: I haz the power, 2014-03-17 18:17:54]

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

[commit-bot: I haz the power, 2014-03-17 18:17:54]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_clang_dbg
  tryserver.chromium on linux_chromium_chromeos_rel
  tryserver.chromium on linux_chromium_clang_dbg
  tryserver.chromium on linux_chromium_rel

[Anton, 2014-03-18 17:17:30]

The CQ bit was checked by anton@chromium.org

[commit-bot: I haz the power, 2014-03-18 17:30:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/anton@chromium.org/196473007/120001

[commit-bot: I haz the power, 2014-03-18 19:05:49]

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

[commit-bot: I haz the power, 2014-03-18 19:05:50]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[Anton, 2014-03-18 19:35:32]

The CQ bit was checked by anton@chromium.org

[commit-bot: I haz the power, 2014-03-18 19:41:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/anton@chromium.org/196473007/120001

[commit-bot: I haz the power, 2014-03-18 22:37:01]

Change committed as 257790