First draft of TileManager's tile prioritzation system

First draft of TileManager's tile prioritzation system

BUG=155209
R=enne@chromium.org


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

[enne (OOO), 2012-11-15 19:03:24]

lgtm as a checkpoint commit

https://codereview.chromium.org/11417002/diff/2001/cc/tile_priority.h
File cc/tile_priority.h (right):

https://codereview.chromium.org/11417002/diff/2001/cc/tile_priority.h#newcode18
cc/tile_priority.h:18: MIGHT_EVENTUALLY_BE_CRISP = 2 // Not perfectly matched,
but hey, keep it around if you've got free space.
It's interesting to me that you said that this made all the difference over the
low res, but this really feels to me like the enum is:

low_res (more preferred low res)
high_res (closest to crisp high res)
non_ideal_res (too high or not the best low)

...am I missing anything?

[nduca, 2012-11-16 02:00:26]

I like it. I'll call it non_ideal_res.

[slavi, 2012-11-16 02:44:53]

http://codereview.chromium.org/11417002/diff/2004/cc/tile_priority.h
File cc/tile_priority.h (right):

http://codereview.chromium.org/11417002/diff/2004/cc/tile_priority.h#newcode16
cc/tile_priority.h:16: LOW_RESOLUTION = 0 ,
Style nit: kFooBar for constants.

[nduca, 2012-11-16 02:52:23]

http://dev.chromium.org/developers/coding-style

Though the Google C++ Style Guide now says to use kConstantNaming for enums,
Chromium was written using MACRO_STYLE naming.  Continue to use this style for
consistency.

[reveman, 2012-11-16 03:05:06]

https://codereview.chromium.org/11417002/diff/8/cc/tile_manager.cc
File cc/tile_manager.cc (right):

https://codereview.chromium.org/11417002/diff/8/cc/tile_manager.cc#newcode86
cc/tile_manager.cc:86: for(TileVector::iterator it = tiles_.begin(); it !=
tiles_.end(); ++it) {
nit: space between "for" and "("

https://codereview.chromium.org/11417002/diff/8/cc/tile_manager.cc#newcode146
cc/tile_manager.cc:146: for(TileVector::iterator it = tiles_.begin(); it !=
tiles_.end(); ++it) {
here too

https://codereview.chromium.org/11417002/diff/8/cc/tile_manager.cc#newcode157
cc/tile_manager.cc:157: for(TileVector::iterator it = tiles_.begin(); it !=
tiles_.end(); ++it) {
here too

https://codereview.chromium.org/11417002/diff/8/cc/tile_manager.cc#newcode170
cc/tile_manager.cc:170: for(TileVector::iterator it = tiles_.begin(); it !=
tiles_.end(); ++it) {
and here

[skaslev, 2012-11-16 03:34:07]

Interesting. It seems that Chrome does indeed use both styles. My grep-fu
is not sufficient to tell which one is more prevalent. I prefer Google
style though.


On Thu, Nov 15, 2012 at 6:52 PM, <nduca@chromium.org> wrote:

>
http://dev.chromium.org/**developers/coding-style<http://dev.chromium.org/dev...
>
> Though the Google C++ Style Guide now says to use kConstantNaming for
> enums,
> Chromium was written using MACRO_STYLE naming.  Continue to use this style
> for
> consistency.
>
>
>
http://codereview.chromium.**org/11417002/<http://codereview.chromium.org/114...
>
> --
>
>
>

[commit-bot: I haz the power, 2012-11-16 03:44:35]

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

[jamesr1, 2012-11-16 04:06:38]

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/RxEyIrolvs0/dis...

On Thu, Nov 15, 2012 at 7:34 PM, Slavomir Kaslev <skaslev@google.com> wrote:

> Interesting. It seems that Chrome does indeed use both styles. My grep-fu
> is not sufficient to tell which one is more prevalent. I prefer Google
> style though.
>
>
> On Thu, Nov 15, 2012 at 6:52 PM, <nduca@chromium.org> wrote:
>
>>
http://dev.chromium.org/**developers/coding-style<http://dev.chromium.org/dev...
>>
>> Though the Google C++ Style Guide now says to use kConstantNaming for
>> enums,
>> Chromium was written using MACRO_STYLE naming.  Continue to use this
>> style for
>> consistency.
>>
>>
>>
http://codereview.chromium.**org/11417002/<http://codereview.chromium.org/114...
>>
>> --
>>
>>
>>
>  --
>
>
>

[commit-bot: I haz the power, 2012-11-16 08:05:49]

Retried try job too often for step(s) browser_tests

[commit-bot: I haz the power, 2012-11-16 08:12:56]

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

[commit-bot: I haz the power, 2012-11-16 10:00:49]

Change committed as 168175