Simplify rowsDecoded returned by incrementalDecode

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 "SkBitmap.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
@@ skipping 497 matching lines @@
             break;
     }
 }
 
 class SkPngNormalDecoder : public SkPngCodec {
 public:
     SkPngNormalDecoder(const SkEncodedInfo& info, const SkImageInfo& imageInfo, SkStream* stream,
             SkPngChunkReader* reader, png_structp png_ptr, png_infop info_ptr, int bitDepth)
         : INHERITED(info, imageInfo, stream, reader, png_ptr, info_ptr, bitDepth)
         , fLinesDecoded(0)
+        , fRowsWrittenToOutput(0)
         , fDst(nullptr)
         , fRowBytes(0)
         , fFirstRow(0)
         , fLastRow(0)
     {}
 
     static void AllRowsCallback(png_structp png_ptr, png_bytep row, png_uint_32 rowNum, int /*pass*/) {
         GetDecoder(png_ptr)->allRowsCallback(row, rowNum);
     }
 
     static void RowCallback(png_structp png_ptr, png_bytep row, png_uint_32 rowNum, int /*pass*/) {
         GetDecoder(png_ptr)->rowCallback(row, rowNum);
     }
 
 #ifdef SK_GOOGLE3_PNG_HACK
     static void RereadInfoCallback(png_structp png_ptr, png_infop) {
         GetDecoder(png_ptr)->rereadInfoCallback();
     }
 #endif
 
 private:
-    int                         fLinesDecoded; // FIXME: Move to baseclass?
+    // This represents the number of lines reported by libpng, minus any we skipped at the
+    // beginning. Only used when we are skipping lines (i.e. not in decodeAllRows).
+    int                         fLinesDecoded;
+    // While fLinesDecoded include lines that we skipped, this only includes lines written to the
+    // output so we can report it to the caller for filling.
+    // FIXME: Can we remove fLinesDecoded and just rely on fRowsWrittenToOutput?

[msarett, 2016/10/10 17:19:58]

I think this would be good.  IMO using this to count just feels more logical.

Not sure how hard that is implementation-wise though.

[scroggo_chromium, 2016/10/10 18:15:04]

Agreed. I don't think it's terribly hard, but it's not super straightforward,
either. My plan is to land this first and then follow-up with a change to
eliminate fLinesDecoded. It may mean reinterpreting what SkSampler::rowNeeded
expects, and I'd like to keep all of that separate from this change.

+    int                         fRowsWrittenToOutput;
     void*                       fDst;
     size_t                      fRowBytes;
 
     // Variables for partial decode
     int                         fFirstRow;  // FIXME: Move to baseclass?
     int                         fLastRow;
 
     typedef SkPngCodec INHERITED;
 
     static SkPngNormalDecoder* GetDecoder(png_structp png_ptr) {
         return static_cast<SkPngNormalDecoder*>(png_get_progressive_ptr(png_ptr));
     }
 
     Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
         const int height = this->getInfo().height();
         png_progressive_info_ptr callback = nullptr;
 #ifdef SK_GOOGLE3_PNG_HACK
         callback = RereadInfoCallback;
 #endif
         png_set_progressive_read_fn(this->png_ptr(), this, callback, AllRowsCallback, nullptr);
         fDst = dst;
         fRowBytes = rowBytes;
 
-        fLinesDecoded = 0;
+        fRowsWrittenToOutput = 0;
 
         this->processData();
 
-        if (fLinesDecoded == height) {
+        if (fRowsWrittenToOutput == height) {
             return SkCodec::kSuccess;
         }
 
         if (rowsDecoded) {
-            *rowsDecoded = fLinesDecoded;
+            *rowsDecoded = fRowsWrittenToOutput;
         }
 
         return SkCodec::kIncompleteInput;
     }
 
     void allRowsCallback(png_bytep row, int rowNum) {
         SkASSERT(rowNum == fLinesDecoded);
         fLinesDecoded++;
         this->applyXformRow(fDst, row);
         fDst = SkTAddOffset<void>(fDst, fRowBytes);
     }
 
     void setRange(int firstRow, int lastRow, void* dst, size_t rowBytes) override {
         png_progressive_info_ptr callback = nullptr;
 #ifdef SK_GOOGLE3_PNG_HACK
         callback = RereadInfoCallback;
 #endif
         png_set_progressive_read_fn(this->png_ptr(), this, callback, RowCallback, nullptr);
         fFirstRow = firstRow;
         fLastRow = lastRow;
         fDst = dst;
         fRowBytes = rowBytes;
         fLinesDecoded = 0;
+        fRowsWrittenToOutput = 0;
     }
 
     SkCodec::Result decode(int* rowsDecoded) override {
         this->processData();
 
         if (fLinesDecoded == fLastRow - fFirstRow + 1) {
             return SkCodec::kSuccess;
         }
 
         if (rowsDecoded) {
-            *rowsDecoded = fLinesDecoded;
+            *rowsDecoded = fRowsWrittenToOutput;
         }
 
         return SkCodec::kIncompleteInput;
     }
 
     void rowCallback(png_bytep row, int rowNum) {
         if (rowNum < fFirstRow) {
             // Ignore this row.
             return;
         }
 
         SkASSERT(rowNum <= fLastRow);
 
         // If there is no swizzler, all rows are needed.
         if (!this->swizzler() || this->swizzler()->rowNeeded(fLinesDecoded)) {
             this->applyXformRow(fDst, row);
             fDst = SkTAddOffset<void>(fDst, fRowBytes);
+            fRowsWrittenToOutput++;
         }
 
         fLinesDecoded++;
 
         if (rowNum == fLastRow) {
             // Fake error to stop decoding scanlines.
             longjmp(PNG_JMPBUF(this->png_ptr()), kStopDecoding);
         }
     }
 };
