Skip memcpy() swizzles in SkPngCodec

src/codec/SkPngCodec.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
 #include "SkColorTable.h"
@@ skipping 408 matching lines @@
                     fSrcConfig = SkSwizzler::kRGBA;
                 }
             }
             break;
         }
         default:
             // We will always recommend one of the above colorTypes.
             SkASSERT(false);
     }
 
+    // If we are not performing a subset decode, and the swizzle will not change the
+    // number of bytes per pixel, we can swizzle in place.
+    fInPlaceSwizzle = (SkSwizzler::BytesPerPixel(fSrcConfig) ==
+            SkColorTypeBytesPerPixel(requestedInfo.colorType()) && !options.fSubset);
+
     // Copy the color table to the client if they request kIndex8 mode
     copy_color_table(requestedInfo, fColorTable, ctable, ctableCount);
 
     // Create the swizzler.  SkPngCodec retains ownership of the color table.
     const SkPMColor* colors = get_color_ptr(fColorTable.get());
     fSwizzler.reset(SkSwizzler::CreateSwizzler(fSrcConfig, colors, requestedInfo, options));
     SkASSERT(fSwizzler);
 
     return kSuccess;
 }
@@ skipping 29 matching lines @@
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
     }
 
     // Note that ctable and ctableCount may be modified if there is a color table
     const Result result = this->initializeSwizzler(requestedInfo, options, ctable, ctableCount);
     if (result != kSuccess) {
         return result;
     }
+
+    const int width = requestedInfo.width();
+    const int height = requestedInfo.height();
+    const int bpp = SkSwizzler::BytesPerPixel(fSrcConfig);
+
     // FIXME: Could we use the return value of setjmp to specify the type of
     // error?
     int row = 0;
     // This must be declared above the call to setjmp to avoid memory leaks on incomplete images.
     SkAutoTMalloc<uint8_t> storage;
     if (setjmp(png_jmpbuf(fPng_ptr))) {
         // Assume that any error that occurs while reading rows is caused by an incomplete input.
         if (fNumberPasses > 1) {
             // FIXME (msarett): Handle incomplete interlaced pngs.
-            return kInvalidInput;
+            return row == height ? kSuccess : kInvalidInput;
         }
         // FIXME: We do a poor job on incomplete pngs compared to other decoders (ex: Chromium,
         // Ubuntu Image Viewer).  This is because we use the default buffer size in libpng (8192
         // bytes), and if we can't fill the buffer, we immediately fail.
         // For example, if we try to read 8192 bytes, and the image (incorrectly) only contains
         // half that, which may have been enough to contain a non-zero number of lines, we fail
         // when we could have decoded a few more lines and then failed.
         // The read function that we provide for libpng has no way of indicating that we have
         // made a partial read.
         // Making our buffer size smaller improves our incomplete decodes, but what impact does
         // it have on regular decode performance?  Should we investigate using a different API
         // instead of png_read_row(s)?  Chromium uses png_process_data.
         *rowsDecoded = row;
-        return kIncompleteInput;
+        return row == height ? kSuccess : kIncompleteInput;
     }
 
     // FIXME: We could split these out based on subclass.
     void* dstRow = dst;
     if (fNumberPasses > 1) {
-        const int width = requestedInfo.width();
-        const int height = requestedInfo.height();
-        const int bpp = SkSwizzler::BytesPerPixel(fSrcConfig);
-        const size_t srcRowBytes = width * bpp;
-
-        storage.reset(width * height * bpp);
-        uint8_t* const base = storage.get();
+        uint8_t* const base = fInPlaceSwizzle ? (uint8_t*) dstRow :
+                storage.reset(width * height * bpp);
+        const size_t srcRowBytes = fInPlaceSwizzle ? dstRowBytes : width * bpp;
 
         for (int i = 0; i < fNumberPasses; i++) {
             uint8_t* srcRow = base;
             for (int y = 0; y < height; y++) {
-                uint8_t* bmRow = srcRow;
-                png_read_rows(fPng_ptr, &bmRow, nullptr, 1);
+                png_read_rows(fPng_ptr, &srcRow, nullptr, 1);
                 srcRow += srcRowBytes;
             }
         }
 
         // Now swizzle it.
         uint8_t* srcRow = base;
         for (int y = 0; y < height; y++) {
             fSwizzler->swizzle(dstRow, srcRow);

[msarett, 2016/02/05 20:48:18]

Problem:

I think this will still perform a function call, even if the function does
nothing.

             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
             srcRow += srcRowBytes;
         }
     } else {
-        storage.reset(requestedInfo.width() * SkSwizzler::BytesPerPixel(fSrcConfig));
-        uint8_t* srcRow = storage.get();
+        uint8_t* srcRow = fInPlaceSwizzle ? (uint8_t*) dstRow : storage.reset(width * bpp);
+        const size_t srcRowBytes = fInPlaceSwizzle ? dstRowBytes : 0;
         for (; row < requestedInfo.height(); row++) {
             png_read_rows(fPng_ptr, &srcRow, nullptr, 1);
-            // FIXME: Only call IsOpaque once, outside the loop. Same for onGetScanlines.
             fSwizzler->swizzle(dstRow, srcRow);
             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
+            srcRow += srcRowBytes;
         }
     }
 
