Make tile size a function of the device scale factor.

Make tile size a function of the device scale factor.

This preserves a similar step function for android, and either 256 or
512 for desktop, but will change when the device scale factor changes.

Once we know a stable "screen size" instead of a viewport size, we can
use this same function for desktop and ganesh also.

R=aelias, enne
BUG=622885
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

[danakj, 2016-06-25 03:28:56]

Description was changed from

==========
Make tile size a function of the device scale factor.

This preserves a similar step function for android, and either 256 or
512 for desktop, but will change when the device scale factor changes.

Once we know a stable "screen size" instead of a viewport size, we can
use this same function for desktop and ganesh also.

R=aelias, enne
BUG=622885
==========

to

==========
Make tile size a function of the device scale factor.

This preserves a similar step function for android, and either 256 or
512 for desktop, but will change when the device scale factor changes.

Once we know a stable "screen size" instead of a viewport size, we can
use this same function for desktop and ganesh also.

R=aelias, enne
BUG=622885
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[enne (OOO), 2016-06-28 18:39:02]

https://codereview.chromium.org/2099903002/diff/1/chrome/browser/about_flags.cc
File chrome/browser/about_flags.cc (left):

https://codereview.chromium.org/2099903002/diff/1/chrome/browser/about_flags....
chrome/browser/about_flags.cc:244: { IDS_FLAGS_DEFAULT_TILE_WIDTH_GRANDE,
*snrk*

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

https://codereview.chromium.org/2099903002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:56: // TODO(crbug.com/622885) But why not just
use kTileMinimalAlignment.
I suspect it's for the same reason as kTileRoundUp, to increase reuse of pooled
resources?

https://codereview.chromium.org/2099903002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:725: if
(layer_tree_impl()->use_gpu_rasterization()) {
This function is a little long and hard to read.  For readability, could you
have a series of "CalculateTileSizeForXXX" functions?

https://codereview.chromium.org/2099903002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:749: // For CPU rasterization we use stuff.
stuff, eh?

https://codereview.chromium.org/2099903002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:806: // Shrink the tile width/height to be
closer content width/height to save
closer...to?

https://codereview.chromium.org/2099903002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/2099903002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:2420: // XXX DIE DIE DIE DIE
https://www.youtube.com/watch?v=UrHVBfSaPtA

https://codereview.chromium.org/2099903002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:2424: gfx::Size
LayerTreeHostImpl::GetStableScreenSize(
How does this relate to gfx::DeviceDisplayInfo::GetPhysicalDisplay*? Is it the
same, always?

https://codereview.chromium.org/2099903002/diff/20001/content/renderer/gpu/re...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/2099903002/diff/20001/content/renderer/gpu/re...
content/renderer/gpu/render_widget_compositor.cc:41: #include
"cc/tiles/picture_layer_tiling.h"
?

[vmpstr, 2016-06-29 01:40:23]

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

https://codereview.chromium.org/2099903002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:750: int tile_size;
Initialize to 0, so that DCHECK on 787 fails if we don't set it

https://codereview.chromium.org/2099903002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:768: int num_tiles = 3;
can you const int kNumTiles this?

https://codereview.chromium.org/2099903002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl.cc:797: tile_height *= 2;
Kind of overly safe, but can this break kMaxUntiledContentSize? Do we care?

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

https://codereview.chromium.org/2099903002/diff/20001/cc/layers/picture_layer...
cc/layers/picture_layer_impl_unittest.cc:4819: int expected_tile_size =
MathUtil::UncheckedRoundUp(viewport_size / 3, 32);
I'd prefer if we moved 3 to some common place, so that we don't have to update
tests if we decide on a different value, but up to you.

[aelias_OOO_until_Jul13, 2016-06-29 07:21:16]

https://codereview.chromium.org/2099903002/diff/20001/cc/trees/layer_tree_impl.h
File cc/trees/layer_tree_impl.h (right):

https://codereview.chromium.org/2099903002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.h:108: gfx::Size GetStableScreenSize() const;
Hmm, any reason you decided not to use OutputSurface::SurfaceSize() like I
mentioned on the bug, since that ought to be more stable?

[danakj, 2016-06-29 19:47:39]

https://codereview.chromium.org/2099903002/diff/20001/cc/trees/layer_tree_impl.h
File cc/trees/layer_tree_impl.h (right):

https://codereview.chromium.org/2099903002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.h:108: gfx::Size GetStableScreenSize() const;
On 2016/06/29 07:21:16, aelias wrote:
> Hmm, any reason you decided not to use OutputSurface::SurfaceSize() like I
> mentioned on the bug, since that ought to be more stable?

SurfaceSize() is passed from LTHI to Renderer::DrawFrame, which does Resize() on
the OutputSurface, which will return it from SurfaceSize() thereafter. The
external viewport size seems to be the input value to me, and SurfaceSize would
be from the same source, but be from the previous frame.

[boliu, 2017-01-06 19:07:18]

boliu@chromium.org changed reviewers:
+ boliu@chromium.org

[boliu, 2017-01-06 19:07:19]

Hi, I saw you mention this CL on the thread, and just happen to be looking at
the same code here in RenderWidgetCompositor. I'm trying to get rid of all usage
of android-only DeviceDisplayInfo.

Stable screen size is already available in the renderer:
RenderWidget::screen_info. Though the dimension there is in integral css pixels,
so the result of multiplying by the device scale factor ends up very close the
screen size. Should be good enough for calculating software tile size though?

I was just gonna make android use that, but if there is a larger effort to
refactor this for all platforms, I can wait for that :)

[boliu, 2017-01-06 19:07:29]

Description was changed from

==========
Make tile size a function of the device scale factor.

This preserves a similar step function for android, and either 256 or
512 for desktop, but will change when the device scale factor changes.

Once we know a stable "screen size" instead of a viewport size, we can
use this same function for desktop and ganesh also.

R=aelias, enne
BUG=622885
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Make tile size a function of the device scale factor.

This preserves a similar step function for android, and either 256 or
512 for desktop, but will change when the device scale factor changes.

Once we know a stable "screen size" instead of a viewport size, we can
use this same function for desktop and ganesh also.

R=aelias, enne
BUG=622885
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[boliu, 2017-01-06 19:07:29]

boliu@chromium.org changed reviewers:
- boliu@chromium.org

[danakj, 2017-01-06 19:21:17]

On Fri, Jan 6, 2017 at 2:07 PM, <boliu@chromium.org> wrote:

> Hi, I saw you mention this CL on the thread, and just happen to be looking
> at
> the same code here in RenderWidgetCompositor. I'm trying to get rid of all
> usage
> of android-only DeviceDisplayInfo.
>
> Stable screen size is already available in the renderer:
> RenderWidget::screen_info. Though the dimension there is in integral css
> pixels,
> so the result of multiplying by the device scale factor ends up very close
> the
> screen size. Should be good enough for calculating software tile size
> though?
>

That does sound good enough ya, thanks.

I was just gonna make android use that, but if there is a larger effort to
> refactor this for all platforms, I can wait for that :)
>

You should go for it, you can base it on this CL if you want or do your own
thing. But I wouldn't wait on me to land this, its super backburner atm.


>
> https://codereview.chromium.org/2099903002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.