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

Issue 15489004: New API for encoding bitmaps during serialization. (Closed)

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

Description

New API for encoding bitmaps during serialization. This change gives more flexibility to the implementation of EncodeBitmap to prefer calling refEncodedData, doing its own encode, or even doing both and making a decision about which to use. The new function signature also allows the implementation to tell the ordered write buffer whether to store the pixel offset, in the case where the encoded bitmap represents the larger bitmap, or to ignore the pixel offset, in the case where the implementation only encoded the subset that is used. Requires changes to chromium to use the new function signature. (https://codereview.chromium.org/15496006/) SkPicture: New API for EncodeBitmap. SkOrderedReadBuffer: Ifdef'd out addition of reading the offset. SkOrderedWriteBuffer: Never call refEncodedData. Allow the user to call that from their EncodeBitmap function, if desired. This addresses https://code.google.com/p/skia/issues/detail?id=1239 Add in ifdef'd out code to record the offset. PictureTest and PictureRenderer: Implement the new definition of EncodeBitmap. Also update the name of the function to meet coding style guidelines. BUG=https://code.google.com/p/skia/issues/detail?id=1239 R=reed@google.com Committed: https://code.google.com/p/skia/source/detail?r=9226

Patch Set 1 #

Patch Set 2 : Remove dead code, keep original data in rerecord. #

Total comments: 1

Patch Set 3 : Respond to comments. #

Total comments: 4

Patch Set 4 : Respond to comments #

Patch Set 5 : Fix ifdef'd out code #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -64 lines) Patch
M include/core/SkPicture.h View 1 2 3 2 chunks +11 lines, -6 lines 1 comment Download
M src/core/SkBitmap.cpp View 1 2 3 6 chunks +15 lines, -11 lines 0 comments Download
M src/core/SkOrderedReadBuffer.cpp View 1 chunk +6 lines, -0 lines 0 comments Download
M src/core/SkOrderedWriteBuffer.h View 1 2 3 1 chunk +3 lines, -2 lines 0 comments Download
M src/core/SkOrderedWriteBuffer.cpp View 1 2 3 4 4 chunks +28 lines, -37 lines 0 comments Download
M tests/PictureTest.cpp View 1 2 2 chunks +4 lines, -3 lines 0 comments Download
M tools/PictureRenderer.cpp View 1 2 5 chunks +15 lines, -5 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
scroggo
https://codereview.chromium.org/15489004/diff/2001/include/core/SkPicture.h File include/core/SkPicture.h (right): https://codereview.chromium.org/15489004/diff/2001/include/core/SkPicture.h#newcode193 include/core/SkPicture.h:193: typedef SkData* (*EncodeBitmap)(RecordPixelRefOffset*, const SkBitmap&); Mike, you mentioned perhaps ...
7 years, 7 months ago (2013-05-20 19:50:52 UTC) #1
scroggo
7 years, 7 months ago (2013-05-20 19:51:06 UTC) #2
reed1
I like the direction. Is it as clear or clearer to return the "new" offset ...
7 years, 7 months ago (2013-05-20 20:08:56 UTC) #3
scroggo
On 2013/05/20 20:08:56, reed1 wrote: > I like the direction. Is it as clear or ...
7 years, 7 months ago (2013-05-20 22:47:44 UTC) #4
djsollen
https://codereview.chromium.org/15489004/diff/12001/include/core/SkPicture.h File include/core/SkPicture.h (right): https://codereview.chromium.org/15489004/diff/12001/include/core/SkPicture.h#newcode178 include/core/SkPicture.h:178: * @param pixelRefOffset Output parameter, telling the deserializer what ...
7 years, 7 months ago (2013-05-21 14:10:43 UTC) #5
scroggo
https://codereview.chromium.org/15489004/diff/12001/include/core/SkPicture.h File include/core/SkPicture.h (right): https://codereview.chromium.org/15489004/diff/12001/include/core/SkPicture.h#newcode178 include/core/SkPicture.h:178: * @param pixelRefOffset Output parameter, telling the deserializer what ...
7 years, 7 months ago (2013-05-21 16:14:15 UTC) #6
scroggo
On 2013/05/21 16:14:15, scroggo wrote: > https://codereview.chromium.org/15489004/diff/12001/include/core/SkPicture.h > File include/core/SkPicture.h (right): > > https://codereview.chromium.org/15489004/diff/12001/include/core/SkPicture.h#newcode178 > ...
7 years, 7 months ago (2013-05-21 18:15:59 UTC) #7
reed1
lgtm https://codereview.chromium.org/15489004/diff/24002/include/core/SkPicture.h File include/core/SkPicture.h (right): https://codereview.chromium.org/15489004/diff/24002/include/core/SkPicture.h#newcode183 include/core/SkPicture.h:183: typedef SkData* (*EncodeBitmap)(size_t* pixelRefOffset, const SkBitmap& bm); mike ...
7 years, 7 months ago (2013-05-21 18:30:47 UTC) #8
scroggo
7 years, 7 months ago (2013-05-21 20:31:32 UTC) #9
Message was sent while issue was closed.
Committed patchset #5 manually as r9226 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698