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

Issue 22591002: Adding 2 functions to the Skia API (Closed)

Created:
7 years, 4 months ago by sugoi1
Modified:
7 years, 4 months ago
Reviewers:
scroggo, djsollen, reed1
CC:
skia-review_googlegroups.com
Visibility:
Public.

Description

Adding 2 functions to the Skia API I need wrappers for both SkOrderedReadBuffer and SkOrderedWriteBuffer inside Chromium in order to do the IPC serialization of Skia SkImageFilter objects. See https://codereview.chromium.org/21271002/ BUG= Committed: http://code.google.com/p/skia/source/detail?r=10642

Patch Set 1 #

Patch Set 2 : New serialization functions #

Total comments: 3

Patch Set 3 : ImageFilter only api with generalized local functions #

Patch Set 4 : Oops, missing semi-colon added to header file #

Patch Set 5 : Back to generic SkFlattenable version #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -0 lines) Patch
M gyp/core.gypi View 1 2 chunks +2 lines, -0 lines 0 comments Download
A include/core/SkFlattenableSerialization.h View 1 2 3 4 1 chunk +20 lines, -0 lines 0 comments Download
A src/core/SkFlattenableSerialization.cpp View 1 2 3 4 1 chunk +28 lines, -0 lines 0 comments Download

Messages

Total messages: 28 (0 generated)
sugoi1
7 years, 4 months ago (2013-08-07 14:43:18 UTC) #1
djsollen
We shouldn't have SK_API headers in our src directory. If this is going to be ...
7 years, 4 months ago (2013-08-07 15:07:47 UTC) #2
sugoi1
On 2013/08/07 15:07:47, djsollen wrote: > We shouldn't have SK_API headers in our src directory. ...
7 years, 4 months ago (2013-08-07 15:09:37 UTC) #3
reed1
Do you need access to all of the particulars of orderedwritebuffer, or can you write ...
7 years, 4 months ago (2013-08-07 15:29:38 UTC) #4
sugoi1
On 2013/08/07 15:29:38, reed1 wrote: > Do you need access to all of the particulars ...
7 years, 4 months ago (2013-08-07 15:46:32 UTC) #5
reed1
Can you show some of the call-sites, and how they might use this?
7 years, 4 months ago (2013-08-07 15:47:52 UTC) #6
sugoi1
On 2013/08/07 15:47:52, reed1 wrote: > Can you show some of the call-sites, and how ...
7 years, 4 months ago (2013-08-07 15:49:22 UTC) #7
sugoi1
On 2013/08/07 15:49:22, sugoi1 wrote: > On 2013/08/07 15:47:52, reed1 wrote: > > Can you ...
7 years, 4 months ago (2013-08-07 15:50:02 UTC) #8
reed1
Thanks for the links. I wonder if we can make it even simpler, given the ...
7 years, 4 months ago (2013-08-07 17:07:23 UTC) #9
sugoi1
On 2013/08/07 17:07:23, reed1 wrote: > Thanks for the links. > > I wonder if ...
7 years, 4 months ago (2013-08-07 17:24:06 UTC) #10
reed1
On 2013/08/07 17:24:06, sugoi1 wrote: > On 2013/08/07 17:07:23, reed1 wrote: > > Thanks for ...
7 years, 4 months ago (2013-08-07 17:41:03 UTC) #11
sugoi1
On 2013/08/07 17:41:03, reed1 wrote: > The orderwritebuffer is 2-pass, so it is already buffering ...
7 years, 4 months ago (2013-08-07 17:59:20 UTC) #12
reed1
On 2013/08/07 17:59:20, sugoi1 wrote: > On 2013/08/07 17:41:03, reed1 wrote: > > The orderwritebuffer ...
7 years, 4 months ago (2013-08-07 18:04:08 UTC) #13
sugoi1
On 2013/08/07 18:04:08, reed1 wrote: > On 2013/08/07 17:59:20, sugoi1 wrote: > > On 2013/08/07 ...
7 years, 4 months ago (2013-08-07 18:09:01 UTC) #14
sugoi1
Here's the new API code. It causes an extra copy of the data, but completely ...
7 years, 4 months ago (2013-08-07 19:21:31 UTC) #15
reed1
https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h File include/core/SkFlattenableSerialization.h (right): https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h#newcode17 include/core/SkFlattenableSerialization.h:17: SK_API SkData* SkSerializeFlattenable(SkFlattenable*); Lets keep these in parallel. Why ...
7 years, 4 months ago (2013-08-07 19:32:56 UTC) #16
scroggo
https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h File include/core/SkFlattenableSerialization.h (right): https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h#newcode17 include/core/SkFlattenableSerialization.h:17: SK_API SkData* SkSerializeFlattenable(SkFlattenable*); Please add some comments. https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h#newcode18 include/core/SkFlattenableSerialization.h:18: ...
7 years, 4 months ago (2013-08-07 19:34:33 UTC) #17
sugoi1
On 2013/08/07 19:32:56, reed1 wrote: > https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h > File include/core/SkFlattenableSerialization.h (right): > > https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h#newcode17 > ...
7 years, 4 months ago (2013-08-07 19:35:57 UTC) #18
sugoi1
On 2013/08/07 19:34:33, scroggo wrote: > https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h > File include/core/SkFlattenableSerialization.h (right): > > https://codereview.chromium.org/22591002/diff/17001/include/core/SkFlattenableSerialization.h#newcode17 > ...
7 years, 4 months ago (2013-08-07 19:41:04 UTC) #19
sugoi1
I changed the API so that it's SkImageFilter only, but added utility functions to make ...
7 years, 4 months ago (2013-08-07 19:56:24 UTC) #20
reed1
The templated version of readFlattenable is just a convenience wrapper around SkFlattenable* readFlattenable() The reader ...
7 years, 4 months ago (2013-08-07 20:27:00 UTC) #21
reed1
I think either of these are safe and acceptable: SerializeImageFilter DeserializeImageFilter or SerializeFlattenable DeserializeFlattenable If ...
7 years, 4 months ago (2013-08-07 20:28:53 UTC) #22
sugoi1
On 2013/08/07 20:27:00, reed1 wrote: > The templated version of readFlattenable is just a convenience ...
7 years, 4 months ago (2013-08-08 11:28:08 UTC) #23
sugoi1
On 2013/08/07 20:28:53, reed1 wrote: > I think either of these are safe and acceptable: ...
7 years, 4 months ago (2013-08-08 11:28:57 UTC) #24
sugoi1
Here's the new version using SkFlattenable.
7 years, 4 months ago (2013-08-08 11:30:51 UTC) #25
scroggo
On 2013/08/08 11:30:51, sugoi1 wrote: > Here's the new version using SkFlattenable. lgtm
7 years, 4 months ago (2013-08-08 18:13:23 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/sugoi@chromium.org/22591002/33001
7 years, 4 months ago (2013-08-08 18:22:16 UTC) #27
commit-bot: I haz the power
7 years, 4 months ago (2013-08-08 19:20:08 UTC) #28
Message was sent while issue was closed.
Change committed as 10642

Powered by Google App Engine
This is Rietveld 408576698