Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(434)

Issue 19705006: Use SkImage as a backing store for copying 2d Contexts to ImageBitmaps.

Created:
7 years, 5 months ago by arbesfeld
Modified:
7 years, 4 months ago
CC:
blink-reviews, jamesr, eae+blinkwatch, danakj, dglazkov+blink, Rik, adamk+blink_chromium.org, Stephen Chennney, aandrey+blink_chromium.org, jeez, pdr.
Visibility:
Public.

Description

Use SkImage as a backing store for copying 2d Contexts to ImageBitmaps. This patch is dependent on skia change https://codereview.chromium.org/19729007/ BUG=260933, 166658

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Patch Set 3 : Add SkiaImage class. #

Patch Set 4 : Change SkImage to use srcRect pointer. #

Total comments: 12

Patch Set 5 : Rebase. #

Patch Set 6 : Rebase. #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+727 lines, -36 lines) Patch
M LayoutTests/fast/canvas/canvas-createImageBitmap-recursive.html View 1 2 3 4 1 chunk +37 lines, -16 lines 0 comments Download
M LayoutTests/fast/canvas/canvas-createImageBitmap-recursive-expected.txt View 1 2 3 4 1 chunk +480 lines, -0 lines 0 comments Download
M Source/core/core.gypi View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/page/ImageBitmap.h View 1 2 3 4 5 2 chunks +4 lines, -2 lines 0 comments Download
M Source/core/page/ImageBitmap.cpp View 1 2 3 4 5 6 chunks +17 lines, -9 lines 0 comments Download
M Source/core/platform/graphics/ImageBuffer.h View 1 2 3 4 5 4 chunks +5 lines, -0 lines 0 comments Download
M Source/core/platform/graphics/ImageBuffer.cpp View 1 2 3 4 5 4 chunks +38 lines, -9 lines 3 comments Download
A Source/core/platform/graphics/ReadOnlyBitmapImage.h View 1 2 3 4 1 chunk +71 lines, -0 lines 0 comments Download
A Source/core/platform/graphics/ReadOnlyBitmapImage.cpp View 1 2 3 4 1 chunk +73 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
arbesfeld
PTAL.
7 years, 5 months ago (2013-07-18 19:33:56 UTC) #1
Justin Novosad
On 2013/07/18 19:33:56, arbesfeld wrote: > PTAL. Don't use SkImage outside of core/platform/graphics. You should ...
7 years, 5 months ago (2013-07-18 22:14:02 UTC) #2
arbesfeld
On 2013/07/18 22:14:02, junov wrote: > On 2013/07/18 19:33:56, arbesfeld wrote: > > PTAL. > ...
7 years, 5 months ago (2013-07-19 19:16:32 UTC) #3
Justin Novosad
https://codereview.chromium.org/19705006/diff/1/Source/core/html/canvas/CanvasRenderingContext2D.cpp File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right): https://codereview.chromium.org/19705006/diff/1/Source/core/html/canvas/CanvasRenderingContext2D.cpp#newcode1618 Source/core/html/canvas/CanvasRenderingContext2D.cpp:1618: static void drawImageToContext(SkImage* image, GraphicsContext* context, FloatRect& dest, const ...
7 years, 5 months ago (2013-07-22 15:35:41 UTC) #4
arbesfeld
New patch for review. A lot of the case handling for canvas-derived ImageBitmaps could be ...
7 years, 4 months ago (2013-08-06 01:21:23 UTC) #5
arbesfeld
Rebased with https://codereview.chromium.org/22613002/
7 years, 4 months ago (2013-08-09 19:28:48 UTC) #6
Justin Novosad
https://codereview.chromium.org/19705006/diff/23001/Source/core/platform/graphics/ImageBuffer.cpp File Source/core/platform/graphics/ImageBuffer.cpp (right): https://codereview.chromium.org/19705006/diff/23001/Source/core/platform/graphics/ImageBuffer.cpp#newcode65 Source/core/platform/graphics/ImageBuffer.cpp:65: void ImageBuffer::createAcceleratedCanvas(const IntSize& size, OwnPtr<Canvas2DLayerBridge>* outLayerBridge, OpacityMode opacityMode) This ...
7 years, 4 months ago (2013-08-19 20:12:42 UTC) #7
Stephen White
https://codereview.chromium.org/19705006/diff/23001/Source/core/platform/graphics/ImageBuffer.cpp File Source/core/platform/graphics/ImageBuffer.cpp (right): https://codereview.chromium.org/19705006/diff/23001/Source/core/platform/graphics/ImageBuffer.cpp#newcode65 Source/core/platform/graphics/ImageBuffer.cpp:65: void ImageBuffer::createAcceleratedCanvas(const IntSize& size, OwnPtr<Canvas2DLayerBridge>* outLayerBridge, OpacityMode opacityMode) If ...
7 years, 4 months ago (2013-08-19 20:21:21 UTC) #8
danakj
https://codereview.chromium.org/19705006/diff/23001/Source/core/platform/graphics/ImageBuffer.cpp File Source/core/platform/graphics/ImageBuffer.cpp (right): https://codereview.chromium.org/19705006/diff/23001/Source/core/platform/graphics/ImageBuffer.cpp#newcode65 Source/core/platform/graphics/ImageBuffer.cpp:65: void ImageBuffer::createAcceleratedCanvas(const IntSize& size, OwnPtr<Canvas2DLayerBridge>* outLayerBridge, OpacityMode opacityMode) On ...
7 years, 4 months ago (2013-08-19 20:22:32 UTC) #9
Stephen White
On 2013/08/19 20:22:32, danakj wrote: > https://codereview.chromium.org/19705006/diff/23001/Source/core/platform/graphics/ImageBuffer.cpp > File Source/core/platform/graphics/ImageBuffer.cpp (right): > > https://codereview.chromium.org/19705006/diff/23001/Source/core/platform/graphics/ImageBuffer.cpp#newcode65 > ...
7 years, 4 months ago (2013-08-19 20:28:48 UTC) #10
Justin Novosad
7 years, 4 months ago (2013-08-19 20:35:27 UTC) #11
On 2013/08/19 20:28:48, Stephen White wrote:

> Ownership of this SkCanvas is really unclear ATM -- it's created by the
> Canvas2DLayerBridge, but never ref'ed, then passed to the ImageBuffer, which
> stores it in an OwnPtr. Justin has promised to clean it up, though. :)

Yep.  The plan is to refactor this to remove all the skia resource management
stuff out of Canvas2DLayerBridge and to make that class be just an interface to
the compositor.  The current design has the unfortunate side-effect of requiring
a Canvas2DLayerBridge whenever you create an accelerated ImageBuffer.

Powered by Google App Engine
This is Rietveld 408576698