Limit pixels that get cleared to repaint area.

Limit pixels that get cleared to repaint area.

Since we now keep the canvas around in the ContentLayerUpdater, we often
end up getting small invalidations that paint on a large canvas. In those
cases, instead of clearing the entire canvas to transparent, clear only the
portion that will be repainted.

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

[Vangelis Kokkevis, 2013-11-06 01:32:36]

Enne, can you please take a look?

[enne (OOO), 2013-11-06 01:59:45]

lgtm.  80% seems pretty reasonable here.

+reed FYI for any thoughts from the Skia side about clearing vs. drawColor

https://codereview.chromium.org/61403003/diff/1/cc/resources/content_layer_up...
File cc/resources/content_layer_updater.cc (right):

https://codereview.chromium.org/61403003/diff/1/cc/resources/content_layer_up...
cc/resources/content_layer_updater.cc:75: int canvasArea =
canvasArea => canvas_area

https://codereview.chromium.org/61403003/diff/1/cc/resources/content_layer_up...
cc/resources/content_layer_updater.cc:77: int clipArea = layer_rect.width() *
layer_rect.height();
clipArea => clip_area

[commit-bot: I haz the power, 2013-11-06 02:07:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vangelis@chromium.org/61403003/60001

[commit-bot: I haz the power, 2013-11-06 06:23:01]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[reed1, 2013-11-06 14:57:04]

Since you *always* set a clip, why not always call drawColor (which respects the
clip) instead of calling clear()?

[vangelis, 2013-11-06 16:35:53]

On 2013/11/06 14:57:04, reed1 wrote:
> Since you *always* set a clip, why not always call drawColor (which respects
the
> clip) instead of calling clear()?

I was under the impression that clear in general would be faster. But this was
mostly based on a quick code inspection where clear ends up in SkBitmap's clear
routine which just blasts the pixel values everywhere. drawColor seems to be
going though more checks. But if you think they would be equally fast, I'd be
happy to simplify.

[reed1, 2013-11-06 16:38:36]

On 2013/11/06 16:35:53, vangelis wrote:
> On 2013/11/06 14:57:04, reed1 wrote:
> > Since you *always* set a clip, why not always call drawColor (which respects
> the
> > clip) instead of calling clear()?
> 
> I was under the impression that clear in general would be faster. But this was
> mostly based on a quick code inspection where clear ends up in SkBitmap's
clear
> routine which just blasts the pixel values everywhere. drawColor seems to be
> going though more checks. But if you think they would be equally fast, I'd be
> happy to simplify.

On the raster side, the two should definitely be equal (modulo some setup
detection overhead). If they are not, it is a bug.

On the gpu side, I can't say... brian?

[Tom Hudson, 2013-11-06 16:44:38]

On 2013/11/06 16:38:36, reed1 wrote:
> On 2013/11/06 16:35:53, vangelis wrote:
> > On 2013/11/06 14:57:04, reed1 wrote:
> > > Since you *always* set a clip, why not always call drawColor (which
respects
> > the
> > > clip) instead of calling clear()?
> > 
> > I was under the impression that clear in general would be faster. But this
was
> > mostly based on a quick code inspection where clear ends up in SkBitmap's
> clear
> > routine which just blasts the pixel values everywhere. drawColor seems to be
> > going though more checks. But if you think they would be equally fast, I'd
be
> > happy to simplify.
> 
> On the raster side, the two should definitely be equal (modulo some setup
> detection overhead). If they are not, it is a bug.

This was a bug in the opposite direction a year ago
(https://code.google.com/p/skia/issues/detail?id=911), but Mike fixed it.

[robertphillips, 2013-11-06 19:55:03]

I don't think this use case will arise for the GPU. When SW rendering, the
result has been uploaded so the canvas can be recycled. For GPU rendering we
would need to keep the result around for compositing so would need to create a
new canvas or GPUDevice.

The GPU backend does have a flavor of clear that takes a rect and a
"the-rect-is-ignoreable" hint (for those platforms on which full clears are
cheaper then partial clears).

[Vangelis Kokkevis, 2013-11-07 01:46:10]

Thanks folks. Based on the the feedback and my own measurements on my Z620
(which confirm that there's no
appreciable difference between clear and drawPaint when clearing the entire
canvas) I simplified the code to 
always call drawPaint instead.

[commit-bot: I haz the power, 2013-11-07 01:56:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vangelis@chromium.org/61403003/360001

[commit-bot: I haz the power, 2013-11-07 08:02:05]

Change committed as 233539