From 303cfd4b16eb4e9c36e2e021d9577f0d53130674 Mon Sep 17 00:00:00 2001 From: raftias Date: Mon, 21 Nov 2016 12:59:36 -0500 Subject: [PATCH] Fixed issue with some A2B0 images being too dark The issue was with the A2B0 matrix not being scaled by its encoding factor when it needed to be (for A2B0 matrices only). NOTRY=true NOTREECHECKS=true BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=5003 Change-Id: I3f202323e137e1b014e564bd96d87c601c4748ab Reviewed-on: https://skia-review.googlesource.com/5003 Commit-Queue: Robert Aftias Reviewed-by: Matt Sarett Reviewed-on: https://skia-review.googlesource.com/5110 --- src/core/SkColorSpace_ICC.cpp | 92 ++++++++++++++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 22 deletions(-) diff --git a/src/core/SkColorSpace_ICC.cpp b/src/core/SkColorSpace_ICC.cpp index 37c19bf..e365e8a 100644 --- a/src/core/SkColorSpace_ICC.cpp +++ b/src/core/SkColorSpace_ICC.cpp @@ -674,30 +674,78 @@ static bool load_color_lut(sk_sp* colorLUT, uint32_t inputCh return true; } -static bool load_matrix(SkMatrix44* toXYZ, const uint8_t* src, size_t len) { - if (len < 48) { +/** + * Reads a matrix out of an A2B tag of an ICC profile. + * If |translate| is true, it will load a 3x4 matrix out that corresponds to a XYZ + * transform as well as a translation, and if |translate| is false it only loads a + * 3x3 matrix with no translation + * + * @param matrix The matrix to store the result in + * @param src Data to load the matrix out of. + * @param len The length of |src|. + * Must have 48 bytes if |translate| is set and 36 bytes otherwise. + * @param translate Whether to read the translation column or not + * @param pcs The profile connection space of the profile this matrix is for + * + * @return false on failure, true on success + */ +static bool load_matrix(SkMatrix44* matrix, const uint8_t* src, size_t len, bool translate, + SkColorSpace_A2B::PCS pcs) { + const size_t minLen = translate ? 48 : 36; + if (len < minLen) { SkColorSpacePrintf("Matrix tag is too small (%d bytes).", len); return false; } + float encodingFactor; + switch (pcs) { + case SkColorSpace_A2B::PCS::kLAB: + encodingFactor = 1.f; + break; + case SkColorSpace_A2B::PCS::kXYZ: + encodingFactor = 65535 / 32768.f; + break; + default: + encodingFactor = 1.f; + SkASSERT(false); + break; + } float array[16]; - array[ 0] = SkFixedToFloat(read_big_endian_i32(src)); - array[ 1] = SkFixedToFloat(read_big_endian_i32(src + 4)); - array[ 2] = SkFixedToFloat(read_big_endian_i32(src + 8)); - array[ 3] = SkFixedToFloat(read_big_endian_i32(src + 36)); // translate R - array[ 4] = SkFixedToFloat(read_big_endian_i32(src + 12)); - array[ 5] = SkFixedToFloat(read_big_endian_i32(src + 16)); - array[ 6] = SkFixedToFloat(read_big_endian_i32(src + 20)); - array[ 7] = SkFixedToFloat(read_big_endian_i32(src + 40)); // translate G - array[ 8] = SkFixedToFloat(read_big_endian_i32(src + 24)); - array[ 9] = SkFixedToFloat(read_big_endian_i32(src + 28)); - array[10] = SkFixedToFloat(read_big_endian_i32(src + 32)); - array[11] = SkFixedToFloat(read_big_endian_i32(src + 44)); // translate B + array[ 0] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src)); + array[ 1] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 4)); + array[ 2] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 8)); + + array[ 4] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 12)); + array[ 5] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 16)); + array[ 6] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 20)); + + array[ 8] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 24)); + array[ 9] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 28)); + array[10] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 32)); + + if (translate) { + array[ 3] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 36)); // translate R + array[ 7] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 40)); // translate G + array[11] = encodingFactor * SkFixedToFloat(read_big_endian_i32(src + 44)); // translate B + } else { + array[ 3] = 0.0f; + array[ 7] = 0.0f; + array[11] = 0.0f; + } + array[12] = 0.0f; array[13] = 0.0f; array[14] = 0.0f; array[15] = 1.0f; - toXYZ->setRowMajorf(array); + matrix->setRowMajorf(array); + SkColorSpacePrintf("A2B0 matrix loaded:\n"); + for (int r = 0; r < 4; ++r) { + SkColorSpacePrintf("|"); + for (int c = 0; c < 4; ++c) { + SkColorSpacePrintf(" %f ", matrix->get(r, c)); + } + SkColorSpacePrintf("|\n"); + } return true; } @@ -819,7 +867,7 @@ static bool parse_and_load_gamma(SkGammaNamed* gammaNamed, sk_sp* gamm } bool load_a2b0_a_to_b_type(std::vector* elements, const uint8_t* src, - size_t len) { + size_t len, SkColorSpace_A2B::PCS pcs) { // Read the number of channels. The four bytes (4-7) that we skipped are reserved and // must be zero. const uint8_t inputChannels = src[8]; @@ -832,7 +880,7 @@ bool load_a2b0_a_to_b_type(std::vector* elements, con SkColorSpacePrintf("Input and output channels must equal 3 in A to B tag.\n"); return false; } - + // It is important that these are loaded in the order of application, as the // order you construct an A2B color space's elements is the order it is applied @@ -881,13 +929,13 @@ bool load_a2b0_a_to_b_type(std::vector* elements, con const uint32_t offsetToMatrix = read_big_endian_i32(src + 16); if (0 != offsetToMatrix && offsetToMatrix < len) { SkMatrix44 matrix(SkMatrix44::kUninitialized_Constructor); - if (!load_matrix(&matrix, src + offsetToMatrix, len - offsetToMatrix)) { + if (!load_matrix(&matrix, src + offsetToMatrix, len - offsetToMatrix, true, pcs)) { SkColorSpacePrintf("Failed to read matrix from A to B tag.\n"); } else { elements->push_back(SkColorSpace_A2B::Element(matrix)); } } - + const uint32_t offsetToBCurves = read_big_endian_i32(src + 12); if (0 != offsetToBCurves && offsetToBCurves < len) { const size_t tagLen = len - offsetToBCurves; @@ -907,7 +955,7 @@ bool load_a2b0_a_to_b_type(std::vector* elements, con } static bool load_a2b0(std::vector* elements, const uint8_t* src, - size_t len) { + size_t len, SkColorSpace_A2B::PCS pcs) { const uint32_t type = read_big_endian_u32(src); switch (type) { case kTAG_AtoBType: @@ -916,7 +964,7 @@ static bool load_a2b0(std::vector* elements, const ui return false; } SkColorSpacePrintf("A2B0 tag is of type lutAtoBType\n"); - return load_a2b0_a_to_b_type(elements, src, len); + return load_a2b0_a_to_b_type(elements, src, len, pcs); default: SkColorSpacePrintf("Unsupported A to B tag type: %c%c%c%c\n", (type>>24)&0xFF, (type>>16)&0xFF, (type>>8)&0xFF, type&0xFF); @@ -1157,7 +1205,7 @@ sk_sp SkColorSpace::MakeICC(const void* input, size_t len) { ? SkColorSpace_A2B::PCS::kXYZ : SkColorSpace_A2B::PCS::kLAB; std::vector elements; - if (!load_a2b0(&elements, a2b0->addr(base), a2b0->fLength)) { + if (!load_a2b0(&elements, a2b0->addr(base), a2b0->fLength, pcs)) { return_null("Failed to parse A2B0 tag"); } return sk_sp(new SkColorSpace_A2B(pcs, std::move(data), -- 2.7.4