cc: Remove legacy accelerated painting path

cc: Remove legacy accelerated painting path

R=nduca@chromium.org
BUG=none

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

[reveman, 2013-05-28 18:09:09]

Can you also remove ResourceUpdate::picture and all relevant code in
ResourceUpdateController? That code should be dead after removing
SkPictureContentLayerUpdater.

[enne (OOO), 2013-05-28 18:11:50]

On 2013/05/28 18:09:09, David Reveman wrote:
> Can you also remove ResourceUpdate::picture and all relevant code in
> ResourceUpdateController? That code should be dead after removing
> SkPictureContentLayerUpdater.

Done.  Thanks for pointing that out.

[piman, 2013-05-28 18:48:49]

Actually per-tile-painting is on by default for the Chrome OS UI compositor.
reveman@ turned it on a while ago, with data showing it was faster painting
overall. In particular there were benefits wrt damage tracking (tighter damage
regions when multiple parts of the screen update). I don't know if it's still
true...

I think it also has memory implications - since we never delete the layer
updaters, I fear that turning it off would leave large dangling canvas for
layers we only paint once - e.g. background.

[enne (OOO), 2013-05-28 19:49:23]

Boo.  I didn't realize the CrOS UI compositor was using per-tile painting.

What's the likelihood of it switching over to using impl-side painting?  It's
still single-threaded too, right?

[reveman, 2013-05-28 19:57:25]

On 2013/05/28 18:48:49, piman wrote:
> Actually per-tile-painting is on by default for the Chrome OS UI compositor.
> reveman@ turned it on a while ago, with data showing it was faster painting
> overall. In particular there were benefits wrt damage tracking (tighter damage
> regions when multiple parts of the screen update). I don't know if it's still
> true...
> 
> I think it also has memory implications - since we never delete the layer
> updaters, I fear that turning it off would leave large dangling canvas for
> layers we only paint once - e.g. background.

Yes, we probably need to keep BitmapSkpictureContentLayerUpdater and per-tile
painting until impl-side painting is used by the UI compositor. You can still
remove the accelerated painting parts and ResourceUpdate::picture though.

[enne (OOO), 2013-05-28 20:15:31]

PTAL.  Changed to only remove accelerated painting.

[piman, 2013-05-28 21:03:57]

On 2013/05/28 20:15:31, enne wrote:
> PTAL.  Changed to only remove accelerated painting.

LGTM

[piman, 2013-05-28 21:33:34]

On 2013/05/28 19:49:23, enne wrote:
> Boo.  I didn't realize the CrOS UI compositor was using per-tile painting.
> 
> What's the likelihood of it switching over to using impl-side painting?  It's
> still single-threaded too, right?

It's threaded already by default, so that part's good.

For switching to impl-side, it's on the radar, but we definitely didn't want to
rush it because the gains for the UI are only marginal, so we didn't want to pay
the latency cost. I think once all the little things are resolved, and the
latency work is done, we should look at it to help with the architectural
burden.

[reveman, 2013-05-28 22:40:27]

https://codereview.chromium.org/16069004/diff/22001/cc/resources/resource_upd...
File cc/resources/resource_update_controller.cc (right):

https://codereview.chromium.org/16069004/diff/22001/cc/resources/resource_upd...
cc/resources/resource_update_controller.cc:36: skia::RefPtr<SkCanvas>
CreateAcceleratedCanvas(
please also remove this and all includes it pulls in.

[enne (OOO), 2013-05-28 22:53:20]

https://codereview.chromium.org/16069004/diff/22001/cc/resources/resource_upd...
File cc/resources/resource_update_controller.cc (right):

https://codereview.chromium.org/16069004/diff/22001/cc/resources/resource_upd...
cc/resources/resource_update_controller.cc:36: skia::RefPtr<SkCanvas>
CreateAcceleratedCanvas(
On 2013/05/28 22:40:28, David Reveman wrote:
> please also remove this and all includes it pulls in.

Done.  Sorry for missing that.

[reveman, 2013-05-28 23:33:01]

lgtm

[commit-bot: I haz the power, 2013-05-28 23:44:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/16069004/27001

[commit-bot: I haz the power, 2013-05-29 00:08:01]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-29 00:15:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/16069004/39002

[commit-bot: I haz the power, 2013-05-29 03:08:26]

Change committed as 202761