Chromium Code Reviews
Help | Chromium Project | Sign in
(59)

Issue 2527833002: Implement SkEncodeImage() (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months ago by hal.canary
Modified:
2 months, 3 weeks ago
Reviewers:
Lei Zhang, scroggo, dcheng
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, extensions-reviews_chromium.org, jam, darin-cc_chromium.org, chromium-apps-reviews_chromium.org, piman+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement SkEncodeImage() This change replaces the implementation in SkImageEncoder_none.cpp with an implementation that allows the layer above to define an image encoding function. Background: Skia's (experimental) XPS backend needs a PNG encoder. Skia for Chromium does not compile its PNG encoder, but there is one available in Chrome's gfx component. To make layering work right, I allow the layer above skia (gfx) to define an encoding function that can be installed into skia. TODO: initialize the function pointer in printing process. Also: Revert "SK_SUPPORT_LEGACY_IMAGE_ENCODER_CLASS" This reverts commit 1430e85be96647271e370c92c4940244dc149e76. BUG=chromium:616763 Committed: https://crrev.com/05683872522a0e03b9de2243a1f4f30a5e6d3dca Cr-Commit-Position: refs/heads/master@{#441395}

Patch Set 1 #

Patch Set 2 : 2016-11-28 (Monday) 17:33:22 EST #

Total comments: 10

Patch Set 3 : refactor install_skia_codec #

Total comments: 7

Patch Set 4 : 2016-11-30 (Wednesday) 12:18:52 EST #

Total comments: 2

Patch Set 5 : simpleify CL. punt on initialization #

Patch Set 6 : remove size check #

Total comments: 6

Patch Set 7 : more dcheng changes #

Patch Set 8 : hold off on calling new functions #

Patch Set 9 : rebase again #

Unified diffs Side-by-side diffs Delta from patch set Stats (+117 lines, -5 lines) Patch
M skia/BUILD.gn View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download
M skia/config/SkUserConfig.h View 1 2 3 4 5 6 7 1 chunk +0 lines, -4 lines 0 comments Download
A skia/ext/skia_encode_image.h View 1 2 3 4 5 6 1 chunk +25 lines, -0 lines 0 comments Download
A skia/ext/skia_encode_image.cc View 1 2 3 4 5 6 1 chunk +21 lines, -0 lines 0 comments Download
M ui/gfx/BUILD.gn View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
A ui/gfx/codec/skia_image_encoder_adapter.h View 1 2 3 4 5 6 1 chunk +25 lines, -0 lines 0 comments Download
A ui/gfx/codec/skia_image_encoder_adapter.cc View 1 2 3 4 5 6 1 chunk +42 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 45 (26 generated)
hal.canary
Background: Skia's XPS backend needs a PNG encoder. Skia for Chromium does not compile its ...
4 months ago (2016-11-28 23:18:20 UTC) #8
scroggo
On 2016/11/28 23:18:20, Hal Canary wrote: > Background: Skia's XPS backend needs a PNG encoder. ...
4 months ago (2016-11-29 14:52:54 UTC) #11
hal.canary
https://codereview.chromium.org/2527833002/diff/20001/skia/BUILD.gn File skia/BUILD.gn (left): https://codereview.chromium.org/2527833002/diff/20001/skia/BUILD.gn#oldcode261 skia/BUILD.gn:261: "//third_party/skia/src/ports/SkImageEncoder_none.cpp", On 2016/11/29 14:52:53, scroggo wrote: > Once this ...
4 months ago (2016-11-29 18:24:27 UTC) #15
hal.canary
adding dcheng@ to okay at ui/gfx/codec.
4 months ago (2016-11-29 18:25:39 UTC) #17
scroggo
lgtm https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc#newcode22 ui/gfx/codec/install_skia_codec.cc:22: kOpaque_SkAlphaType != pixmap.alphaType())) { Why not support unpremul? ...
4 months ago (2016-11-29 20:15:24 UTC) #18
hal.canary
https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc#newcode22 ui/gfx/codec/install_skia_codec.cc:22: kOpaque_SkAlphaType != pixmap.alphaType())) { On 2016/11/29 20:15:24, scroggo wrote: ...
4 months ago (2016-11-29 20:27:04 UTC) #19
dcheng
https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc#newcode34 ui/gfx/codec/install_skia_codec.cc:34: dst->write(&buffer[0], buffer.size()); Is it guaranteed that |buffer.size()| > 0 ...
4 months ago (2016-11-29 23:37:43 UTC) #20
hal.canary
https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc#newcode34 ui/gfx/codec/install_skia_codec.cc:34: dst->write(&buffer[0], buffer.size()); On 2016/11/29 23:37:43, dcheng wrote: > Is ...
3 months, 4 weeks ago (2016-11-30 18:24:07 UTC) #21
dcheng
https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc#newcode34 ui/gfx/codec/install_skia_codec.cc:34: dst->write(&buffer[0], buffer.size()); On 2016/11/30 18:24:06, Hal Canary wrote: > ...
3 months, 4 weeks ago (2016-12-01 06:22:38 UTC) #22
hal.canary
On 2016/12/01 06:22:38, dcheng wrote: > https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc > File ui/gfx/codec/install_skia_codec.cc (right): > > https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_skia_codec.cc#newcode34 > ...
3 months, 1 week ago (2016-12-20 16:51:00 UTC) #23
hal.canary
https://codereview.chromium.org/2527833002/diff/60001/ui/gfx/codec/install_skia_codec.cc File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/60001/ui/gfx/codec/install_skia_codec.cc#newcode51 ui/gfx/codec/install_skia_codec.cc:51: static base::Lock lock; On 2016/12/01 06:22:38, dcheng wrote: > ...
3 months, 1 week ago (2016-12-20 16:51:13 UTC) #24
dcheng
https://codereview.chromium.org/2527833002/diff/100001/skia/ext/skia_encode_image.h File skia/ext/skia_encode_image.h (right): https://codereview.chromium.org/2527833002/diff/100001/skia/ext/skia_encode_image.h#newcode19 skia/ext/skia_encode_image.h:19: int quality); Prefer using A = B over typedef ...
3 months, 1 week ago (2016-12-20 19:40:47 UTC) #25
hal.canary
https://codereview.chromium.org/2527833002/diff/100001/skia/ext/skia_encode_image.h File skia/ext/skia_encode_image.h (right): https://codereview.chromium.org/2527833002/diff/100001/skia/ext/skia_encode_image.h#newcode19 skia/ext/skia_encode_image.h:19: int quality); On 2016/12/20 19:40:47, dcheng wrote: > Prefer ...
3 months, 1 week ago (2016-12-20 20:45:09 UTC) #27
dcheng
lgtm
3 months, 1 week ago (2016-12-21 01:55:37 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2527833002/140001
2 months, 3 weeks ago (2017-01-03 18:09:40 UTC) #35
commit-bot: I haz the power
Failed to apply patch for skia/BUILD.gn: While running git apply --index -p1; error: patch failed: ...
2 months, 3 weeks ago (2017-01-03 22:24:55 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2527833002/160001
2 months, 3 weeks ago (2017-01-04 15:07:02 UTC) #40
commit-bot: I haz the power
Committed patchset #9 (id:160001)
2 months, 3 weeks ago (2017-01-04 16:45:48 UTC) #43
commit-bot: I haz the power
2 months, 3 weeks ago (2017-01-04 16:48:04 UTC) #45
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/05683872522a0e03b9de2243a1f4f30a5e6d3dca
Cr-Commit-Position: refs/heads/master@{#441395}
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46