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

Issue 381193002: Add alternate SkPicture::clone (Closed)

Created:
6 years, 5 months ago by robertphillips
Modified:
6 years, 5 months ago
Reviewers:
mtklein, reed1
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Project:
skia
Visibility:
Public.

Description

Add alternate SkPicture::clone This adds an alternate version of SkPicture::clone for two reasons: 1) Chromium uses the SkPicture copy constructor to unpack the pictures from the old-style clone interface (and I would like to remove the copy ctor) 2) This is part of the long term plan to wean Chrome off of cloning. Once pictures are thread safe we will switch the new SkPicture::clone call to just return 'this'. From there it is a small step to removing clone entirely. Note that the two versions of clone() is temporary. Once this is landed (and rolled) I will land a Chrome-side patch to remove their use of the old interface (Use new SkPicture::clone interface - https://codereview.chromium.org/380323002/) Committed: https://skia.googlesource.com/skia/+/e372e78223a8ce916d276d6e0420d552fb0267e9

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -2 lines) Patch
M include/core/SkPicture.h View 1 chunk +1 line, -0 lines 1 comment Download
M src/core/SkPicture.cpp View 2 chunks +68 lines, -2 lines 2 comments Download

Messages

Total messages: 9 (0 generated)
robertphillips
6 years, 5 months ago (2014-07-10 18:56:08 UTC) #1
reed1
lgtm https://codereview.chromium.org/381193002/diff/1/include/core/SkPicture.h File include/core/SkPicture.h (right): https://codereview.chromium.org/381193002/diff/1/include/core/SkPicture.h#newcode125 include/core/SkPicture.h:125: void clone(SkPicture* pictures, int count) const; What does ...
6 years, 5 months ago (2014-07-10 19:10:06 UTC) #2
reed1
On 2014/07/10 19:10:06, reed1 wrote: > lgtm > > https://codereview.chromium.org/381193002/diff/1/include/core/SkPicture.h > File include/core/SkPicture.h (right): > ...
6 years, 5 months ago (2014-07-10 19:11:36 UTC) #3
mtklein
https://codereview.chromium.org/381193002/diff/1/src/core/SkPicture.cpp File src/core/SkPicture.cpp (right): https://codereview.chromium.org/381193002/diff/1/src/core/SkPicture.cpp#newcode205 src/core/SkPicture.cpp:205: Can we share somehow the code below here? Looks ...
6 years, 5 months ago (2014-07-10 19:14:01 UTC) #4
mtklein
On 2014/07/10 19:14:01, mtklein wrote: > https://codereview.chromium.org/381193002/diff/1/src/core/SkPicture.cpp > File src/core/SkPicture.cpp (right): > > https://codereview.chromium.org/381193002/diff/1/src/core/SkPicture.cpp#newcode205 > ...
6 years, 5 months ago (2014-07-10 19:22:27 UTC) #5
robertphillips
The CQ bit was checked by robertphillips@google.com
6 years, 5 months ago (2014-07-10 19:24:08 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/robertphillips@google.com/381193002/1
6 years, 5 months ago (2014-07-10 19:24:56 UTC) #7
commit-bot: I haz the power
Change committed as e372e78223a8ce916d276d6e0420d552fb0267e9
6 years, 5 months ago (2014-07-10 21:11:02 UTC) #8
robertphillips
6 years, 5 months ago (2014-07-11 13:45:15 UTC) #9
Message was sent while issue was closed.
A revert of this CL has been created in
https://codereview.chromium.org/386933004/ by robertphillips@google.com.

The reason for reverting is: Going to try to just remove the many-at-once clone
interface.

Powered by Google App Engine
This is Rietveld 408576698