Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2426723005:
Use SkColorSpaceXform to handle color conversions in decoders (Closed)
DescriptionUse SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. The plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
NO_DEPENDENCY_CHECKS=true
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Committed: https://crrev.com/8de5bb3d8b53791711e7c387dc736817dd8947a7
Cr-Original-Commit-Position: refs/heads/master@{#426588}
Cr-Commit-Position: refs/heads/master@{#426858}
Patch Set 1 #Patch Set 2 : Remove ifdefs - fixes blink_platform_unittests #
Total comments: 12
Patch Set 3 : Fix png decodes #Patch Set 4 : Response to comments #
Total comments: 8
Patch Set 5 : Refactor a bit for clarity #
Total comments: 1
Patch Set 6 : Rebaselines #Patch Set 7 : More rebaselines #Patch Set 8 : Rebase #Depends on Patchset: Messages
Total messages: 75 (56 generated)
Description was changed from
==========
Use SkColorSpaceXform to handle color space conversions in decoders
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Matching Color Space Optimization:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Measures:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Correctness Measures:
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Matching Color Space Optimization:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Measures:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Correctness Measures:
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Matching Color Space Optimization:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Measures:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Correctness Measures:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Matching Color Space Optimization:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Measures:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Correctness Measures:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB
2.2
Other
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(sRGB is actually the fastest xform here - I just had to
drop a bunch of images that tripped up QCMS from the sRGB
average. SkColorSpaceXform was recognizing sRGB->sRGB
was a no-op, so the comparisons skewed the results in
its favor, we were seeing like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function (~90%), 2.2 (~2%),
other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(sRGB is actually the fastest xform here - I just had to
drop a bunch of images that tripped up QCMS from the sRGB
average. SkColorSpaceXform was recognizing sRGB->sRGB
was a no-op, so the comparisons skewed the results in
its favor, we were seeing like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(sRGB is actually the fastest xform here - I just had to
drop a bunch of images that tripped up QCMS from the sRGB
average. SkColorSpaceXform was recognizing sRGB->sRGB
was a no-op, so the comparisons skewed the results in
its favor, we were seeing like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other (~8%).
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(sRGB is actually the fastest xform here - I just had to
drop a bunch of images that tripped up QCMS from the sRGB
average. SkColorSpaceXform was recognizing sRGB->sRGB
was a no-op, so the comparisons skewed the results in
its favor, we were seeing like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(sRGB is actually the fastest xform here - I just had to
drop a bunch of images that tripped up QCMS from the sRGB
average. SkColorSpaceXform was recognizing sRGB->sRGB
was a no-op, so the comparisons skewed the results in
its favor, we were seeing like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(sRGB is actually the fastest xform here - I just had to
drop a bunch of images that tripped up QCMS from the sRGB
average. SkColorSpaceXform was recognizing sRGB->sRGB
was a no-op, so the comparisons skewed the results in
its favor, we were seeing like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being xformed inside Skia, but
it's worth mentioning that this change makes it simpler
to do this xform at decode time if we want.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as small LUT.
QCMS does not recognize that we have sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
Patchset #2 (id:20001) has been deleted
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpaceXform builds
in a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
Patchset #2 (id:40001) has been deleted
Patchset #1 (id:1) has been deleted
The CQ bit was checked by msarett@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 checked by msarett@google.com to run a CQ dry run
Patchset #1 (id:60001) has been deleted
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: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
msarett@google.com changed reviewers: + ccameron@chromium.org, scroggo@chromium.org
I anticipate needing to do a bunch of rebaselines and test diagnosing. But I think the core part of this change is ready to be reviewed.
> Turns color xforms on for Android I wonder if this should be a separate change, just to be conservative. I think that would mean leaving around some of the ifdefs (with a different macro) to keep correction off of Android. > More lenient approval of ICC profiles. QCMS rejects > xform matrices that are not "close enough to D50" by > some arbitrary measure. We don't have enough data to > know that this is or isn't necessary. QCMS also > rejects profiles that it thinks are "too big". Again, > this seems reasonable, but we'll wait until we > understand the motivation better. Again, I wonder if we should be conservative. If these *do* cause problems, we can revert the changes that stopped rejecting big profiles and far from D50 without reverting the whole change. https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp:334: SpinLock gTargetColorSpaceLock; Why not leave these in an anonymous namespace? https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp:351: SkColorSpace::NewICC(profile.data(), profile.size()).release(); nit: Can this fit on one line? https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp:393: gTargetColorSpace = nit: Can this fit on one line? https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:240: // FIXME: nit: I think this should be: TODO (msarett): Same for other FIXMEs https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:387: // We cannot do this because, when we apply the gamma encoding after nit: No need for comma in between "because" and "when" https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:418: // written the ImageFrame, purely because SkColorSpaceXform supports RGBA nit: to* the ImageFrame
Awesome! I agree with the suggestion that we should make this change on existing platforms and enable it on Android in another pass. I'd suggest doing this by just leaving some of the "#if QCMS" bits in place, and removing them in a follow-on. Other than that, lgtm % scroggo's comments.
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Behavior Changes:
*** Turns color xforms on for Android. SkColorSpaceXform
has Arm NEON opt code (in addition to SSE2), so
performance on Android devices is no longer blocking
this.
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
> > Turns color xforms on for Android > I wonder if this should be a separate change, just to be conservative. I think > that would mean leaving around some of the ifdefs (with a different macro) to > keep correction off of Android. Yes I agree! I hate the ifdefs, but this makes more sense as a separate change. Turning them back on. > More lenient approval of ICC profiles. QCMS rejects > xform matrices that are not "close enough to D50" by > some arbitrary measure. We don't have enough data to > know that this is or isn't necessary. QCMS also > rejects profiles that it thinks are "too big". Again, > this seems reasonable, but we'll wait until we > understand the motivation better. > Again, I wonder if we should be conservative. If these *do* cause problems, we > can revert the changes that stopped rejecting big profiles and far from D50 > without reverting the whole change. I used to agree here as well. I tried to land these checks in Skia, but nobody liked them :). https://codereview.chromium.org/2257983003 FWIW, Chrome has been using SkColorSpace::NewICC() for a while now and there have been no complaints (about this). That makes me a little less concerned. Maybe it's enough to just have this CL ready? Or I can take another shot at landing it? https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp:334: SpinLock gTargetColorSpaceLock; On 2016/10/19 18:26:25, scroggo_chromium wrote: > Why not leave these in an anonymous namespace? Didn't mean to remove this, adding it back. https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp:351: SkColorSpace::NewICC(profile.data(), profile.size()).release(); On 2016/10/19 18:26:25, scroggo_chromium wrote: > nit: Can this fit on one line? Autoformatter actually keeps forcing me to less than 80 chars. https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp:393: gTargetColorSpace = On 2016/10/19 18:26:25, scroggo_chromium wrote: > nit: Can this fit on one line? Autoformatter puts it on two lines. https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:240: // FIXME: On 2016/10/19 18:26:25, scroggo_chromium wrote: > nit: I think this should be: > > TODO (msarett): > > Same for other FIXMEs Done. https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:387: // We cannot do this because, when we apply the gamma encoding after On 2016/10/19 18:26:25, scroggo_chromium wrote: > nit: No need for comma in between "because" and "when" Done. https://codereview.chromium.org/2426723005/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:418: // written the ImageFrame, purely because SkColorSpaceXform supports RGBA On 2016/10/19 18:26:25, scroggo_chromium wrote: > nit: to* the ImageFrame Done.
> > Again, I wonder if we should be conservative. If these *do* cause problems, we > > can revert the changes that stopped rejecting big profiles and far from D50 > > without reverting the whole change. > > I used to agree here as well. I tried to land these checks > in Skia, but nobody liked them :). > https://codereview.chromium.org/2257983003 > > FWIW, Chrome has been using SkColorSpace::NewICC() for > a while now and there have been no complaints (about this). > That makes me a little less concerned. > > Maybe it's enough to just have this CL ready? Or I can > take another shot at landing it? If we cannot determine why QCMS made those limitations (maybe it was due to their internal implementation? maybe it was just in case?), I'm fine with landing as is. FWIW, rejecting "too big" profiles seems analogous to rejecting "too big" images, which we've decided not to do, so it seems in keeping to not do that. A designer should keep that in mind. Close to D50 probably made QCMS able to restrict their code, rather than make anything safer. lgtm https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:374: ImageFrame::PixelData* address = buffer.getAddr(0, y); The variables "address" and "addr" could be easy to mix up. Maybe make this one an ImageFrame::PixelData* const addr ? https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:388: // We cannot do this because when we apply the gamma encoding after Is there an actual TODO here? Or should is this comment just an acknowledgement that this is not "correct", but we don't see a way to do it better? https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:414: ImageFrame::PixelData* addr = address; Another way to distinguish addr from address - you could put this declaration and the for loop inside its own scope. Or you could put addr inside the for loop, e.g. for (auto* addr = address; addr < address + width; addr++, pixel += 3) { (This also lets you drop the variable "x".) (A counter-argument to this suggestion - I initially wrote "addr < addr + width", which would loop infinitely :( But that should be caught pretty quickly, and with distinguished names maybe it would be harder to make that mistake...) In addition/instead, you could name them something like "rowAddress" and "pixelAddress". https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoder.cpp:367: // FIXME: nit: TODO (msarett):
The CQ bit was checked by msarett@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...
https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:374: ImageFrame::PixelData* address = buffer.getAddr(0, y); On 2016/10/20 12:45:04, scroggo_chromium wrote: > The variables "address" and "addr" could be easy to mix up. Maybe make this one > an > > ImageFrame::PixelData* const addr > > ? Done. https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:388: // We cannot do this because when we apply the gamma encoding after On 2016/10/20 12:45:04, scroggo_chromium wrote: > Is there an actual TODO here? Or should is this comment just an acknowledgement > that this is not "correct", but we don't see a way to do it better? It's the latter. I'll drop the TODO. https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:414: ImageFrame::PixelData* addr = address; On 2016/10/20 12:45:04, scroggo_chromium wrote: > Another way to distinguish addr from address - you could put this declaration > and the for loop inside its own scope. Or you could put addr inside the for > loop, e.g. > > for (auto* addr = address; addr < address + width; addr++, pixel += 3) { > > (This also lets you drop the variable "x".) > (A counter-argument to this suggestion - I initially wrote "addr < addr + > width", which would loop infinitely :( But that should be caught pretty quickly, > and with distinguished names maybe it would be harder to make that mistake...) > > In addition/instead, you could name them something like "rowAddress" and > "pixelAddress". Changed the style of the loop. Using dstRow and dstPixel. https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/90007/third_party/WebKit/Sour... third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoder.cpp:367: // FIXME: On 2016/10/20 12:45:04, scroggo_chromium wrote: > nit: TODO (msarett): Done.
msarett@google.com changed reviewers: + dpranke@chromium.org
dpranke@ Can you please take a look at third_party/WebKit/Source/config.gni?
lgtm https://codereview.chromium.org/2426723005/diff/150001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp (right): https://codereview.chromium.org/2426723005/diff/150001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp:408: for (auto *dstPixel = dstRow; dstPixel < dstRow + width; nit: * goes next to the type: auto* dstPixel
The CQ bit was checked by msarett@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...
Patchset #6 (id:170001) has been deleted
Patchset #6 (id:190001) has been deleted
The CQ bit was checked by msarett@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...
config.gni lgtm.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by msarett@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: This issue passed the CQ dry run.
The CQ bit was checked by msarett@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from ccameron@chromium.org, scroggo@chromium.org, dpranke@chromium.org Link to the patchset: https://codereview.chromium.org/2426723005/#ps230001 (title: "More rebaselines")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
Message was sent while issue was closed.
Committed patchset #7 (id:230001)
Message was sent while issue was closed.
A revert of this CL (patchset #7 id:230001) has been created in https://chromiumcodereview.appspot.com/2436223002/ by pkasting@chromium.org. The reason for reverting is: I believe this is the (indirect) cause of a crash on Linux MSAN bot due to use of uninitialized variable: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Precis... I imagine this is merely exposing this bug in some existing not-currently-exercised code. But if the variable is really used uninitialized, this could result in test flakiness as well..
Message was sent while issue was closed.
lgtm
Message was sent while issue was closed.
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
Message was sent while issue was closed.
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Cr-Commit-Position: refs/heads/master@{#426588}
==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797 Cr-Commit-Position: refs/heads/master@{#426588}
Message was sent while issue was closed.
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Cr-Commit-Position: refs/heads/master@{#426588}
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Cr-Commit-Position: refs/heads/master@{#426588}
==========
I believe I've made the MSAN fix in Skia. https://skia-review.googlesource.com/c/3800/ Once this rolls into Chrome, I plan to verify the MSAN bot and reland.
The CQ bit was checked by msarett@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: This issue passed the CQ dry run.
The CQ bit was checked by msarett@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@chromium.org, dpranke@chromium.org, ccameron@chromium.org Link to the patchset: https://codereview.chromium.org/2426723005/#ps250001 (title: "Rebase")
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 2426723005 Patch 230001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. THe plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Cr-Commit-Position: refs/heads/master@{#426588}
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. The plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
NO_DEPENDENCY_CHECKS=true
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Cr-Commit-Position: refs/heads/master@{#426588}
==========
The CQ bit was checked by msarett@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. The plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
NO_DEPENDENCY_CHECKS=true
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Cr-Commit-Position: refs/heads/master@{#426588}
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. The plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
NO_DEPENDENCY_CHECKS=true
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Cr-Commit-Position: refs/heads/master@{#426588}
==========
Message was sent while issue was closed.
Committed patchset #8 (id:250001)
Message was sent while issue was closed.
Description was changed from
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. The plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
NO_DEPENDENCY_CHECKS=true
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Cr-Commit-Position: refs/heads/master@{#426588}
==========
to
==========
Use SkColorSpaceXform to handle color conversions in decoders
Note on matching color spaces:
*** Roughly 90% of image profiles specify sRGB. Most
dsts are also sRGB (and so is our default when we
don't know the dst). In the sRGB->sRGB case (and in
other cases where the src and dst match), we should
skip the xform. QCMS checks for matching profiles,
but the check is very weak (only works if profiles have
the same text description). SkColorSpace has
a robust check for color space equality.
Performance Notes:
***Test Srcs: Averaged across 97 images pulled from the top
10k skps.
***Test Dsts: sRGB transfer function, 2.2, other.
HP z620 Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 1.51x
2.2 1.61x
Other 1.44x
(SkColorSpaceXform is actually fastest for sRGB - I just
had to drop a bunch of images from the sRGB average,
including a few that tripped up QCMS. SkColorSpaceXform
was recognizing sRGB->sRGB was a no-op, so including these
images skewed the results in its favor - I saw like 17x)
Nexus 6P Average Xform Speed-Up (QCMS / SkColorSpaceXform)
sRGB 2.08x
2.2 1.42x
Other 3.33x
(interesting that the fallback code is actually faster than
the special cases on Arm - likely this is because the
sqrt instructions that we're using to model the curves are
slow on this platform - would be interesting to investigate
further to see if we can do better)
*** Note that turning this on for Android is left to a
later CL.
Correctness Notes:
***QCMS (and the Skia fallback code path) use a LUT to
convert linear floats to the dst transfer function.
This results in a loss of accuracy since the float
must be rounded to an int before being used as an
index into the table. Both Skia and QCMS do not
interpolate the table for performance reasons.
Skia avoids table-based implementations for two
common cases (sRGB, 2.2), guaranteeing a result
within 1 (on a 0-255 scale) of the "true" value.
QCMS may be off by as much as 5 on the steep 2.2
curve.
***There are several fairly common sRGB profiles that
represent the transfer function as a small LUT.
QCMS does not recognize that it is sRGB, and
interpolates this LUT to build a larger LUT that
it then uses. This results in the conversion being
off from the "true" value by as much as 9 (on a
0-255 scale).
Other Behavior Changes:
*** More lenient approval of ICC profiles. QCMS rejects
xform matrices that are not "close enough to D50" by
some arbitrary measure. We don't have enough data to
know that this is or isn't necessary. QCMS also
rejects profiles that it thinks are "too big". Again,
this seems reasonable, but we'll wait until we
understand the motivation better.
What ICC profiles are supported?
***SkColorSpaceXform supports the same subset of profiles
as QCMS. Adding support for A2B and Lab profiles is
a WIP (We had support for a few of these, but we've
dropped it as part of a refactor. The plan is to add
it back as a part of a more logical, complete design).
How does this relate to color correct rendering for Chrome?
***This modifies the legacy path, not the color correct
path.
***It's still somewhat interestingly related to color
correct rendering, since the color correct renderer
will also use SkColorSpaceXform in some cases
(likely just to xform scary profiles into linear F16).
***It looks like the color correct rendering path will
rely on tagged images being handled inside Skia, but
it's worth mentioning that this change makes it simpler
to do flexible xforms (ex: to F16) at decode time.
NO_DEPENDENCY_CHECKS=true
BUG=
Committed: https://crrev.com/8602b7fdc54d18aff914b5ddf0bba2a406078797
Committed: https://crrev.com/8de5bb3d8b53791711e7c387dc736817dd8947a7
Cr-Original-Commit-Position: refs/heads/master@{#426588}
Cr-Commit-Position: refs/heads/master@{#426858}
==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/8de5bb3d8b53791711e7c387dc736817dd8947a7 Cr-Commit-Position: refs/heads/master@{#426858} |
