Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2527833002:
Implement SkEncodeImage() (Closed)
|
Created:
4 years ago by hal.canary Modified:
3 years, 11 months ago 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. |
DescriptionImplement 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 #
Messages
Total messages: 45 (26 generated)
The CQ bit was checked by halcanary@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by halcanary@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
halcanary@google.com changed reviewers: + scroggo@google.com, thestig@chromium.org
Background: Skia's 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 install a encoding function into skia. I am not sure that I've done this in the best or most Chromeish way. By the way, scroggo@ is looking at unifying Skia and Chrome's image decoders. Why not eventually do the same for image encoding?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/11/28 23:18:20, Hal Canary wrote: > Background: Skia's 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 install a > encoding function into skia. I think this info belongs in the commit message. > > I am not sure that I've done this in the best or most Chromeish way. > > By the way, scroggo@ is looking at unifying Skia and Chrome's image decoders. > Why not eventually do the same for image encoding? That has been discussed, but is not a top priority right now. 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", Once this CL lands, can we delete this file? https://codereview.chromium.org/2527833002/diff/20001/skia/ext/skia_encode_im... File skia/ext/skia_encode_image.h (right): https://codereview.chromium.org/2527833002/diff/20001/skia/ext/skia_encode_im... skia/ext/skia_encode_image.h:21: SK_API void SetEmageEncoder(ImageEncoder); Image*? https://codereview.chromium.org/2527833002/diff/20001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/20001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:19: SkBitmap bitmap; This is only used in the PNG case - why not move it there? https://codereview.chromium.org/2527833002/diff/20001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:27: return gfx::JPEGCodec::Encode((const unsigned char*)pixmap.addr(), reinterpret_cast? https://codereview.chromium.org/2527833002/diff/20001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:30: (int)pixmap.rowBytes(), quality, &buffer) && static_cast?
Description was changed from ========== 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. Also: Revert "SK_SUPPORT_LEGACY_IMAGE_ENCODER_CLASS" This reverts commit 1430e85be96647271e370c92c4940244dc149e76. BUG= ========== to ========== 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 install a encoding function into skia. Also: Revert "SK_SUPPORT_LEGACY_IMAGE_ENCODER_CLASS" This reverts commit 1430e85be96647271e370c92c4940244dc149e76. BUG=chromium:616763 ==========
halcanary@google.com changed reviewers: + dcheng@chromium.org
halcanary@google.com changed reviewers: - dcheng@chromium.org
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 CL lands, can we delete this file? Yes. It can be deleted as part of the later cleanup CL. https://codereview.chromium.org/2527833002/diff/20001/skia/ext/skia_encode_im... File skia/ext/skia_encode_image.h (right): https://codereview.chromium.org/2527833002/diff/20001/skia/ext/skia_encode_im... skia/ext/skia_encode_image.h:21: SK_API void SetEmageEncoder(ImageEncoder); On 2016/11/29 14:52:54, scroggo wrote: > Image*? Wow. I cut-and-pasted that typo twice without seeing it. This is what I get for working while on holiday. Done. https://codereview.chromium.org/2527833002/diff/20001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/20001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:19: SkBitmap bitmap; On 2016/11/29 14:52:54, scroggo wrote: > This is only used in the PNG case - why not move it there? Done. https://codereview.chromium.org/2527833002/diff/20001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:27: return gfx::JPEGCodec::Encode((const unsigned char*)pixmap.addr(), On 2016/11/29 14:52:54, scroggo wrote: > reinterpret_cast? Done. https://codereview.chromium.org/2527833002/diff/20001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:30: (int)pixmap.rowBytes(), quality, &buffer) && On 2016/11/29 14:52:54, scroggo wrote: > static_cast? Done.
halcanary@google.com changed reviewers: + dcheng@chromium.org
adding dcheng@ to okay at ui/gfx/codec.
lgtm https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:22: kOpaque_SkAlphaType != pixmap.alphaType())) { Why not support unpremul? It looks like PNGCodec::Encode expects premultiplication (it goes through this unpremultiply step [1]), but we *could* support it. I guess none of your input data is unpremul? (Or is there a TODO?) [1] https://cs.chromium.org/chromium/src/ui/gfx/skia_util.cc?cl=GROK&gsn=ConvertS...
https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:22: kOpaque_SkAlphaType != pixmap.alphaType())) { On 2016/11/29 20:15:24, scroggo wrote: > Why not support unpremul? It looks like PNGCodec::Encode expects > premultiplication (it goes through this unpremultiply step [1]), but we *could* > support it. I guess none of your input data is unpremul? (Or is there a TODO?) > > [1] > https://cs.chromium.org/chromium/src/ui/gfx/skia_util.cc?cl=GROK&gsn=ConvertS... Fixing gfx::PNGCodec::Encode() to allow unpremuliplied input is outside of the scope of what I am doing.
https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:34: dst->write(&buffer[0], buffer.size()); Is it guaranteed that |buffer.size()| > 0 here and below? (It's not clear to me; it seems like the answer is probably "Yes" but it's also not guaranteed) https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:48: // FIXME: What is the chromium version of SkOnce? There is no Chromium version: the base::Lock here doesn't do anything, since it's a stack variable. How is the GPU benchmarking extension installed? Is this something we expect to be more widely used in the future? Usually, writing code like this in Chromium would be a bit of an anti-pattern, since initialization is usually supposed to be better coordinated.
https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:34: dst->write(&buffer[0], buffer.size()); On 2016/11/29 23:37:43, dcheng wrote: > Is it guaranteed that |buffer.size()| > 0 here and below? > > (It's not clear to me; it seems like the answer is probably "Yes" but it's also > not guaranteed) Done. https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:48: // FIXME: What is the chromium version of SkOnce? On 2016/11/29 23:37:43, dcheng wrote: > There is no Chromium version: the base::Lock here doesn't do anything, since > it's a stack variable. d'oh! That's what I meant to do. > How is the GPU benchmarking extension installed? Is this something we expect to > be more widely used in the future? Usually, writing code like this in Chromium > would be a bit of an anti-pattern, since initialization is usually supposed to > be better coordinated. I am using the benchmarking extension to test printing to XPS. In the future, we will need to both initialize image encoding and set up SkXPS to work inside the sandbox when the printing process is started.
https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:34: dst->write(&buffer[0], buffer.size()); On 2016/11/30 18:24:06, Hal Canary wrote: > On 2016/11/29 23:37:43, dcheng wrote: > > Is it guaranteed that |buffer.size()| > 0 here and below? > > > > (It's not clear to me; it seems like the answer is probably "Yes" but it's > also > > not guaranteed) > > Done. Rather than the conditional, I think it's probably better to just write(buffer.data(), buffer.size()) - C++11 guarantee that data() will always return *something*: it's just not dereferencable if the container is empty. https://codereview.chromium.org/2527833002/diff/60001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/60001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:51: static base::Lock lock; Unfortunately, this doesn't work. Our lock implementation is not considered a POD, thus the C++ style guide does not permit this: https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables Furthermore, we don't enable thread-safe statics in Chromium, so the initialization of base::Lock itself is racy. It's OK to have some sort of DCHECK static bool, but I do not think we should have it in the general path: initialization should be well-defined, and a DCHECK should be sufficient to guard.
On 2016/12/01 06:22:38, dcheng wrote: > https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... > File ui/gfx/codec/install_skia_codec.cc (right): > > https://codereview.chromium.org/2527833002/diff/40001/ui/gfx/codec/install_sk... > ui/gfx/codec/install_skia_codec.cc:34: dst->write(&buffer[0], buffer.size()); > On 2016/11/30 18:24:06, Hal Canary wrote: > > On 2016/11/29 23:37:43, dcheng wrote: > > > Is it guaranteed that |buffer.size()| > 0 here and below? > > > > > > (It's not clear to me; it seems like the answer is probably "Yes" but it's > > also > > > not guaranteed) > > > > Done. > > Rather than the conditional, I think it's probably better to just > write(buffer.data(), buffer.size()) - C++11 guarantee that data() will always > return *something*: it's just not dereferencable if the container is empty. > done.
https://codereview.chromium.org/2527833002/diff/60001/ui/gfx/codec/install_sk... File ui/gfx/codec/install_skia_codec.cc (right): https://codereview.chromium.org/2527833002/diff/60001/ui/gfx/codec/install_sk... ui/gfx/codec/install_skia_codec.cc:51: static base::Lock lock; On 2016/12/01 06:22:38, dcheng wrote: > Unfortunately, this doesn't work. > > Our lock implementation is not considered a POD, thus the C++ style guide does > not permit this: > https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables > > Furthermore, we don't enable thread-safe statics in Chromium, so the > initialization of base::Lock itself is racy. > > It's OK to have some sort of DCHECK static bool, but I do not think we should > have it in the general path: initialization should be well-defined, and a DCHECK > should be sufficient to guard. Okay. I have removed this for now.
https://codereview.chromium.org/2527833002/diff/100001/skia/ext/skia_encode_i... File skia/ext/skia_encode_image.h (right): https://codereview.chromium.org/2527833002/diff/100001/skia/ext/skia_encode_i... skia/ext/skia_encode_image.h:19: int quality); Prefer using A = B over typedef B A in new code. Also, I'd suffix this with "Function" and not include the pointer explicitly, so it's clear from the function signature that we're passing a function pointer, i.e. something like: SK_API void SetImageEncoder(ImageEncoderFunction*); https://codereview.chromium.org/2527833002/diff/100001/ui/gfx/codec/install_s... File ui/gfx/codec/install_skia_codec.h (right): https://codereview.chromium.org/2527833002/diff/100001/ui/gfx/codec/install_s... ui/gfx/codec/install_skia_codec.h:1: // Copyright (c) 2016 The Chromium Authors. All rights reserved. Nit: rename this file to something like skia_image_encoder_adapter. https://codereview.chromium.org/2527833002/diff/100001/ui/gfx/codec/install_s... ui/gfx/codec/install_skia_codec.h:18: GFX_EXPORT bool EncodeImage(SkWStream* dst, Nit: call this something less generic, since it's an adapter to help Skia use the gfx image encoding functions, instead of a generic EncodeImage function.
The CQ bit was checked by halcanary@google.com to run a CQ dry run
https://codereview.chromium.org/2527833002/diff/100001/skia/ext/skia_encode_i... File skia/ext/skia_encode_image.h (right): https://codereview.chromium.org/2527833002/diff/100001/skia/ext/skia_encode_i... skia/ext/skia_encode_image.h:19: int quality); On 2016/12/20 19:40:47, dcheng wrote: > Prefer using A = B over typedef B A in new code. > > Also, I'd suffix this with "Function" and not include the pointer explicitly, so > it's clear from the function signature that we're passing a function pointer, > i.e. something like: > SK_API void SetImageEncoder(ImageEncoderFunction*); Done. https://codereview.chromium.org/2527833002/diff/100001/ui/gfx/codec/install_s... File ui/gfx/codec/install_skia_codec.h (right): https://codereview.chromium.org/2527833002/diff/100001/ui/gfx/codec/install_s... ui/gfx/codec/install_skia_codec.h:1: // Copyright (c) 2016 The Chromium Authors. All rights reserved. On 2016/12/20 19:40:47, dcheng wrote: > Nit: rename this file to something like skia_image_encoder_adapter. Done. https://codereview.chromium.org/2527833002/diff/100001/ui/gfx/codec/install_s... ui/gfx/codec/install_skia_codec.h:18: GFX_EXPORT bool EncodeImage(SkWStream* dst, On 2016/12/20 19:40:47, dcheng wrote: > Nit: call this something less generic, since it's an adapter to help Skia use > the gfx image encoding functions, instead of a generic EncodeImage function. Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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 install a encoding function into skia. Also: Revert "SK_SUPPORT_LEGACY_IMAGE_ENCODER_CLASS" This reverts commit 1430e85be96647271e370c92c4940244dc149e76. BUG=chromium:616763 ========== to ========== 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 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by halcanary@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com, dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2527833002/#ps140001 (title: "hold off on calling new functions")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for skia/BUILD.gn:
While running git apply --index -p1;
error: patch failed: skia/BUILD.gn:207
error: skia/BUILD.gn: patch does not apply
Patch: skia/BUILD.gn
Index: skia/BUILD.gn
diff --git a/skia/BUILD.gn b/skia/BUILD.gn
index
da4eb2e7a95f7048164ef60afadf261bebf6f5c9..3c30d588a02867ca0ab6c5805f156d1a1adac7e6
100644
--- a/skia/BUILD.gn
+++ b/skia/BUILD.gn
@@ -207,6 +207,8 @@ component("skia") {
"ext/platform_device_mac.cc",
"ext/platform_device_win.cc",
"ext/recursive_gaussian_convolution.cc",
+ "ext/skia_encode_image.cc",
+ "ext/skia_encode_image.h",
"ext/skia_histogram.cc",
"ext/skia_memory_dump_provider.cc",
"ext/skia_trace_memory_dump_impl.cc",
@@ -247,7 +249,6 @@ component("skia") {
"//third_party/skia/src/ports/SkFontMgr_android_parser.cpp",
"//third_party/skia/src/ports/SkFontMgr_win_dw.cpp",
"//third_party/skia/src/ports/SkGlobalInitialization_default.cpp",
- "//third_party/skia/src/ports/SkImageEncoder_none.cpp",
"//third_party/skia/src/ports/SkImageGenerator_none.cpp",
"//third_party/skia/src/ports/SkOSFile_posix.cpp",
"//third_party/skia/src/ports/SkOSFile_stdio.cpp",
The CQ bit was checked by halcanary@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com, dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2527833002/#ps160001 (title: "rebase again")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1483542405603200,
"parent_rev": "42e74a0edb43fe93b6eea6ab6edfdcc97c988159", "commit_rev":
"956174fbfcb7a391c992b9613ecb678956158bdc"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 Review-Url: https://codereview.chromium.org/2527833002 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== 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 Review-Url: https://codereview.chromium.org/2527833002 ========== to ========== 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} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/05683872522a0e03b9de2243a1f4f30a5e6d3dca Cr-Commit-Position: refs/heads/master@{#441395} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