-    if (setjmp(png_jmpbuf(fPng_ptr))) {
-        // We've already read all the scanlines. This is a success.
-        return kSuccess;
-    }
-
     // read rest of file, and get additional comment and time chunks in info_ptr
     png_read_end(fPng_ptr, fInfo_ptr);
 
     return kSuccess;
 }
 
 uint32_t SkPngCodec::onGetFillValue(SkColorType colorType) const {
     const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
     if (colorPtr) {
         return get_color_table_fill_value(colorType, colorPtr, 0);
@@ skipping 31 matching lines @@
     int onGetScanlines(void* dst, int count, size_t rowBytes) override {
         // Assume that an error in libpng indicates an incomplete input.
         int row = 0;
         if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
             return row;
         }
 
         void* dstRow = dst;
         for (; row < count; row++) {
-            png_read_rows(this->png_ptr(), &fSrcRow, nullptr, 1);
-            this->swizzler()->swizzle(dstRow, fSrcRow);
+            uint8_t* srcRow = this->inPlaceSwizzle() ? (uint8_t*) dstRow : fSrcRow;
+            png_read_rows(this->png_ptr(), &srcRow, nullptr, 1);
+            this->swizzler()->swizzle(dstRow, srcRow);
             dstRow = SkTAddOffset<void>(dstRow, rowBytes);
         }
 
         return row;
     }
 
     bool onSkipScanlines(int count) override {
         // Assume that an error in libpng indicates an incomplete input.
         if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
@@ skipping 73 matching lines @@
             this->updateCurrScanline(currScanline);
         }
 
         if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
             // FIXME (msarett): Returning 0 is pessimistic.  If we can complete a single pass,
             // we may be able to report that all of the memory has been initialized.  Even if we
             // fail on the first pass, we can still report than some scanlines are initialized.
             return 0;
         }
-        SkAutoTMalloc<uint8_t> storage(count * fSrcRowBytes);
-        uint8_t* storagePtr = storage.get();
+        SkAutoTMalloc<uint8_t> storage;
+        uint8_t* const base = this->inPlaceSwizzle() ? (uint8_t*) dst :
+                storage.reset(count * fSrcRowBytes);
+        const size_t srcRowBytes = this->inPlaceSwizzle() ? dstRowBytes : fSrcRowBytes;
         uint8_t* srcRow;
         const int startRow = this->nextScanline();
         for (int i = 0; i < this->numberPasses(); i++) {
             // read rows we planned to skip into garbage row
             for (int y = 0; y < startRow; y++){
                 png_read_rows(this->png_ptr(), &fGarbageRowPtr, nullptr, 1);
             }
             // read rows we care about into buffer
-            srcRow = storagePtr;
+            uint8_t* srcRow = base;
             for (int y = 0; y < count; y++) {
                 png_read_rows(this->png_ptr(), &srcRow, nullptr, 1);
-                srcRow += fSrcRowBytes;
+                srcRow += srcRowBytes;
             }
             // read rows we don't want into garbage buffer
             for (int y = 0; y < fHeight - startRow - count; y++) {
                 png_read_rows(this->png_ptr(), &fGarbageRowPtr, nullptr, 1);
             }
         }
         //swizzle the rows we care about
-        srcRow = storagePtr;
+        srcRow = base;
         void* dstRow = dst;
         for (int y = 0; y < count; y++) {
             this->swizzler()->swizzle(dstRow, srcRow);
             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
-            srcRow += fSrcRowBytes;
+            srcRow += srcRowBytes;
         }
 
         return count;
     }
 
     bool onSkipScanlines(int count) override {
         // The non-virtual version will update fCurrScanline.
         return true;
     }
 
@@ skipping 33 matching lines @@
     }
 
     if (1 == numberPasses) {
         return new SkPngScanlineDecoder(imageInfo, streamDeleter.detach(), chunkReader,
                                         png_ptr, info_ptr, bitDepth);
     }
 
     return new SkPngInterlacedScanlineDecoder(imageInfo, streamDeleter.detach(), chunkReader,
                                               png_ptr, info_ptr, bitDepth, numberPasses);
 }