Fix color xform width bug when scaling/subsetting

This was not caught by the bots because we don't test
color correct modes with our many image decoding tests
(takes too long).

Adding a unit test.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2247743002

Review-Url: https://codereview.chromium.org/2247743002

diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index eb5f6b0..05f40bd 100644 (file)
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -495,11 +495,13 @@ int SkJpegCodec::readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes
     uint32_t* swizzleDst = (uint32_t*) dst;
     size_t decodeDstRowBytes = rowBytes;
     size_t swizzleDstRowBytes = rowBytes;
+    int dstWidth = dstInfo.width();
     if (fSwizzleSrcRow && fColorXformSrcRow) {
         decodeDst = (JSAMPLE*) fSwizzleSrcRow;
         swizzleDst = fColorXformSrcRow;
         decodeDstRowBytes = 0;
         swizzleDstRowBytes = 0;
+        dstWidth = fSwizzler->swizzleWidth();
     } else if (fColorXformSrcRow) {
         decodeDst = (JSAMPLE*) fColorXformSrcRow;
         swizzleDst = fColorXformSrcRow;
@@ -508,6 +510,7 @@ int SkJpegCodec::readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes
     } else if (fSwizzleSrcRow) {
         decodeDst = (JSAMPLE*) fSwizzleSrcRow;
         decodeDstRowBytes = 0;
+        dstWidth = fSwizzler->swizzleWidth();
     }
 
     for (int y = 0; y < count; y++) {
@@ -523,8 +526,7 @@ int SkJpegCodec::readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes
         }
 
         if (fColorXform) {
-            fColorXform->apply(dst, swizzleDst, dstInfo.width(), dstInfo.colorType(),
-                               kOpaque_SkAlphaType);
+            fColorXform->apply(dst, swizzleDst, dstWidth, dstInfo.colorType(), kOpaque_SkAlphaType);
             dst = SkTAddOffset<void>(dst, rowBytes);
         }
 
@@ -590,16 +592,19 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
 }
 
 void SkJpegCodec::allocateStorage(const SkImageInfo& dstInfo) {
+    int dstWidth = dstInfo.width();
+
     size_t swizzleBytes = 0;
     if (fSwizzler) {
         swizzleBytes = get_row_bytes(fDecoderMgr->dinfo());
+        dstWidth = fSwizzler->swizzleWidth();
         SkASSERT(!fColorXform || SkIsAlign4(swizzleBytes));
     }
 
     size_t xformBytes = 0;
     if (kRGBA_F16_SkColorType == dstInfo.colorType()) {
         SkASSERT(fColorXform);
-        xformBytes = dstInfo.width() * sizeof(uint32_t);
+        xformBytes = dstWidth * sizeof(uint32_t);
     }
 
     size_t totalBytes = swizzleBytes + xformBytes;

diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 0d13e6a..2b52ab7 100644 (file)
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -358,9 +358,8 @@ static bool png_conversion_possible(const SkImageInfo& dst, const SkImageInfo& s
     }
 }
 
-void SkPngCodec::allocateStorage(const SkImageInfo& dstInfo) {
-    const int width = this->getInfo().width();
-    size_t colorXformBytes = fColorXform ? width * sizeof(uint32_t) : 0;
+void SkPngCodec::allocateStorage() {
+    size_t colorXformBytes = fColorXform ? fSwizzler->swizzleWidth() * sizeof(uint32_t) : 0;
 
     fStorage.reset(SkAlign4(fSrcRowBytes) + colorXformBytes);
     fSwizzlerSrcRow = fStorage.get();
@@ -384,7 +383,7 @@ public:
             return kInvalidConversion;
         }
 
-        this->allocateStorage(dstInfo);
+        this->allocateStorage();
         return kSuccess;
     }
 
@@ -413,7 +412,7 @@ public:
             fSwizzler->swizzle(swizzlerDstRow, fSwizzlerSrcRow);
 
             if (fColorXform) {
-                fColorXform->apply(dst, (const uint32_t*) swizzlerDstRow, dstInfo.width(),
+                fColorXform->apply(dst, (const uint32_t*) swizzlerDstRow, fSwizzler->swizzleWidth(),
                                    dstInfo.colorType(), xformAlphaType);
                 dst = SkTAddOffset<void>(dst, rowBytes);
             }
@@ -464,7 +463,7 @@ public:
             return kInvalidConversion;
         }
 
