diff options
author | Brian Osman <brianosman@google.com> | 2017-12-15 11:50:20 -0500 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2017-12-15 19:25:08 +0000 |
commit | ef8dda227b55445de090cfc5c59fb633f8a1763d (patch) | |
tree | 78ee7c2c5d7ae6d2c2e9c16a68d015185681a2e6 | |
parent | 2d53d984251a753b9d0fb3adad3be09243cf5c14 (diff) |
Increase accuracy of float -> fixed in ICC code
Add a comment to SkFixed explaining the accuracy issues of the macros.
Bug: skia:
Change-Id: Ibfecb16821fefe87822cc3acd1cf8498df10a492
Reviewed-on: https://skia-review.googlesource.com/85742
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
-rw-r--r-- | include/private/SkFixed.h | 5 | ||||
-rw-r--r-- | src/core/SkICC.cpp | 28 |
2 files changed, 23 insertions, 10 deletions
diff --git a/include/private/SkFixed.h b/include/private/SkFixed.h index adb0343bc7..05e1de49c9 100644 --- a/include/private/SkFixed.h +++ b/include/private/SkFixed.h @@ -30,6 +30,11 @@ typedef int32_t SkFixed; #define SK_FixedTanPIOver8 (0x6A0A) #define SK_FixedRoot2Over2 (0xB505) +// NOTE: SkFixedToFloat is exact. SkFloatToFixed seems to lack a rounding step. For all fixed-point +// values, this version is as accurate as possible for (fixed -> float -> fixed). Rounding reduces +// accuracy if the intermediate floats are in the range that only holds integers (adding 0.5f to an +// odd integer then snaps to nearest even). Using double for the rounding math gives maximum +// accuracy for (float -> fixed -> float), but that's usually overkill. #define SkFixedToFloat(x) ((x) * 1.52587890625e-5f) #define SkFloatToFixed(x) sk_float_saturate2int((x) * SK_Fixed1) diff --git a/src/core/SkICC.cpp b/src/core/SkICC.cpp index 128bce368d..cfc66e1238 100644 --- a/src/core/SkICC.cpp +++ b/src/core/SkICC.cpp @@ -281,25 +281,33 @@ static constexpr uint32_t kICCTagTable[3 * kICCNumEntries] { SkEndian_SwapBE32(kTAG_cprt_Bytes), }; +// This is like SkFloatToFixed, but rounds to nearest, preserving as much accuracy as possible +// when going float -> fixed -> float (it has the same accuracy when going fixed -> float -> fixed). +// The use of double is necessary to accomodate the full potential 32-bit mantissa of the 16.16 +// SkFixed value, and so avoiding rounding problems with float. Also, see the comment in SkFixed.h. +static SkFixed float_round_to_fixed(float x) { + return sk_float_saturate2int((float)floor((double)x * SK_Fixed1 + 0.5)); +} + static void write_xyz_tag(uint32_t* ptr, const SkMatrix44& toXYZ, int col) { ptr[0] = SkEndian_SwapBE32(kXYZ_PCSSpace); ptr[1] = 0; - ptr[2] = SkEndian_SwapBE32(SkFloatToFixed(toXYZ.getFloat(0, col))); - ptr[3] = SkEndian_SwapBE32(SkFloatToFixed(toXYZ.getFloat(1, col))); - ptr[4] = SkEndian_SwapBE32(SkFloatToFixed(toXYZ.getFloat(2, col))); + ptr[2] = SkEndian_SwapBE32(float_round_to_fixed(toXYZ.getFloat(0, col))); + ptr[3] = SkEndian_SwapBE32(float_round_to_fixed(toXYZ.getFloat(1, col))); + ptr[4] = SkEndian_SwapBE32(float_round_to_fixed(toXYZ.getFloat(2, col))); } static void write_trc_tag(uint32_t* ptr, const SkColorSpaceTransferFn& fn) { ptr[0] = SkEndian_SwapBE32(kTAG_ParaCurveType); ptr[1] = 0; ptr[2] = (uint32_t) (SkEndian_SwapBE16(kGABCDEF_ParaCurveType)); - ptr[3] = SkEndian_SwapBE32(SkFloatToFixed(fn.fG)); - ptr[4] = SkEndian_SwapBE32(SkFloatToFixed(fn.fA)); - ptr[5] = SkEndian_SwapBE32(SkFloatToFixed(fn.fB)); - ptr[6] = SkEndian_SwapBE32(SkFloatToFixed(fn.fC)); - ptr[7] = SkEndian_SwapBE32(SkFloatToFixed(fn.fD)); - ptr[8] = SkEndian_SwapBE32(SkFloatToFixed(fn.fE)); - ptr[9] = SkEndian_SwapBE32(SkFloatToFixed(fn.fF)); + ptr[3] = SkEndian_SwapBE32(float_round_to_fixed(fn.fG)); + ptr[4] = SkEndian_SwapBE32(float_round_to_fixed(fn.fA)); + ptr[5] = SkEndian_SwapBE32(float_round_to_fixed(fn.fB)); + ptr[6] = SkEndian_SwapBE32(float_round_to_fixed(fn.fC)); + ptr[7] = SkEndian_SwapBE32(float_round_to_fixed(fn.fD)); + ptr[8] = SkEndian_SwapBE32(float_round_to_fixed(fn.fE)); + ptr[9] = SkEndian_SwapBE32(float_round_to_fixed(fn.fF)); } static bool is_3x3(const SkMatrix44& toXYZD50) { |