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

Issue 933043006: Implement SkBaseDevice snapshot support

Created:
5 years, 10 months ago by Kimmo Kinnunen
Modified:
5 years, 9 months ago
Reviewers:
bsalomon, reed2, reed1
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@skimage-filters-04-snapshot-devices
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Implement SkBaseDevice snapshot support Implement SkImage snapshots for SkBaseDevices. This way filter source image, eg. the argument of SkCanvas::drawSprite or the resulting device of save layer can be converted to a SkImage snapshot. In later commits, this SkImage can be used as input to the filters. Decouples the following concepts: * Previously surface associated, now device associated snapshot SkImage snapshot * Surface generation id For SkBitmapDevice based devices and surfaces, the notification to do deep copy of the device "backend" (e.g the device backbuffer) is now SkBitmapDevice::onAccessBitmap and SkBitmapDevice::discard. For SkGpuDevice based surfaces, each draw operation of the device checks if the deep copy is needed. Before, these were intiated upon unneeded SkCanvas::predrawNotify(). (Unneeded, because by definition SkBaseDevice must see all the modification operations) For SkBitmapDevice based surfaces, the notification to change surface generation id is when the device backend bitmap has changed. This is tracked with SkBitmap generation id. For SkGpuDevice based surfaces, all device write operations call SkSurface_Gpu::notifyContentWillChange(), which will trigger surface generation id change. Removes redundant SkSurface::notifyContentWillChange(). The contents change conditions and notifications are internal details of the subclasses. This call is not useful for Skia clients using SkSurface. It is also not useful for implementing concrete SkSurface subclasses. Use new member function SkBaseDevice::newImageSnapshot() instead of SkBaseDevice::accessBitmap() to refer to the device contents for filter -related callsites. These callsites will later be modified to hand the SkImage directly to the filters. Move canvas->surface association to the device subclasses. This way the association is more natural: * One or many canvases refer to one device * SkCanvas does not know anything about surfaces * SkBaseDevice does not know anything about surfaces * One surface-aware device refers to one surfaces * The surface-aware device knows the concrete type of the surface (no upcasting) Bug=skia:3388

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : merge with the other patch, clear todos #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : rebase #

Patch Set 8 : rebase #

Patch Set 9 : rebase #

Patch Set 10 : rebase #

Patch Set 11 : rebase #

Patch Set 12 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+516 lines, -442 lines) Patch
M include/core/SkBitmapDevice.h View 1 2 3 4 5 6 7 8 9 3 chunks +7 lines, -3 lines 0 comments Download
M include/core/SkCanvas.h View 1 2 3 4 5 6 7 8 9 4 chunks +6 lines, -16 lines 0 comments Download
M include/core/SkDevice.h View 1 2 3 4 5 6 7 8 9 3 chunks +9 lines, -1 line 0 comments Download
M include/core/SkSurface.h View 1 2 3 2 chunks +9 lines, -34 lines 0 comments Download
M src/core/SkBitmapDevice.cpp View 1 2 3 4 5 6 7 8 9 6 chunks +44 lines, -6 lines 0 comments Download
M src/core/SkCanvas.cpp View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +29 lines, -21 lines 0 comments Download
M src/core/SkPictureRecord.h View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M src/gpu/SkGpuDevice.h View 1 2 3 4 5 6 7 8 9 6 chunks +29 lines, -4 lines 0 comments Download
M src/gpu/SkGpuDevice.cpp View 1 2 3 4 5 6 7 8 9 10 14 chunks +150 lines, -47 lines 0 comments Download
M src/image/SkSurface.cpp View 1 2 5 chunks +1 line, -47 lines 0 comments Download
M src/image/SkSurface_Base.h View 1 2 3 1 chunk +2 lines, -30 lines 0 comments Download
M src/image/SkSurface_Gpu.h View 1 2 3 1 chunk +9 lines, -4 lines 0 comments Download
M src/image/SkSurface_Gpu.cpp View 1 2 3 3 chunks +15 lines, -33 lines 0 comments Download
M src/image/SkSurface_Raster.cpp View 1 2 3 4 5 3 chunks +79 lines, -37 lines 0 comments Download
M src/utils/SkDeferredCanvas.cpp View 1 2 3 4 5 6 7 8 9 3 chunks +2 lines, -8 lines 0 comments Download
M tests/DeferredCanvasTest.cpp View 1 2 3 4 5 8 chunks +116 lines, -131 lines 0 comments Download
M tests/SurfaceTest.cpp View 1 2 3 4 5 6 7 8 9 6 chunks +9 lines, -18 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
Kimmo Kinnunen
depends on https://chromiumcodereview.appspot.com/926843003/
5 years, 10 months ago (2015-02-18 15:09:19 UTC) #1
Kimmo Kinnunen
so this would be ready for review..
5 years, 10 months ago (2015-02-23 14:39:53 UTC) #3
Kimmo Kinnunen
On 2015/02/23 14:39:53, Kimmo Kinnunen wrote: > so this would be ready for review.. .. ...
5 years, 10 months ago (2015-02-23 14:41:40 UTC) #4
reed2
How is this CL related to https://codereview.chromium.org/920513003/ ? There are several core api changes in ...
5 years, 10 months ago (2015-02-24 23:17:19 UTC) #6
Kimmo Kinnunen
5 years, 10 months ago (2015-02-25 13:42:28 UTC) #7
On 2015/02/24 23:17:19, reed2 wrote:
> How is this CL related to https://codereview.chromium.org/920513003/ ?

The CL above makes image filters accept SkImage.

This CL makes the SkCanvas / SkBaseDevice subclass produce SkImage instances out
of devices and bitmaps.

I tried to clarify the commit message.
 
> There are several core api changes in this CL (e.g. pixelref, device,
surface).
> Are all of them required? Do any of them stand on their own w/o the rest of
this
> CL? I'm trying to get my head around the number of changes to the api, but
> having trouble.

I tried to split the patch. Please mention if it is still too big and need to be
split further. Any comment about your feeling which part should be separable
would be appreciated.

(This patch was already in split form, but the split patches did not get reviews
either.. Splitting patches on top of rietveld causes much churn (for me) when
the review comments come seldom..)

The bug lists the patch order:
https://code.google.com/p/skia/issues/detail?id=3388

Powered by Google App Engine
This is Rietveld 408576698