cc: Rework how PLI cleans tilings up.

cc: Rework how PLI cleans tilings up.

This patch changes how PLI cleans tilings up. Previously, we'd iterate
all tilings to find the ones that match the criteria, then iterate
those and remove them in PLI.

With this patch, we ask PLTS to do most of the work. We calculate
scales that use layer information and pass it down to PLTS to
do the cleanup. PLTS, in turn, constructs a list of things to remove
and removes it, also removing it from the passed twin sets.

This allows us to remove one function from PLI and one from PLTS,
as well as hides more of the implementation details of PLTS from PLI.

R=danakj, enne
BUG=433048
Committed: https://crrev.com/4b892804b9aea20e96b31d5ddd4673ad622fd72f
Cr-Commit-Position: refs/heads/master@{#304972}

[vmpstr, 2014-11-19 21:50:24]

PTAL.

[enne (OOO), 2014-11-19 22:15:22]

Generating vectors of things that you could just operate on directly.  :(

Does this really clean things up? It seems like this function is pretty
intertwined with twins and PictureLayerImpl knowledge such that trying to add
this indirection only makes it harder to follow.

[vmpstr, 2014-11-19 22:25:47]

On 2014/11/19 22:15:22, enne wrote:
> Generating vectors of things that you could just operate on directly.  :(
> 
> Does this really clean things up? It seems like this function is pretty
> intertwined with twins and PictureLayerImpl knowledge such that trying to add
> this indirection only makes it harder to follow.

Yeah... I mean my hope is to one day remove "num_tilings" and "tiling_at"
from PLTS, and this is the only non-test non-iterator spot left...
the rest of them are equivalent to "bool PLTS::IsEmpty" checks.

The step after that would hopefully work towards not returning tilings,
but rather you can add scales (float), query which scales exist, and
ask PLTS things about scales, etc.

This would leave PLI in a spot where it deals with managing scales,
and using PLTS for everything else (like coverage for append quads,
and dealing with the actual logistics of dealing with tilings).

I don't have a good story for the whole twin thing yet though...


Anyway, I'm happy to hold off on this patch and work on cleaning up
the interaction a bit more. We'll see where it ends up.

[enne (OOO), 2014-11-19 22:32:50]

Yeah, I think this would be cleaner if you just pushed the whole thing into
PLTS, twins and used tilings and all.  If append quads was already there, then
it would have the used tilings already, perhaps?

[vmpstr, 2014-11-19 23:00:39]

On 2014/11/19 22:32:50, enne wrote:
> Yeah, I think this would be cleaner if you just pushed the whole thing into
> PLTS, twins and used tilings and all.  If append quads was already there, then
> it would have the used tilings already, perhaps?

Hmm... Ok, I'll see if that makes more sense. I gotta find the right balance,
so I don't push too much to PLTS :) Otherwise, it will just end up being
PLI that is a thin wrapper around PLTS.

[vmpstr, 2014-11-20 00:40:28]

PTAL. I followed your advice and moved more things into PLTS, and it actually
turned out pretty well, I think: we can now remove a function on PLI and PLTS
that was only used in this clean up process.

[enne (OOO), 2014-11-20 01:02:36]

Ooh, I like that a ton better.  lgtm

(Can you also update your change description to reflect this?)

[vmpstr, 2014-11-20 01:15:25]

On 2014/11/20 01:02:36, enne wrote:
> Ooh, I like that a ton better.  lgtm
> 
> (Can you also update your change description to reflect this?)

Done.

[vmpstr, 2014-11-20 01:16:39]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2014-11-20 01:18:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/742903002/40001

[commit-bot: I haz the power, 2014-11-20 04:27:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-20 04:29:05]

Patchset 3 (id:??) landed as
https://crrev.com/4b892804b9aea20e96b31d5ddd4673ad622fd72f
Cr-Commit-Position: refs/heads/master@{#304972}