@@ skipping 129 matching lines @@
         // Now apply Xforms on all the rows that were decoded.
         if (!fLinesDecoded) {
             if (rowsDecoded) {
                 *rowsDecoded = 0;
             }
             return SkCodec::kIncompleteInput;
         }
         const int lastRow = fLinesDecoded + fFirstRow - 1;
         SkASSERT(lastRow <= fLastRow);
 
+        int rowsWrittenToOutput = 0;
+
         // FIXME: For resuming interlace, we may swizzle a row that hasn't changed. But it
         // may be too tricky/expensive to handle that correctly.
         png_bytep srcRow = fInterlaceBuffer.get();
         const int sampleY = this->swizzler() ? this->swizzler()->sampleY() : 1;
         void* dst = fDst;
         for (int rowNum = fFirstRow; rowNum <= lastRow; rowNum += sampleY) {
             this->applyXformRow(dst, srcRow);
             dst = SkTAddOffset<void>(dst, fRowBytes);
             srcRow = SkTAddOffset<png_byte>(srcRow, fPng_rowbytes * sampleY);
+            rowsWrittenToOutput++;
         }
 
         if (fInterlacedComplete) {
             return SkCodec::kSuccess;
         }
 
         if (rowsDecoded) {
-            *rowsDecoded = fLinesDecoded;
+            *rowsDecoded = rowsWrittenToOutput;
         }
         return SkCodec::kIncompleteInput;
     }
 
     void setUpInterlaceBuffer(int height) {
         fPng_rowbytes = png_get_rowbytes(this->png_ptr(), this->info_ptr());
         fInterlaceBuffer.reset(fPng_rowbytes * height);
         fInterlacedComplete = false;
     }
 };
@@ skipping 502 matching lines @@
     SkCodec* outCodec = nullptr;
     if (read_header(streamDeleter.get(), chunkReader, &outCodec, nullptr, nullptr)) {
         // Codec has taken ownership of the stream.
         SkASSERT(outCodec);
         streamDeleter.release();
         return outCodec;
     }
 
     return nullptr;
 }