-        this->allocateStorage(dstInfo);
+        this->allocateStorage();
         fCanSkipRewind = true;
         return SkCodec::kSuccess;
     }
@@ -515,8 +514,9 @@ public:
 
             if (fColorXform) {
                 if (fColorXform) {
-                    fColorXform->apply(dst, (const uint32_t*) swizzlerDstRow, dstInfo.width(),
-                                       dstInfo.colorType(), xformAlphaType);
+                    fColorXform->apply(dst, (const uint32_t*) swizzlerDstRow,
+                                       fSwizzler->swizzleWidth(), dstInfo.colorType(),
+                                       xformAlphaType);
                     dst = SkTAddOffset<void>(dst, rowBytes);
                 }
             }
@@ -865,7 +865,7 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
         return kUnimplemented;
     }
 
-    this->allocateStorage(dstInfo);
+    this->allocateStorage();
     int count = this->readRows(dstInfo, dst, rowBytes, dstInfo.height(), 0);
     if (count > dstInfo.height()) {
         *rowsDecoded = count;

diff --git a/src/codec/SkPngCodec.h b/src/codec/SkPngCodec.h
index b689f6f..9d97c71 100644 (file)
--- a/src/codec/SkPngCodec.h
+++ b/src/codec/SkPngCodec.h
@@ -41,7 +41,7 @@ protected:
         SkASSERT(fSwizzler);
         return fSwizzler;
     }
-    void allocateStorage(const SkImageInfo& dstInfo);
+    void allocateStorage();
 
     virtual int readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count,
                          int startRow) = 0;

diff --git a/src/codec/SkSwizzler.h b/src/codec/SkSwizzler.h
index 4845047..6172af5 100644 (file)
--- a/src/codec/SkSwizzler.h
+++ b/src/codec/SkSwizzler.h
@@ -72,6 +72,12 @@ public:
      */
     int sampleX() const { return fSampleX; }
 
+    /**
+     *  Returns the actual number of pixels written to destination memory, taking
+     *  scaling, subsetting, and partial frames into account.
+     */
+    int swizzleWidth() const { return fSwizzleWidth; }
+
 private:
 
     /**

diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index a6b44eb..8023ff2 100644 (file)
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1044,6 +1044,36 @@ DEF_TEST(Codec_jpeg_rewind, r) {
     REPORTER_ASSERT(r, SkCodec::kSuccess == result);
 }
 
+static void check_color_xform(skiatest::Reporter* r, const char* path) {
+    SkAutoTDelete<SkAndroidCodec> codec(SkAndroidCodec::NewFromStream(resource(path)));
+
+    SkAndroidCodec::AndroidOptions opts;
+    opts.fSampleSize = 3;
+    const int subsetWidth = codec->getInfo().width() / 2;
+    const int subsetHeight = codec->getInfo().height() / 2;
+    SkIRect subset = SkIRect::MakeWH(subsetWidth, subsetHeight);
+    opts.fSubset = &subset;
+
+    const int dstWidth = subsetWidth / opts.fSampleSize;
+    const int dstHeight = subsetHeight / opts.fSampleSize;
+    sk_sp<SkData> data = SkData::MakeFromFileName(
+            GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
+    sk_sp<SkColorSpace> colorSpace = SkColorSpace::NewICC(data->data(), data->size());
+    SkImageInfo dstInfo = codec->getInfo().makeWH(dstWidth, dstHeight)
+                                          .makeColorType(kN32_SkColorType)
+                                          .makeColorSpace(colorSpace);
+
+    size_t rowBytes = dstInfo.minRowBytes();
+    SkAutoMalloc pixelStorage(dstInfo.getSafeSize(rowBytes));
+    SkCodec::Result result = codec->getAndroidPixels(dstInfo, pixelStorage.get(), rowBytes, &opts);
+    REPORTER_ASSERT(r, SkCodec::kSuccess == result);
+}
+
+DEF_TEST(Codec_ColorXform, r) {
+    check_color_xform(r, "mandrill_512_q075.jpg");
+    check_color_xform(r, "mandrill_512.png");
+}
+
 DEF_TEST(Codec_Png565, r) {
     // Create an arbitrary 565 bitmap.
     const char* path = "mandrill_512_q075.jpg";