diff options
author | Leon Scroggins <scroggo@google.com> | 2017-12-05 15:38:14 +0000 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2017-12-05 15:38:22 +0000 |
commit | 1793e7bb46c1f9d430c1a699a1c3d3168159b659 (patch) | |
tree | 22362338d6b7fec5dc58ff9e6aba05b3e55997b1 /src/codec/SkCodec.cpp | |
parent | a2e7f5ae10e3cf0514d69edc12c9ccbca39ccb17 (diff) |
Revert "Hide SkEncodedInfo"
This reverts commit c6f7a4ffa9522159efc42f7c948bba5e66bb8844.
Reason for revert: Causing differences in Gold, stemming from the fact that this changes the recommended SkImageInfo for 16 bits-per-component PNG from N32 to F16.
- an F16 bitmap already png-encodes to a 16 bits-per-component PNG, but it does not encode a linear colorspace (possibly a bug?). when we decode this PNG using getInfo(), it fails because it has an F16 color type and non-linear colorspace. (In the encode-srgb-png gm, this results in blank results for F16.) We could correct this on the encoder side, but it seems possible that a 16 bits-per-component PNG could be encoded with a different color space. In that case, we'd want SkCodec to recommend F16/SRGBLinear, but I think we'd want the SkCodec to store the encoded SkColorSpace so that we can Xform between the two. Currently SkCodec only stores one color space, so that will require a refactor.
- When decoding 16-bits-per-component PNGs, we are now decoding them to F16. This shows differences in Gold. The srgb/gpu results now look more like F16. I think this is fine.
Original change's description:
> Hide SkEncodedInfo
>
> Bug: skia:7353
> Bug: skia:6839
>
> This contains information that is not necessary for clients to know. The
> Color enum tells the number of components in the input, but this is only
> interesting internally (to the SkSwizzler).
>
> Similarly, the Alpha enum differs from SkAlphaType in that it has
> kBinary instead of kPremul. This is useful information only internally
> for determining whether the SkColorSpaceXform needs to premultiply.
>
> The bitsPerComponent is potentially useful for a client; Android (in
> SkAndroidCodec) uses it to determine the SkColorType. Rather than
> exposing bitsPerComponent, use it to make the same decision that Android
> would have made - 16 bits per component means to set the info to F16. Add
> a test that computeOutputColorType behaves as expected.
>
> Switch conversionSupported to use an SkColorType, which is enough info.
>
> Replace the SkEncodedInfo::Alpha field on SkCodec::FrameInfo with an
> SkAlphaType.
>
> SkCodec still needs an SkEncodedInfo, so move its header (which is
> already not SK_API) to include/private.
>
> Change-Id: Ie2cf11339bf999ebfd4390c0f448f7edd6feabda
> Reviewed-on: https://skia-review.googlesource.com/79260
> Reviewed-by: Mike Reed <reed@google.com>
> Reviewed-by: Mike Klein <mtklein@chromium.org>
> Commit-Queue: Leon Scroggins <scroggo@google.com>
TBR=mtklein@chromium.org,scroggo@google.com,reed@google.com
Change-Id: I0c5dd1461e1b70d1e55349a8e7ee6b029c3f556e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:7353, skia:6839
Reviewed-on: https://skia-review.googlesource.com/80660
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Diffstat (limited to 'src/codec/SkCodec.cpp')
-rw-r--r-- | src/codec/SkCodec.cpp | 16 |
1 files changed, 7 insertions, 9 deletions
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp index 7de083db3c..a0f1fb2706 100644 --- a/src/codec/SkCodec.cpp +++ b/src/codec/SkCodec.cpp @@ -158,7 +158,7 @@ SkCodec::SkCodec(const SkEncodedInfo& info, const SkImageInfo& imageInfo, SkCodec::~SkCodec() {} -bool SkCodec::conversionSupported(const SkImageInfo& dst, SkColorType srcColor, +bool SkCodec::conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color srcColor, bool srcIsOpaque, const SkColorSpace* srcCS) const { if (!valid_alpha(dst.alphaType(), srcIsOpaque)) { return false; @@ -173,7 +173,7 @@ bool SkCodec::conversionSupported(const SkImageInfo& dst, SkColorType srcColor, case kRGB_565_SkColorType: return srcIsOpaque; case kGray_8_SkColorType: - return kGray_8_SkColorType == srcColor && srcIsOpaque && + return SkEncodedInfo::kGray_Color == srcColor && srcIsOpaque && !needs_color_xform(dst, srcCS, false); case kAlpha_8_SkColorType: // conceptually we can convert anything into alpha_8, but we haven't actually coded @@ -224,7 +224,7 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, const Options& options) { const int index = options.fFrameIndex; if (0 == index) { - if (!this->conversionSupported(info, fSrcInfo.colorType(), fEncodedInfo.opaque(), + if (!this->conversionSupported(info, fEncodedInfo.color(), fEncodedInfo.opaque(), fSrcInfo.colorSpace()) || !this->initializeColorXform(info, fEncodedInfo.alpha(), options.fPremulBehavior)) { @@ -253,7 +253,7 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, const auto* frame = frameHolder->getFrame(index); SkASSERT(frame); - if (!this->conversionSupported(info, fSrcInfo.colorType(), !frame->hasAlpha(), + if (!this->conversionSupported(info, fEncodedInfo.color(), !frame->hasAlpha(), fSrcInfo.colorSpace())) { return kInvalidConversion; } @@ -297,7 +297,9 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, } } - return this->initializeColorXform(info, frame->reportedAlpha(), options.fPremulBehavior) + FrameInfo frameInfo; + SkAssertResult(this->getFrameInfo(index, &frameInfo)); + return this->initializeColorXform(info, frameInfo.fAlpha, options.fPremulBehavior) ? kSuccess : kInvalidConversion; } @@ -628,10 +630,6 @@ bool SkCodec::initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Al if (!this->usesColorXform()) { return true; } - // FIXME: In SkWebpCodec, if a frame is blending with a prior frame, we don't need - // a colorXform to do a color correct premul, since the blend step will handle - // premultiplication. But there is no way to know whether we need to blend from - // inside this call. bool needsColorCorrectPremul = needs_premul(dstInfo.alphaType(), encodedAlpha) && SkTransferFunctionBehavior::kRespect == premulBehavior; if (needs_color_xform(dstInfo, fSrcInfo.colorSpace(), needsColorCorrectPremul)) { |