This is a reland of fc4fdc5b25
Original change's description:
> SkAndroidCodec: Support decoding all frames
>
> Bug: b/160984428
> Bug: b/163595585
>
> Add support to SkAndroidCodec for decoding all frames with an
> fSampleSize, so that an entire animation can be decoded to a smaller
> size.
>
> dm/:
> - Test scaled + animated decodes
>
> SkAndroidCodec:
> - Make AndroidOptions inherit from SkCodec::Options. This allows
> SkAndroidCodec to use fFrameIndex. (It also combines the two versions
> of fSubset, which is now const for both.)
> - Respect fFrameIndex, and call SkCodec::handleFrameIndex to decode
> the required frame.
> - Disallow decoding with kRespect + fFrameIndex > 0 if there is a
> non-default orientation. As currently written (except without
> disabling this combination), SkPixmapPriv::Orient would draw the new
> portion of the frame on top of uninitialized pixels, instead of the
> prior frame. This could be fixed by
> - If SkAndroidCodec needs to decode the required frame, it could do so
> without applying the orientation, then decode fFrameIndex, and then
> apply the orientation.
> - If the client provided the required frame, SkAndroidCodec would need
> to un-apply the orientation to get the proper starting state, then
> decode and apply.
> I think it is simpler to force the client to handle the orientation
> externally.
>
> SkCodec:
> - Allow SkAndroidCodec to call its private method handleFrameIndex. This
> method handles decoding a required frame, if necessary. When called by
> SkAndroidCodec, it now uses the SkAndroidCodec to check for/decode the
> required frame, so that it will scale properly.
> - Call rewindIfNeeded inside handleFrameIndex. handleFrameIndex calls a
> virtual method which may set some state (e.g. in SkJpegCodec). Without
> this change, that state would be reset by rewindIfNeeded.
> - Simplify handling a kRestoreBGColor frame. Whether provided or not,
> take the same path to calling zero_rect.
> - Updates to zero_rect:
> - Intersect after scaling, which will also check for empty.
> - Round out instead of in - this ensures we don't under-erase
> - Use kFill_ScaleToFit, which better matches the intent.
>
> Change-Id: Ibe1951980a0dca8f5b7b1f20192432d395681683
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333225
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
Bug: b/160984428
Bug: b/163595585
Change-Id: I7c1e79e0f92c75b4840eef65c8fc2b8497189e81
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334842
Auto-Submit: Leon Scroggins <scroggo@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
This reverts commit fc4fdc5b25.
Reason for revert: Google3 and ASAN failures
Change-Id: I890cd76109c0375391637f879550837d01e650f9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334840
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: b/160984428
Bug: b/163595585
Add support to SkAndroidCodec for decoding all frames with an
fSampleSize, so that an entire animation can be decoded to a smaller
size.
dm/:
- Test scaled + animated decodes
SkAndroidCodec:
- Make AndroidOptions inherit from SkCodec::Options. This allows
SkAndroidCodec to use fFrameIndex. (It also combines the two versions
of fSubset, which is now const for both.)
- Respect fFrameIndex, and call SkCodec::handleFrameIndex to decode
the required frame.
- Disallow decoding with kRespect + fFrameIndex > 0 if there is a
non-default orientation. As currently written (except without
disabling this combination), SkPixmapPriv::Orient would draw the new
portion of the frame on top of uninitialized pixels, instead of the
prior frame. This could be fixed by
- If SkAndroidCodec needs to decode the required frame, it could do so
without applying the orientation, then decode fFrameIndex, and then
apply the orientation.
- If the client provided the required frame, SkAndroidCodec would need
to un-apply the orientation to get the proper starting state, then
decode and apply.
I think it is simpler to force the client to handle the orientation
externally.
SkCodec:
- Allow SkAndroidCodec to call its private method handleFrameIndex. This
method handles decoding a required frame, if necessary. When called by
SkAndroidCodec, it now uses the SkAndroidCodec to check for/decode the
required frame, so that it will scale properly.
- Call rewindIfNeeded inside handleFrameIndex. handleFrameIndex calls a
virtual method which may set some state (e.g. in SkJpegCodec). Without
this change, that state would be reset by rewindIfNeeded.
- Simplify handling a kRestoreBGColor frame. Whether provided or not,
take the same path to calling zero_rect.
- Updates to zero_rect:
- Intersect after scaling, which will also check for empty.
- Round out instead of in - this ensures we don't under-erase
- Use kFill_ScaleToFit, which better matches the intent.
Change-Id: Ibe1951980a0dca8f5b7b1f20192432d395681683
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333225
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
This is a reland of ed63444587
Original change's description:
> Add idea of DataType to SkYUVAPixmapInfo.
>
> DataType describes the data type of YUVA channels
> independent of how they are grouped into planes.
>
> Adds mapping functions between SkColorType/channel count
> and DataType.
>
> SkYUVAPixmapInfo can be constructed from DataType and will
> choose appropriate SkColorTypes for each plane.
>
> Valid SkYUVAPixmapInfos now have the same DataType for each
> plane (could relax this in the future, esp for alpha plane).
>
> SkYUVAPixmapInfo::SupportedDataTypes specifies the supported
> combinations of SkYUVAInfo::PlanarConfig and
> kYUVAPixmapInfo::DataType supported by a GrContext (based on
> supported texture formats).
>
> SkImageGenerator/SkCodec YUVA query API now takes a
> SupportedDataTypes.
>
> Change-Id: I8791234638e6ba3396d1e7960b7bc210edc6dd57
> Bug: skia:10632
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314276
> Reviewed-by: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:10632
Change-Id: I35b55b7477c11c822fdb3729a9f84acff1eb785d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315284
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reverts commit ed63444587.
Reason for revert: trying to unblock android roller, which incorrectly thinks this has a merge conflict: https://b.corp.google.com/issues/167576324
Original change's description:
> Add idea of DataType to SkYUVAPixmapInfo.
>
> DataType describes the data type of YUVA channels
> independent of how they are grouped into planes.
>
> Adds mapping functions between SkColorType/channel count
> and DataType.
>
> SkYUVAPixmapInfo can be constructed from DataType and will
> choose appropriate SkColorTypes for each plane.
>
> Valid SkYUVAPixmapInfos now have the same DataType for each
> plane (could relax this in the future, esp for alpha plane).
>
> SkYUVAPixmapInfo::SupportedDataTypes specifies the supported
> combinations of SkYUVAInfo::PlanarConfig and
> kYUVAPixmapInfo::DataType supported by a GrContext (based on
> supported texture formats).
>
> SkImageGenerator/SkCodec YUVA query API now takes a
> SupportedDataTypes.
>
> Change-Id: I8791234638e6ba3396d1e7960b7bc210edc6dd57
> Bug: skia:10632
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314276
> Reviewed-by: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,scroggo@google.com
Change-Id: I72c39539a4766f10cac3ca3cdef6c503a8319ff1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10632
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314895
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
DataType describes the data type of YUVA channels
independent of how they are grouped into planes.
Adds mapping functions between SkColorType/channel count
and DataType.
SkYUVAPixmapInfo can be constructed from DataType and will
choose appropriate SkColorTypes for each plane.
Valid SkYUVAPixmapInfos now have the same DataType for each
plane (could relax this in the future, esp for alpha plane).
SkYUVAPixmapInfo::SupportedDataTypes specifies the supported
combinations of SkYUVAInfo::PlanarConfig and
kYUVAPixmapInfo::DataType supported by a GrContext (based on
supported texture formats).
SkImageGenerator/SkCodec YUVA query API now takes a
SupportedDataTypes.
Change-Id: I8791234638e6ba3396d1e7960b7bc210edc6dd57
Bug: skia:10632
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314276
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
SkYUVAPixmapInfo is a SkYUVAInfo with per-plane color types and row
bytes. It describes a set of pixmaps that make up a planar image.
Consolidates validity checks of the SkYUVAInfo with the color types
and row bytes. It can provide SkImageInfos for each plane and also
assist with configuring planes to share a common allocation.
SkYUVAPixmaps is a collection of SkPixmaps that are valid for a
SkYUVAInfo. It can either wrap existing SkPixmaps or allocate and
own the memory. It consolidates validity checking of SkPixmaps with
the SkYUVAInfo. Replaces sk_gpu_test::YUVAPixmaps.
Minor tweaks to SkYUVAInfo naming, parameter order consistency, adds a
hasAlpha() method.
Bug: skia:10632
Change-Id: Ib0f48b8448fff22805fd0c04e07887d0b7338b76
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312886
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Tunnels through SkImageGenerator as well.
The new SkCodec interface doesn't assume three 8 bit planes.
New SkYUVASpec more clearly defines chroma subsampling and siting of
the planes.
The intent is to use this for other YUVA APIs as well, in particular
SkImage factories in the future.
In this change we convert to the SkYUVASpec to SkYUVASizeInfo
and SkYUVAIndex[4] representation. But the intent is to use
the SkYUVASpec representation throughout the pipeline once
legacy APIs are removed.
orientation GM is replicated to test a variety of chroma
subsampling configs.
Bug: skia:10632
Change-Id: I3fad35752b87cac16c51b24824331f2ae7d458d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309658
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
This fixes a large number of SkSL namespaces which were labeled as if
they were anonymous, and also a handful of other mislabeled namespaces.
Missing namespace-end comments have been added throughout.
A number of diffs are just indentation-related (adjusting 1- or 3-
space indents to 2-space).
Change-Id: I6c62052a0d3aea4ae12ca07e0c2a8587b2fce4ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308503
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Bug: b/135133301
Follow-on to 196f319b.
- Add SkCodec::getICCProfile to match the SkAndroidCodec version.
- Update comments on getPixels() regarding how the SkColorSpace on the
SkImageInfo is treated.
- Add two new images that have ICC profiles that do not map to an
SkColorSpace. Add a test to verify that they have the un-transformed
color we expect.
- Stop uploading ColorCodecSrc images decoded to a null SkColorSpace to
Gold. Though they may be correct, they do not match other images they're
compared against. The new test above verifies that we do not do color
conversion with a null SkColorSpace.
Change-Id: I08635e4262f16500fab32ef97511d305c2c06483
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269236
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This reverts commit bcbf9633db.
Next patch set implements a local shared spinlock
rather than depending on SkSharedMutex.
Next patch removes the thread safety and the unit test.
Original change's description:
> Temporarily revert runtime decoder registration.
>
> Some builds in Google3 can't find SkSharedMutex at link time.
>
> This reverts commit ff7006ff80.
> This reverts commit 05f45b21b7.
>
> Change-Id: I889e9a3169cf017b8d5daacf75099663fc996d73
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/210785
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: I6df8e2dfb64a92c780b62acddf7a797f24ba8e31
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/210792
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Some builds in Google3 can't find SkSharedMutex at link time.
This reverts commit ff7006ff80.
This reverts commit 05f45b21b7.
Change-Id: I889e9a3169cf017b8d5daacf75099663fc996d73
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/210785
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
What do you think of this sort of thing for letting
users register codecs at runtime instead of at build time?
Change-Id: I448ea3466862326c283ad9b2f1b7eea56cffc80c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/210449
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Current strategy: everything from the top
Things to look at first are the manual changes:
- added tools/rewrite_includes.py
- removed -Idirectives from BUILD.gn
- various compile.sh simplifications
- tweak tools/embed_resources.py
- update gn/find_headers.py to write paths from the top
- update gn/gn_to_bp.py SkUserConfig.h layout
so that #include "include/config/SkUserConfig.h" always
gets the header we want.
No-Presubmit: true
Change-Id: I73a4b181654e0e38d229bc456c0d0854bae3363e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/209706
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Hal Canary <halcanary@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Bug: skia:8579
Zero-initialize the pixel buffer that wuffs uses inside the frame rect.
If the encoded image is incomplete, we will swizzle this memory into the
destination, so this prevents swizzling uninitialized memory.
In addition, zero-initialize the client's memory if the above buffer
will not fill it, and if the frame is independent.
Move decodeFrameConfig into onStartIncrementalDecode. This makes it
clear that we haven't begun decoding the pixels so rowsDecoded is
irrelevant. Update the documentation for onStartIncrementalDecode to
make it clear that it can be called more than once (and when to do so).
If a frame is incomplete and depends on prior frames, do not blend it.
This will overwrite pixels from the prior frames without a way to undo.
Current clients only want to show incomplete images for the first frame
anyway.
Add some debugging information to Codec_partialAnim test, including
writing out pngs when failing the test.
TBR=djsollen@google.com for documentation change in SkCodec.h
Change-Id: I85c8ca4075301306f4738ddfc2f5992a5745108b
Reviewed-on: https://skia-review.googlesource.com/c/174310
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Nigel Tao <nigeltao@google.com>
SkWuffsCodec tells wuffs to decode only the current frame's
contribution, and then SkWuffsCodec blends it onto the prior frame (if
any). If there is a prior frame, do not call SkSampler::Fill, and blend
the new frame onto the old, rather than replacing it.
In addition, set rowsDecoded to scaledHeight to work properly for
scaled decodes.
TBR=reed@google.com No change to public API
Bug: skia:8235
Change-Id: I08e89f1083da3f9b98f93d8d2375ce78f0c378f8
Reviewed-on: https://skia-review.googlesource.com/c/171645
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Nigel Tao <nigeltao@google.com>
This reverts commit 06a477c330.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> remove old name for SkCodec::kNoFrame
>
> Cq-Include-Trybots: skia.primary:Build-Debian9-Clang-arm-Release-Flutter_Android
> Change-Id: I039aed7591aa7767046f28a48798b6fa2f7fd643
> Reviewed-on: https://skia-review.googlesource.com/c/169224
> Reviewed-by: Leon Scroggins <scroggo@google.com>
TBR=mtklein@google.com,scroggo@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: I6c61ee27a4167127c74c14e0f9e05a4bbac931a8
Cq-Include-Trybots: skia.primary:Build-Debian9-Clang-arm-Release-Flutter_Android
Reviewed-on: https://skia-review.googlesource.com/c/170422
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit 49e1239f94.
Reason for revert: need more
Original change's description:
> put kNone back too
>
> Change-Id: I07969ca6b7367460259901cf526b11cbf2793663
> Reviewed-on: https://skia-review.googlesource.com/c/170420
> Auto-Submit: Mike Klein <mtklein@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,reed@google.com
Change-Id: Id027f6d82d2c61bfc6a2c4ee4f894452830e6889
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/170421
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: I07969ca6b7367460259901cf526b11cbf2793663
Reviewed-on: https://skia-review.googlesource.com/c/170420
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Bug: skia:7903
Change-Id: If5acd50711ed8bd4a49efcb93db66fd3d14c8992
Reviewed-on: https://skia-review.googlesource.com/c/164681
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Bug: skia:7903
Change-Id: I7301d943b679a4670511b6cf60c594baf615834a
Reviewed-on: https://skia-review.googlesource.com/c/164261
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:7901
Change-Id: Ic83e9f0c2a493335671fe431ffba6f649812d406
Reviewed-on: https://skia-review.googlesource.com/c/163481
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This reverts commit 0c583af06d.
Reason for revert: DDL is failing
Original change's description:
> Widen internal API to support more complex YUV formats
>
> Bug: skia:7901
> Change-Id: I46fec08711b8b483cf58ccae733e4dc2a9689231
> Reviewed-on: https://skia-review.googlesource.com/c/162280
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=egdaniel@google.com,jvanverth@google.com,bsalomon@google.com
Change-Id: Ibe3dd7abbce4a3b6afe74c565198dadc61a9f439
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:7901
Reviewed-on: https://skia-review.googlesource.com/c/163257
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Bug: skia:7901
Change-Id: I46fec08711b8b483cf58ccae733e4dc2a9689231
Reviewed-on: https://skia-review.googlesource.com/c/162280
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This object is now redundant with fEncodedInfo. It holds an
SkColorSpace, which is never used now that SkCodec uses skcms.
SkCodec::getInfo() no longer returns a reference, but creates an
SkImageInfo from the SkEncodedInfo the same way fSrcInfo was
previously created.
Add SkCodec::bounds() and ::dimensions() to replace calling these
methods on getInfo().
Remove srcInfo from SkMaskSwizzler::Create, which just wants to know
whether the src is opaque.
Remove the srcColorType from conversionSupported. Only the base class
version used it. Make it look at fEncodedInfo.color() instead.
Bug: skia:6839
Bug: chromium:887372
Change-Id: I2c0583cae76121211c1a6b49979972fa86daf077
Reviewed-on: https://skia-review.googlesource.com/c/157563
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 49894f450f.
Reason for revert: Breaking a CTS test on the Android roll:
java.lang.AssertionError: expected:<67043583> but was:<50266367>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at org.junit.Assert.assertEquals(Assert.java:631)
at android.graphics.cts.BitmapColorSpaceTest.verifyGetPixel(BitmapColorSpaceTest.java:301)
at android.graphics.cts.BitmapColorSpaceTest.inColorSpaceP3ToSRGB(BitmapColorSpaceTest.java:612)
Expected: 3FF00FF Actual: 2FF00FF
Original change's description:
> Reland "Switch SkCodec to use skcms" plus fixes
>
> This reverts commit 33d5394d08,
> relanding 81886e8f94 as well as
> "Fix CMYK handling in JPEG codec" (commit
> f8ae5ce20c)
>
> Add a test based on the CTS test that failed in the original commit.
> purple-displayprofile.png is the image used in the CTS test, with the
> Android license.
>
> This also adds a fix for SkAndroidCodec, ensuring that we continue to
> use a wide gamut SkColorSpace for images that do not have a numerical
> transfer function and have a wide gamut. This includes a test, with
> wide-gamut.png, which was created with Photoshop and the profile
> "sRGB_Calibrated_Homogeneous.icc" from the skcms tree.
>
> Bug: skia:6839
> Bug: skia:8052
> Bug: skia:8278
>
> TBR=djsollen@google.com
> As with the original, no API change
>
> Change-Id: I4e5bba6a3151f9dc6491e8eda73d4de0535bd692
> Reviewed-on: https://skia-review.googlesource.com/149043
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Leon Scroggins <scroggo@google.com>
TBR=djsollen@google.com,mtklein@google.com,scroggo@google.com,brianosman@google.com
Change-Id: Ie71e1fecc26de8225d2fe603765c1e1e0d738634
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:6839, skia:8052, skia:8278
Reviewed-on: https://skia-review.googlesource.com/149262
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This reverts commit 33d5394d08,
relanding 81886e8f94 as well as
"Fix CMYK handling in JPEG codec" (commit
f8ae5ce20c)
Add a test based on the CTS test that failed in the original commit.
purple-displayprofile.png is the image used in the CTS test, with the
Android license.
This also adds a fix for SkAndroidCodec, ensuring that we continue to
use a wide gamut SkColorSpace for images that do not have a numerical
transfer function and have a wide gamut. This includes a test, with
wide-gamut.png, which was created with Photoshop and the profile
"sRGB_Calibrated_Homogeneous.icc" from the skcms tree.
Bug: skia:6839
Bug: skia:8052
Bug: skia:8278
TBR=djsollen@google.com
As with the original, no API change
Change-Id: I4e5bba6a3151f9dc6491e8eda73d4de0535bd692
Reviewed-on: https://skia-review.googlesource.com/149043
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
This reverts commit 81886e8f94 and
f8ae5ce20c
("Fix CMYK handling in JPEG codec")
This fixes the Android build, which was failing a CTS test with this
change.
Bug: skia:6839
Bug: skia:8052
TBR=djsollen@google.com
As with the original, no API change
Change-Id: Ic744a610e9f431707f871de44f9f64040bc60d14
Reviewed-on: https://skia-review.googlesource.com/148810
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: skia:6839
Bug: skia:8052
Create an skcms_Profile instead of an SkColorSpace when creating an
SkCodec. Eventually we'll move the SkImageInfo (and its SkColorSpace)
entirely to clients (e.g. SkAndroidCodec, SkCodecImageGenerator), but
for now, create it with SkEncodedInfo::makeImageInfo.
Create new SkEncodedInfo::Colors for the special PNG cases that we
want to map to specific SkColorTypes.
SkEncodedInfo:
- Add ICCProfile, which owns an skcms_ICCProfile
- FIXME: maybe we should have a single instance for
SRGB like SkColorSpace?
- Add kXAlpha_Color, for kAlpha_8. Since I'm not longer creating
an SkImageInfo (at least in SkPngCodec), it needs a way to pass
this info to the caller.
- Add k565_Color, for the same reason. Matt originally had this in
https://codereview.chromium.org/2212563003/#ps120001, but didn't
land that version. I like it though. Mike didn't like the bits
per component for 565, but it seems like a sensible hack, much
like the existing one for kAlpha_8
- Add width and height. These were removed for redundancy with
SkImageInfo, but it makes sense to have them here without it.
BUILD.gn:
- Build the new SkEncodedInfo.cpp
SkCodec:
- Remove the constructor with an SkImageInfo. Edit the other one
to drop width and height (now in SkEncodedInfo) and take a RHS
reference to SkEncodedInfo
- Create the SkImageInfo from fEncodedInfo (for now)
- Consolidate choosing skcms_AlphaFormat for Transform here
- Call conversionSupported from initializeColorXform, with a new
parameter for whether there is a color Xform, allowing SkJpegCodec
and SkHeifCodec to override that method instead of having another
method.
SkBmpCodec (etc)
- Adapt to the changes above
- Create a new SkEncodedInfo w/o profile for the swizzler.
SkPngCodec:
- use the new SkEncodedInfo::Colors rather than a custom SkImageInfo
SkRawCodec:
- Remove SkEncodedInfo from SkDngImage, which doesn't actually need it.
This is helpful since we don't know all the info yet.
- Rewrite gAdobeRGB_toXYZD50 as an skcms_Matrix3x3
SkWebpCodec:
- Remove premul_step computation, and simplify to just rely on
the base class' handling of applying the transform.
SkSwizzler:
- Add cases for the new SkEncodedInfo::Colors
TBR=reed@google.com
No public API changes. Only private/public members of SkCodec.h are
modified.
Change-Id: Ic0d3bb752b03f13be886b80331987aa5a5713fc0
Reviewed-on: https://skia-review.googlesource.com/136062
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Add a SK_LEGACY_SKCODEC_NONE_ENUM #define'able to ease transition.
Also rename another internal "kNone" enum in SkWebpCodec.cpp.
The "kNone" name is overloaded:
- include/core/SkImageGenerator.h defines its own kNone.
- include/encode/SkPngEncoder.h defines its own kNone.
- include/private/GrTypesPriv.h defines multiple kNone's.
Bug: skia:
Change-Id: I18c03a74f2f1b0237016e0eef53dcd4478b82609
Reviewed-on: https://skia-review.googlesource.com/146020
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This is a behavior change and a simplification.
When an image is incomplete or subset, we fill the remaining/all rows
with the fill color. A virtual method chose the fill color. Here were
the implementations:
- SkGifCodec:
Use transparent. This was changed previously to match Chromium.
- SkPngCodec/SkBmpStandardCodec:
Use the first color in the color table. This made sense when we had to
support kIndex_8, when we had to use an index from the color table.
Using that color for other types ensured that the image looked the same
in e.g. kN32 as kIndex_8. Removing this arbitrary choice simplifies the
code and moves the behavior toward Chromium's.
- SkCodec (default):
Use black for opaque images and transparent for images with alpha. A
theoretical advantage to this decision was that an incomplete image
would look the same in k565 and kN32. I don't think this is a good
enough reason to behave differently from Chromium.
Consolidate them all to just use 0. This results in transparent where
that is something we can specify. For 565 and Gray, this results in
black.
Only change to include headers is the removal of protected methods.
TBR=hcm@google.com
Change-Id: I9a7224b4e91b5c4988f3a87381653e99e40e10a5
Reviewed-on: https://skia-review.googlesource.com/145378
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:
Change-Id: Iccbe609b2a8d4c9098ef36aa6530dbc3771e0c11
Reviewed-on: https://skia-review.googlesource.com/141055
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
- Encoders and decoders always assume kIgnore.
- They are less opinionated about F16 and color space,
we just trust the color space that's passed in, and
put that directly in the image (no sRGB encoding).
- SkBitmap and SkPixmap read/write pixels functions were
defaulting to kResepct, those are now always kIgnore.
- Many other bits of plumbing are simplified, and I
added a default of kIgnore to SkImage::makeColorSpace,
so we can phase out that argument entirely.
- Still need to add defaults to other public APIs that
take SkTransferFunctionBehavior.
- This makes gold think that we've dramatically changed
the contents of all F16 images, but that's because
it doesn't understand the (now linear) color space
that's embedded. Once we triage them all once, they
will work fine (and they'll look perfect in the browser).
Bug: skia:
Change-Id: I62fa090f96cae1b67d181ce14bd91f34ff2ed747
Reviewed-on: https://skia-review.googlesource.com/140570
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This wasn't being run (AFAICT) and was built on pre-skcms color management.
Change-Id: I506e8767f716bc6e4590ce255c5e40f1064fc152
Reviewed-on: https://skia-review.googlesource.com/129644
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This partially reverts commit
1793e7bb46.
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, make SkAndroidCodec a friend so it can
access the SkEncodedInfo. A future change will change SkCodec to
recommend F16 for bitsPerComponent > 8, but that will be more involved;
it was the reason for the revert of this CL.
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.
TBR=mtklein@chromium.org,reed@google.com
Change-Id: I928b1f55317602cb37d29da63b53026c8d139cee
Reviewed-on: https://skia-review.googlesource.com/80860
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This reverts commit c6f7a4ffa9.
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>
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>
It is now used by SkPixmap and will soon be in SkJpegEncoder. No need
for those to depend on SkCodec.
Bug: 768878
TBR=reed@google.com
(reed@ already approved the API change in
https://skia-review.googlesource.com/60721)
Change-Id: If1a6e1d5b60a7a3d8c97818e15a48d28ba804668
Reviewed-on: https://skia-review.googlesource.com/61680
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This reverts commit 5411a60e0d.
Reason for revert: ASAN and Coverage failing: https://chromium-swarm.appspot.com/task?id=394978f3b7d44610
Flutter_Android failing.
Original change's description:
> Add an Option for orientation on JPEG encodes
>
> Move Origin to its own header so that SkPixmap and SkJpegEncoder need
> not depend on SkCodec.
>
> Add libexif, which is already used by Android, and use it to write the
> orientation. Write a makefile based on the Android.bp in Android, minus
> warnings. (libexif has an LGPL license.)
>
> Add a test that verifies all the orientations work.
>
> Optionally enable writing the orientation (and therefore including
> libexif). Chromium does not currently need it, and Android does not
> expose an API that would allow using it. Disable on Windows, where we
> still have build errors to fix.
>
> Bug: skia:7138
> Change-Id: Iaeff44c36aebe0e639666979dc00e1b7594bbeb1
> Reviewed-on: https://skia-review.googlesource.com/60721
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Mike Klein <mtklein@chromium.org>
> Reviewed-by: Mike Reed <reed@google.com>
TBR=mtklein@chromium.org,mtklein@google.com,scroggo@google.com,reed@google.com
Change-Id: I05b7ae8d1c5bbd1de1642d9ef024943500256273
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:7138
Reviewed-on: https://skia-review.googlesource.com/61620
Reviewed-by: Ben Wagner <benjaminwagner@google.com>
Commit-Queue: Ben Wagner <benjaminwagner@google.com>
Move Origin to its own header so that SkPixmap and SkJpegEncoder need
not depend on SkCodec.
Add libexif, which is already used by Android, and use it to write the
orientation. Write a makefile based on the Android.bp in Android, minus
warnings. (libexif has an LGPL license.)
Add a test that verifies all the orientations work.
Optionally enable writing the orientation (and therefore including
libexif). Chromium does not currently need it, and Android does not
expose an API that would allow using it. Disable on Windows, where we
still have build errors to fix.
Bug: skia:7138
Change-Id: Iaeff44c36aebe0e639666979dc00e1b7594bbeb1
Reviewed-on: https://skia-review.googlesource.com/60721
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
