Fill incomplete images in SkCodec parent class

include/codec/SkCodec.h

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkCodec_DEFINED
 #define SkCodec_DEFINED
 
@@ skipping 207 matching lines @@
      *  of the encoded data, but then never use any colors which have alpha
      *  less than 100%. This function can be called *after* decoding to
      *  determine if such an image truly had alpha. Calling it before decoding
      *  is undefined.
      *  FIXME: see skbug.com/3582.
      */
     bool reallyHasAlpha() const {
         return this->onReallyHasAlpha();
     }
 
+    /**
+     * @return the value with which to fill uninitialized pixels.
+     *
+     * This is used when onGetScanlines() returns kIncompleteInput.
+     *
+     * Note that we can interpret the return value as an SkPMColor, a 16-bit 565 color,
+     * an 8-bit gray color, or an 8-bit index into a color table, depending on the color
+     * type specified in dstInfo.
+     */
+    uint32_t getFillValue(const SkImageInfo& dstInfo) const {

[scroggo, 2015/09/22 18:02:48]

I assume the implementation only needs color type and alpha type?

[msarett, 2015/09/23 13:22:39]

Yes, I will add a comment explaining this.

As an alternative to using dstInfo as a parameter, I could use a colorType and
and an alphaType.  I don't really have a preference, do you have an opinion on
which is better?

[scroggo, 2015/09/25 15:55:05]

I think only passing the information needed by the function is generally better.

[msarett, 2015/10/01 12:44:51]

Done.

+        return this->onGetFillValue(dstInfo);
+    }
+
+    /**
+     * @param dstStart         Pointer to the start of destination memory
+     * @param dstRowBytes      Stride length in destination memory
+     * @param decodedScanlines Number of scanlines decoded successfully
+     * @return                 Pointer to the destination memory that we need to fill
+     */
+    void* getFillDst(void* dstStart, size_t rowBytes, uint32_t decodedScanlines) const {

[scroggo, 2015/09/22 18:02:48]

Could you add a comment with an overview of what this function does?

Does this work for all scanline orderings?

[msarett, 2015/09/23 13:22:39]

Yes this needs clarification.

+        return this->onGetFillDst(dstStart, rowBytes, decodedScanlines);
+    }
+
 protected:
     SkCodec(const SkImageInfo&, SkStream*);
 
     virtual SkISize onGetScaledDimensions(float /* desiredScale */) const {
         // By default, scaling is not supported.
         return this->getInfo().dimensions();
     }
 
     virtual SkEncodedFormat onGetEncodedFormat() const = 0;
 
@@ skipping 24 matching lines @@
     /**
      *  Called by rewindIfNeeded, if the stream needed to be rewound.
      *
      *  Subclasses should do any set up needed after a rewind.
      */
     virtual bool onRewind() {
         return true;
     }
 
     /**
+     * Some subclasses will override this function, but this is a useful default for the color
+     * types that we support.  Note that for color types that do not use the full 32-bits,
+     * we will simply take the low bits of the fill value.
+     *
+     * kN32_SkColorType: Transparent or Black
+     * kRGB_565_SkColorType: Black
+     * kGray_8_SkColorType: Black
+     * kIndex_8_SkColorType: First color in color table
+     */
+    virtual uint32_t onGetFillValue(const SkImageInfo& dstInfo) const {
+        return kOpaque_SkAlphaType == dstInfo.alphaType() ? SK_ColorBLACK : SK_ColorTRANSPARENT;
+    }
+
+    /**
+     * The will only need to be overridden for images where the rows are not

[scroggo, 2015/09/22 18:02:48]

This*

[msarett, 2015/09/23 13:22:39]

Done.

+     * decoded in top down order.
+     */
+    virtual void* onGetFillDst(void* dstStart, size_t rowBytes, uint32_t decodedScanlines) const {
+        return SkTAddOffset<void>(dstStart, rowBytes * decodedScanlines);
+    }
+
+    /*
+     * Allows the subclass to indicate how many scanlines it failed to decode due to an
+     * incomplete input image.  This value tells us how many uninitialized scanlines to fill
+     * in destination memory.
+     */
+    void setIncompleteScanlines(uint32_t incompleteScanlines) {

[scroggo, 2015/09/22 18:02:48]

Maybe this should be returned by getPixels (in an out parameter)?

Oh, wait, this is only needed inside getPixels, right? What if we made it an out
parameter on onGetPixels. getPixels does not need to know about it.

[msarett, 2015/09/23 13:22:40]

Good idea!  I like this better than what I have.

+        fIncompleteScanlines = incompleteScanlines;
+    }
+
+    /**
      * Get method for the input stream
      */
     SkStream* stream() {
         return fStream.get();
     }
 
 private:
     const SkImageInfo       fInfo;
     SkAutoTDelete<SkStream> fStream;
     bool                    fNeedsRewind;
+    uint32_t                fIncompleteScanlines;
 };
 #endif // SkCodec_DEFINED