Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 49894f4

Browse files
LeonScrogginsSkia Commit-Bot
authored andcommitted
Reland "Switch SkCodec to use skcms" plus fixes
This reverts commit 33d5394, relanding 81886e8 as well as "Fix CMYK handling in JPEG codec" (commit f8ae5ce) 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>
1 parent 12d966f commit 49894f4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+666
-555
lines changed

BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,7 @@ component("skia") {
863863
"src/codec/SkCodec.cpp",
864864
"src/codec/SkCodecImageGenerator.cpp",
865865
"src/codec/SkColorTable.cpp",
866+
"src/codec/SkEncodedInfo.cpp",
866867
"src/codec/SkGifCodec.cpp",
867868
"src/codec/SkMaskSwizzler.cpp",
868869
"src/codec/SkMasks.cpp",

gn/tests.gni

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ tests_sources = [
6464
"$_tests/EGLImageTest.cpp",
6565
"$_tests/EmptyPathTest.cpp",
6666
"$_tests/EncodeTest.cpp",
67+
"$_tests/EncodedInfoTest.cpp",
6768
"$_tests/ExifTest.cpp",
6869
"$_tests/F16StagesTest.cpp",
6970
"$_tests/FillPathTest.cpp",

include/codec/SkCodec.h

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
#include "../private/SkEncodedInfo.h"
1414
#include "SkCodecAnimation.h"
1515
#include "SkColor.h"
16-
#include "SkColorSpaceXform.h"
1716
#include "SkEncodedImageFormat.h"
1817
#include "SkEncodedOrigin.h"
1918
#include "SkImageInfo.h"
@@ -671,21 +670,9 @@ class SK_API SkCodec : SkNoncopyable {
671670
protected:
672671
const SkEncodedInfo& getEncodedInfo() const { return fEncodedInfo; }
673672

674-
using XformFormat = SkColorSpaceXform::ColorFormat;
673+
using XformFormat = skcms_PixelFormat;
675674

676-
SkCodec(int width,
677-
int height,
678-
const SkEncodedInfo&,
679-
XformFormat srcFormat,
680-
std::unique_ptr<SkStream>,
681-
sk_sp<SkColorSpace>,
682-
SkEncodedOrigin = kTopLeft_SkEncodedOrigin);
683-
684-
/**
685-
* Allows the subclass to set the recommended SkImageInfo
686-
*/
687-
SkCodec(const SkEncodedInfo&,
688-
const SkImageInfo&,
675+
SkCodec(SkEncodedInfo&&,
689676
XformFormat srcFormat,
690677
std::unique_ptr<SkStream>,
691678
SkEncodedOrigin = kTopLeft_SkEncodedOrigin);
@@ -780,16 +767,14 @@ class SK_API SkCodec : SkNoncopyable {
780767

781768
virtual int onOutputScanline(int inputScanline) const;
782769

783-
bool initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Alpha);
784770
// Some classes never need a colorXform e.g.
785771
// - ICO uses its embedded codec's colorXform
786772
// - WBMP is just Black/White
787773
virtual bool usesColorXform() const { return true; }
788-
void applyColorXform(void* dst, const void* src, int count, SkAlphaType) const;
789774
void applyColorXform(void* dst, const void* src, int count) const;
790775

791-
SkColorSpaceXform* colorXform() const { return fColorXform.get(); }
792-
bool xformOnDecode() const { return fXformOnDecode; }
776+
bool colorXform() const { return fXformTime != kNo_XformTime; }
777+
bool xformOnDecode() const { return fXformTime == kDecodeRow_XformTime; }
793778

794779
virtual int onGetFrameCount() {
795780
return 1;
@@ -813,22 +798,32 @@ class SK_API SkCodec : SkNoncopyable {
813798

814799
SkImageInfo fDstInfo;
815800
Options fOptions;
801+
802+
enum XformTime {
803+
kNo_XformTime,
804+
kPalette_XformTime,
805+
kDecodeRow_XformTime,
806+
};
807+
XformTime fXformTime;
816808
XformFormat fDstXformFormat; // Based on fDstInfo.
817-
std::unique_ptr<SkColorSpaceXform> fColorXform;
818-
bool fXformOnDecode;
809+
skcms_ICCProfile fDstProfile;
810+
skcms_AlphaFormat fDstXformAlphaFormat;
819811

820812
// Only meaningful during scanline decodes.
821813
int fCurrScanline;
822814

823815
bool fStartedIncrementalDecode;
824816

825817
/**
826-
* Return whether {srcColor, srcIsOpaque, srcCS} can convert to dst.
818+
* Return whether we can convert to dst.
827819
*
828820
* Will be called for the appropriate frame, prior to initializing the colorXform.
829821
*/
830822
virtual bool conversionSupported(const SkImageInfo& dst, SkColorType srcColor,
831-
bool srcIsOpaque, const SkColorSpace* srcCS) const;
823+
bool srcIsOpaque, bool needsColorXform);
824+
825+
bool initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Alpha, bool srcIsOpaque);
826+
832827
/**
833828
* Return whether these dimensions are supported as a scale.
834829
*

include/private/SkEncodedInfo.h

Lines changed: 101 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,25 @@
88
#ifndef SkEncodedInfo_DEFINED
99
#define SkEncodedInfo_DEFINED
1010

11+
#include "SkData.h"
1112
#include "SkImageInfo.h"
12-
13-
class SkColorSpace;
13+
#include "../../third_party/skcms/skcms.h"
1414

1515
struct SkEncodedInfo {
1616
public:
17+
class ICCProfile {
18+
public:
19+
static std::unique_ptr<ICCProfile> Make(sk_sp<SkData>);
20+
static std::unique_ptr<ICCProfile> MakeSRGB();
21+
static std::unique_ptr<ICCProfile> Make(const skcms_ICCProfile&);
22+
23+
const skcms_ICCProfile* profile() const { return &fProfile; }
24+
private:
25+
ICCProfile(const skcms_ICCProfile&, sk_sp<SkData> = nullptr);
26+
27+
skcms_ICCProfile fProfile;
28+
sk_sp<SkData> fData;
29+
};
1730

1831
enum Alpha {
1932
kOpaque_Alpha,
@@ -39,6 +52,20 @@ struct SkEncodedInfo {
3952
// PNG
4053
kGrayAlpha_Color,
4154

55+
// PNG with Skia-specific sBIT
56+
// Like kGrayAlpha, except this expects to be treated as
57+
// kAlpha_8_SkColorType, which ignores the gray component. If
58+
// decoded to full color (e.g. kN32), the gray component is respected
59+
// (so it can share code with kGrayAlpha).
60+
kXAlpha_Color,
61+
62+
// PNG
63+
// 565 images may be encoded to PNG by specifying the number of
64+
// significant bits for each channel. This is a strange 565
65+
// representation because the image is still encoded with 8 bits per
66+
// component.
67+
k565_Color,
68+
4269
// PNG, GIF, BMP
4370
kPalette_Color,
4471

@@ -67,7 +94,18 @@ struct SkEncodedInfo {
6794
kYCCK_Color,
6895
};
6996

70-
static SkEncodedInfo Make(Color color, Alpha alpha, int bitsPerComponent) {
97+
static SkEncodedInfo Make(int width, int height, Color color, Alpha alpha,
98+
int bitsPerComponent) {
99+
return Make(width, height, color, alpha, bitsPerComponent, nullptr);
100+
}
101+
102+
static SkEncodedInfo MakeSRGB(int width, int height, Color color, Alpha alpha,
103+
int bitsPerComponent) {
104+
return Make(width, height, color, alpha, bitsPerComponent, ICCProfile::MakeSRGB());
105+
}
106+
107+
static SkEncodedInfo Make(int width, int height, Color color, Alpha alpha,
108+
int bitsPerComponent, std::unique_ptr<ICCProfile> profile) {
71109
SkASSERT(1 == bitsPerComponent ||
72110
2 == bitsPerComponent ||
73111
4 == bitsPerComponent ||
@@ -105,43 +143,67 @@ struct SkEncodedInfo {
105143
SkASSERT(kOpaque_Alpha != alpha);
106144
SkASSERT(8 == bitsPerComponent);
107145
break;
146+
case kXAlpha_Color:
147+
SkASSERT(kUnpremul_Alpha == alpha);
148+
SkASSERT(8 == bitsPerComponent);
149+
break;
150+
case k565_Color:
151+
SkASSERT(kOpaque_Alpha == alpha);
152+
SkASSERT(8 == bitsPerComponent);
153+
break;
108154
default:
109155
SkASSERT(false);
110156
break;
111157
}
112158

113-
return SkEncodedInfo(color, alpha, bitsPerComponent);
159+
return SkEncodedInfo(width, height, color, alpha, bitsPerComponent, std::move(profile));
114160
}
115161

116162
/*
117-
* Returns an SkImageInfo with Skia color and alpha types that are the
118-
* closest possible match to the encoded info.
163+
* Returns a recommended SkImageInfo.
164+
*
165+
* TODO: Leave this up to the client.
119166
*/
120-
SkImageInfo makeImageInfo(int width, int height, sk_sp<SkColorSpace> colorSpace) const {
121-
auto ct = kGray_Color == fColor ? kGray_8_SkColorType :
122-
kN32_SkColorType ;
167+
SkImageInfo makeImageInfo() const {
168+
auto ct = kGray_Color == fColor ? kGray_8_SkColorType :
169+
kXAlpha_Color == fColor ? kAlpha_8_SkColorType :
170+
k565_Color == fColor ? kRGB_565_SkColorType :
171+
kN32_SkColorType ;
123172
auto alpha = kOpaque_Alpha == fAlpha ? kOpaque_SkAlphaType
124173
: kUnpremul_SkAlphaType;
125-
return SkImageInfo::Make(width, height, ct, alpha, std::move(colorSpace));
174+
sk_sp<SkColorSpace> cs = fProfile ? SkColorSpace::Make(*fProfile->profile())
175+
: nullptr;
176+
if (!cs) {
177+
cs = SkColorSpace::MakeSRGB();
178+
}
179+
return SkImageInfo::Make(fWidth, fHeight, ct, alpha, std::move(cs));
126180
}
127181

128-
Color color() const { return fColor; }
129-
Alpha alpha() const { return fAlpha; }
182+
int width() const { return fWidth; }
183+
int height() const { return fHeight; }
184+
Color color() const { return fColor; }
185+
Alpha alpha() const { return fAlpha; }
130186
bool opaque() const { return fAlpha == kOpaque_Alpha; }
187+
const skcms_ICCProfile* profile() const {
188+
if (!fProfile) return nullptr;
189+
return fProfile->profile();
190+
}
131191

132192
uint8_t bitsPerComponent() const { return fBitsPerComponent; }
133193

134194
uint8_t bitsPerPixel() const {
135195
switch (fColor) {
136196
case kGray_Color:
137197
return fBitsPerComponent;
198+
case kXAlpha_Color:
138199
case kGrayAlpha_Color:
139200
return 2 * fBitsPerComponent;
140201
case kPalette_Color:
141202
return fBitsPerComponent;
142203
case kRGB_Color:
143204
case kBGR_Color:
144205
case kYUV_Color:
206+
case k565_Color:
145207
return 3 * fBitsPerComponent;
146208
case kRGBA_Color:
147209
case kBGRA_Color:
@@ -156,17 +218,38 @@ struct SkEncodedInfo {
156218
}
157219
}
158220

159-
private:
221+
SkEncodedInfo(const SkEncodedInfo& orig) = delete;
222+
SkEncodedInfo& operator=(const SkEncodedInfo&) = delete;
160223

161-
SkEncodedInfo(Color color, Alpha alpha, uint8_t bitsPerComponent)
162-
: fColor(color)
224+
SkEncodedInfo(SkEncodedInfo&& orig) = default;
225+
SkEncodedInfo& operator=(SkEncodedInfo&&) = default;
226+
227+
// Explicit copy method, to avoid accidental copying.
228+
SkEncodedInfo copy() const {
229+
auto copy = SkEncodedInfo::Make(fWidth, fHeight, fColor, fAlpha, fBitsPerComponent);
230+
if (fProfile) {
231+
copy.fProfile.reset(new ICCProfile(*fProfile.get()));
232+
}
233+
return copy;
234+
}
235+
236+
private:
237+
SkEncodedInfo(int width, int height, Color color, Alpha alpha,
238+
uint8_t bitsPerComponent, std::unique_ptr<ICCProfile> profile)
239+
: fWidth(width)
240+
, fHeight(height)
241+
, fColor(color)
163242
, fAlpha(alpha)
164243
, fBitsPerComponent(bitsPerComponent)
244+
, fProfile(std::move(profile))
165245
{}
166246

167-
Color fColor;
168-
Alpha fAlpha;
169-
uint8_t fBitsPerComponent;
247+
int fWidth;
248+
int fHeight;
249+
Color fColor;
250+
Alpha fAlpha;
251+
uint8_t fBitsPerComponent;
252+
std::unique_ptr<ICCProfile> fProfile;
170253
};
171254

172255
#endif

resources/images/mandrill_cmyk.jpg

575 KB
Loading
5.54 KB
Loading

resources/images/wide-gamut.png

11.7 KB
Loading

src/codec/SkAndroidCodec.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@ static bool is_valid_sample_size(int sampleSize) {
2323
/**
2424
* Loads the gamut as a set of three points (triangle).
2525
*/
26-
static void load_gamut(SkPoint rgb[], const SkMatrix44& xyz) {
26+
static void load_gamut(SkPoint rgb[], const skcms_Matrix3x3& xyz) {
2727
// rx = rX / (rX + rY + rZ)
2828
// ry = rY / (rX + rY + rZ)
2929
// gx, gy, bx, and gy are calulcated similarly.
3030
for (int rgbIdx = 0; rgbIdx < 3; rgbIdx++) {
31-
float sum = xyz.get(0, rgbIdx) + xyz.get(1, rgbIdx) + xyz.get(2, rgbIdx);
32-
rgb[rgbIdx].fX = xyz.get(0, rgbIdx) / sum;
33-
rgb[rgbIdx].fY = xyz.get(1, rgbIdx) / sum;
31+
float sum = xyz.vals[rgbIdx][0] + xyz.vals[rgbIdx][1] + xyz.vals[rgbIdx][2];
32+
rgb[rgbIdx].fX = xyz.vals[rgbIdx][0] / sum;
33+
rgb[rgbIdx].fY = xyz.vals[rgbIdx][1] / sum;
3434
}
3535
}
3636

@@ -46,13 +46,12 @@ static float calculate_area(SkPoint abc[]) {
4646

4747
static constexpr float kSRGB_D50_GamutArea = 0.084f;
4848

49-
static bool is_wide_gamut(const SkColorSpace* colorSpace) {
49+
static bool is_wide_gamut(const skcms_ICCProfile& profile) {
5050
// Determine if the source image has a gamut that is wider than sRGB. If so, we
5151
// will use P3 as the output color space to avoid clipping the gamut.
52-
const SkMatrix44* toXYZD50 = colorSpace->toXYZD50();
53-
if (toXYZD50) {
52+
if (profile.has_toXYZD50) {
5453
SkPoint rgb[3];
55-
load_gamut(rgb, *toXYZD50);
54+
load_gamut(rgb, profile.toXYZD50);
5655
return calculate_area(rgb) > kSRGB_D50_GamutArea;
5756
}
5857

@@ -177,14 +176,14 @@ sk_sp<SkColorSpace> SkAndroidCodec::computeOutputColorSpace(SkColorType outputCo
177176
return prefColorSpace;
178177
}
179178

180-
SkColorSpace* encodedSpace = fCodec->getInfo().colorSpace();
181-
if (encodedSpace->isNumericalTransferFn(&fn)) {
179+
const skcms_ICCProfile* encodedProfile = fCodec->getEncodedInfo().profile();
180+
if (auto encodedSpace = SkColorSpace::Make(*encodedProfile)) {
182181
// Leave the pixels in the encoded color space. Color space conversion
183182
// will be handled after decode time.
184-
return sk_ref_sp(encodedSpace);
183+
return encodedSpace;
185184
}
186185

187-
if (is_wide_gamut(encodedSpace)) {
186+
if (is_wide_gamut(*encodedProfile)) {
188187
return SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
189188
SkColorSpace::kDCIP3_D65_Gamut);
190189
}

src/codec/SkBmpBaseCodec.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99

1010
SkBmpBaseCodec::~SkBmpBaseCodec() {}
1111

12-
SkBmpBaseCodec::SkBmpBaseCodec(int width, int height, const SkEncodedInfo& info,
13-
std::unique_ptr<SkStream> stream,
12+
SkBmpBaseCodec::SkBmpBaseCodec(SkEncodedInfo&& info, std::unique_ptr<SkStream> stream,
1413
uint16_t bitsPerPixel, SkCodec::SkScanlineOrder rowOrder)
15-
: INHERITED(width, height, info, std::move(stream), bitsPerPixel, rowOrder)
14+
: INHERITED(std::move(info), std::move(stream), bitsPerPixel, rowOrder)
1615
, fSrcBuffer(sk_malloc_canfail(this->srcRowBytes()))
1716
{}

src/codec/SkBmpBaseCodec.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class SkBmpBaseCodec : public SkBmpCodec {
2525
bool didCreateSrcBuffer() const { return fSrcBuffer != nullptr; }
2626

2727
protected:
28-
SkBmpBaseCodec(int width, int height, const SkEncodedInfo& info, std::unique_ptr<SkStream>,
28+
SkBmpBaseCodec(SkEncodedInfo&& info, std::unique_ptr<SkStream>,
2929
uint16_t bitsPerPixel, SkCodec::SkScanlineOrder rowOrder);
3030

3131
uint8_t* srcBuffer() { return reinterpret_cast<uint8_t*>(fSrcBuffer.get()); }

0 commit comments

Comments
 (0)