Fixing 2D canvas invalidation region tracking.

Fixing 2D canvas invalidation region tracking.

The partial layer invalidation logic that was added in r176815 was not
properly transforming the invalidation region from the layer's local
frame of reference to the layer's content area.  The proper logic
already existed for non-accelerated 2d canvases, but we were short-
circuiting it whith acceleration enabled because we did not
support partial invalidations in the past.  This fix makes
accelerated 2D canvases use the same invalidation code path as
non-accelerated canvases.

TEST=fast/canvas/canvas-partial-invalidation-zoomed.html
BUG=393192
NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178392

[Justin Novosad, 2014-07-17 15:13:24]

https://codereview.chromium.org/402613006/diff/1/Source/core/html/HTMLCanvasE...
File Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/402613006/diff/1/Source/core/html/HTMLCanvasE...
Source/core/html/HTMLCanvasElement.cpp:204: FloatRect mappedDirtyRect =
mapRect(r, srcRect, ro->contentBoxRect());
The change to this function does not actually fix anything, it is just a
drive-by optimization. I changed it so that we accumulate the dirty rect in
local space. This way, the early exit happens before the mapRect call.

[Justin Novosad, 2014-07-17 15:17:29]

A note regarding test coverage: the layout test I added only fails on ToT on
windows with dx11 disabled. Therefore the chromium.webkit waterfall would not
detect the failure. The failure would be detected on the intel GPU bots on the
chromium.gpu waterfall.

[Stephen White, 2014-07-17 15:28:19]

LGTM

https://codereview.chromium.org/402613006/diff/1/Source/core/html/canvas/Canv...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (left):

https://codereview.chromium.org/402613006/diff/1/Source/core/html/canvas/Canv...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:1749: if (isAccelerated()
&& platformLayer()) {
Can we get rid of the platformLayer() override now? It looks like this might be
the only use.

https://codereview.chromium.org/402613006/diff/1/Source/core/rendering/compos...
File Source/core/rendering/compositing/CompositedLayerMapping.cpp (right):

https://codereview.chromium.org/402613006/diff/1/Source/core/rendering/compos...
Source/core/rendering/compositing/CompositedLayerMapping.cpp:1079: //           
    hasPaintedContent = false;
?

https://codereview.chromium.org/402613006/diff/1/Source/platform/graphics/Ima...
File Source/platform/graphics/ImageBuffer.cpp (right):

https://codereview.chromium.org/402613006/diff/1/Source/platform/graphics/Ima...
Source/platform/graphics/ImageBuffer.cpp:126: void ImageBuffer::didPresent()
It seems unfortunate that ImageBuffer has all this functionality that's not
really related to being a leaf class bag o' pixels. But I guess that ship has
sailed.

[Justin Novosad, 2014-07-17 16:31:29]

On 2014/07/17 15:28:19, Stephen White wrote:
> It seems unfortunate that ImageBuffer has all this functionality that's not
> really related to being a leaf class bag o' pixels. But I guess that ship has
> sailed.

Yeah, Canvas2DLayerBridge murdered that paradigm.  But we may be able to go back
to that utopia depending on how far we push the display list canvas
implementation. The layer bridge may become entirely obsolete if we start
delegating all rasterization to other threads (i.e. to the compositor or to a
raster thread for the synchronous cases that can't be handled on the compositor
thread).

[Justin Novosad, 2014-07-17 17:07:21]

https://codereview.chromium.org/402613006/diff/1/Source/core/html/canvas/Canv...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (left):

https://codereview.chromium.org/402613006/diff/1/Source/core/html/canvas/Canv...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:1749: if (isAccelerated()
&& platformLayer()) {
On 2014/07/17 15:28:19, Stephen White wrote:
> Can we get rid of the platformLayer() override now? It looks like this might
be
> the only use.

We can't. That method is a virtual override from CanvasRenderingContext and is
invoked by CompositedLayerMapping.

https://codereview.chromium.org/402613006/diff/1/Source/core/rendering/compos...
File Source/core/rendering/compositing/CompositedLayerMapping.cpp (right):

https://codereview.chromium.org/402613006/diff/1/Source/core/rendering/compos...
Source/core/rendering/compositing/CompositedLayerMapping.cpp:1079: //           
    hasPaintedContent = false;
On 2014/07/17 15:28:19, Stephen White wrote:
> ?

Done.

[Justin Novosad, 2014-07-17 17:07:45]

The CQ bit was checked by junov@chromium.org

[commit-bot: I haz the power, 2014-07-17 17:08:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/junov@chromium.org/402613006/20001

[commit-bot: I haz the power, 2014-07-17 18:26:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-17 19:25:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-17 19:25:38]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[Justin Novosad, 2014-07-17 19:33:29]

On 2014/07/17 19:25:38, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   win_gpu_triggered_tests on tryserver.chromium.gpu
>
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

Failure was an svn error on the bot. I will commit manually.

[Justin Novosad, 2014-07-17 19:37:00]

The CQ bit was checked by junov@chromium.org

[commit-bot: I haz the power, 2014-07-17 19:37:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/junov@chromium.org/402613006/20001

[commit-bot: I haz the power, 2014-07-17 19:37:55]

Change committed as 178392