Commit Graph

13 Commits

Author SHA1 Message Date
Brian Osman
ea236bf818 Move skcms.h to include/third_party/skcms
Add a shim to redirect until clients are updated

Change-Id: Ib43614e5620b1a24ca18187c1646a8ed1a9ee7a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/211003
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2019-04-29 15:02:45 +00:00
Mike Klein
c0bd9f9fe5 rewrite includes to not need so much -Ifoo
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>
2019-04-24 16:27:11 +00:00
Brian Osman
c3186302bd Add skcms directory to public includes
skcms is part of Skia's public API now. This attempts to recognize that,
and pave the way for moving the header to another location more easily
in a follow up CL, or - at a minimum - for clients that redistribute
Skia as a library + includes to relocate the skcms.h header as part of
that.

Change-Id: I15da63b0d4ab8916a71fb7e6ab3656db87252707
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/209640
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2019-04-22 20:32:23 +00:00
Leon Scroggins III
34a2a76e20 Remove assert that BGRA and RGBA are not opaque
Bug: skia:7353
Bug: skia:8235

This should fix the Wuffs bot. A recent change set SkWuffsCodec's
SkEncodedInfo::Color to BGRA or RGBA even if they are opaque, which
resulted in this assert firing. As has been brought up previously,
SkEncodedInfo::Color is overspecified. The only code that cares
about the type is SkSwizzler, which will treat the alpha as
potentially non-opaque (whereas BGRX_Color (there is no RGBX_Color)
ignores the alpha channel, which is important for some BMPs).

TBR=reed@google.com

Change-Id: I3396b53aef114e09513579f829c5b5345acba4f0
Reviewed-on: https://skia-review.googlesource.com/c/193028
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
2019-02-15 19:53:04 +00:00
Leon Scroggins III
5dd47e4f15 Make SkCodec truly default to sRGB
Remove SkEncodedInfo::ICCProfile::MakeSRGB. Instead of creating
this object whenever there is no encoded color profile, just
treat null as sRGB, like skcms does.

This may help with crbug.com/887372. Regardless it simplifies the
code.

Also fix a bug where SkCodec could have passed a null
skcms_ICCProfile to skcms_ApproximatelyEqualProfiles (related
to b/116608007).

Bug: chromium:887372
Change-Id: I2374e8d8a1aed261f1291b7f6fd6c7ea662f26fd
Reviewed-on: https://skia-review.googlesource.com/157561
Auto-Submit: Leon Scroggins <scroggo@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2018-09-27 20:08:06 +00:00
Leon Scroggins III
36f7e3298e Reland "Switch SkCodec to use skcms plus fixes""
This reverts commit 83988edfd3.

The CTS failure was actually due to another CL.

TBR=brianosman@google.com
TBR=djsollen@google.com

Bug: skia:6839
Bug: skia:8052
Bug: skia:8278
Change-Id: Id9f152ec2c66467d90f49df223cb9b7c168ac2ac
Reviewed-on: https://skia-review.googlesource.com/149483
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
2018-08-27 16:16:14 +00:00
Leon Scroggins
83988edfd3 Revert "Reland "Switch SkCodec to use skcms" plus fixes"
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>
2018-08-24 21:41:37 +00:00
Leon Scroggins III
49894f450f 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>
2018-08-24 17:46:25 +00:00
Leon Scroggins III
33d5394d08 Revert "Switch SkCodec to use skcms" and follow on change
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>
2018-08-22 21:13:50 +00:00
Leon Scroggins III
81886e8f94 Switch SkCodec to use skcms
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>
2018-08-22 18:16:20 +00:00
Leon Scroggins III
c8037dc5ed Reland "Hide SkEncodedInfo"
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>
2017-12-05 19:28:08 +00:00
Leon Scroggins
1793e7bb46 Revert "Hide SkEncodedInfo"
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>
2017-12-05 15:38:22 +00:00
Leon Scroggins III
c6f7a4ffa9 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>
2017-12-05 00:52:42 +00:00