Pass the destination pointer to next

src/codec/SkSwizzler.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 SkSwizzler_DEFINED
 #define SkSwizzler_DEFINED
 
@@ skipping 100 matching lines @@
         SkASSERT(SkIsAlign8(BitsPerPixel(sc)));
         return BitsPerPixel(sc) >> 3;
     }
 
     /**
      *  Create a new SkSwizzler.
      *  @param SrcConfig Description of the format of the source.
      *  @param SkImageInfo dimensions() describe both the src and the dst.
      *              Other fields describe the dst.
      *  @param dst Destination to write pixels. Must match info and dstRowBytes
      *  @param dstRowBytes rowBytes for dst.

[scroggo, 2015/07/27 20:42:06]

Please remove the parameter descriptions for parameters that are no longer
there.

[msarett, 2015/07/27 21:11:34]

Done.

      *  @param ZeroInitialized Whether dst is zero-initialized. The
                                implementation may choose to skip writing zeroes
      *                         if set to kYes_ZeroInitialized.
      *  @return A new SkSwizzler or NULL on failure.
      */
     static SkSwizzler* CreateSwizzler(SrcConfig, const SkPMColor* ctable,
-                                      const SkImageInfo&, void* dst,
-                                      size_t dstRowBytes,
+                                      const SkImageInfo&,
                                       SkCodec::ZeroInitialized);
 
     /**
      * Fill the remainder of the destination with a single color
      *
      * @param dstStartRow
      * The destination row to fill from.
      *
      * @param numRows
      * The number of rows to fill.
@@ skipping 17 matching lines @@
      *
      * If dstInfo.colorType() is kGray, colorOrIndex is always treated as an 8-bit color.
      *
      * Other SkColorTypes are not supported.
      *
      */
     static void Fill(void* dstStartRow, const SkImageInfo& dstInfo, size_t dstRowBytes,
             uint32_t numRows, uint32_t colorOrIndex, const SkPMColor* colorTable);
 
     /**
-     *  Swizzle the next line. Call height times, once for each row of source.
+     *  Swizzle the next line. Generally this will be called height times, once
+     *  for each row of source.
+     *  By allowing the caller to pass in the dst pointer, we give the caller
+     *  flexibility to use the swizzler even when the encoded data does not
+     *  store the rows in order.  This also improves usability for scaled and
+     *  subset decodes.
      *  @param src The next row of the source data.
+     *  @param dst The intended destination row for the output.

[scroggo, 2015/07/27 20:42:06]

Maybe this should be "where to write the output", or something like that?

[msarett, 2015/07/27 21:11:34]

Yeah that is clearer.

      *  @return A result code describing if the row was fully opaque, fully
      *          transparent, or neither
      */
-    ResultAlpha next(const uint8_t* SK_RESTRICT src);
-
-    /**
-     *
-     * Alternate version of next that allows the caller to specify the row.
-     * It is very important to only use one version of next.  Since the other
-     * version modifies the dst pointer, it will change the behavior of this
-     * function.  We will check this in Debug mode.
-     *
-     */
-    ResultAlpha next(const uint8_t* SK_RESTRICT src, int y);
-
-    /**
-     *  Update the destination row.
-     *
-     *  Typically this is done by next, but for a client that wants to manually
-     *  modify the destination row (for example, for decoding scanline one at a
-     *  time) they can call this before each call to next.
-     *  TODO: Maybe replace this with a version of next which allows supplying the
-     *  destination?
-     */
-    void setDstRow(void* dst) { fDstRow = dst; }
-
-    /**
-     *  Get the next destination row to decode to
-     */
-    void* getDstRow() {
-        // kDesignateRow_NextMode does not update the fDstRow ptr.  This function is
-        // unnecessary in that case since fDstRow will always be equal to the pointer
-        // passed to CreateSwizzler().
-        SkASSERT(kDesignateRow_NextMode != fNextMode);
-        return fDstRow;
-    }
+    ResultAlpha next(const uint8_t* SK_RESTRICT src, void* dst);

[scroggo, 2015/07/27 20:42:06]

I believe this function was named "next" because it swizzled the next line. Now
it swizzles whatever line you tell it to, into wherever you tell it to. Maybe it
should be renamed to "swizzle" (same for SkMaskSwizzler)

[msarett, 2015/07/27 21:11:34]

Agreed!

 
 private:
 
-#ifdef SK_DEBUG
-    /*
-     *
-     * Keep track of which version of next the caller is using
-     *
-     */
-    enum NextMode {
-        kUninitialized_NextMode,
-        kConsecutive_NextMode,
-        kDesignateRow_NextMode,
-    };
-
-    NextMode fNextMode;
-#endif
-
     /**
      *  Method for converting raw data to Skia pixels.
      *  @param dstRow Row in which to write the resulting pixels.
      *  @param src Row of src data, in format specified by SrcConfig
      *  @param width Width in pixels
      *  @param deltaSrc if bitsPerPixel % 8 == 0, deltaSrc is bytesPerPixel
      *                  else, deltaSrc is bitsPerPixel
      *  @param y Line of source.

[scroggo, 2015/07/27 20:42:06]

Please remove this line.

(y was never actually used in any way, which should reassure us that this is a
good change.)

[msarett, 2015/07/27 21:11:34]

Done.

      *  @param ctable Colors (used for kIndex source).
      */
     typedef ResultAlpha (*RowProc)(void* SK_RESTRICT dstRow,
                                    const uint8_t* SK_RESTRICT src,
-                                   int width, int deltaSrc, int y,
+                                   int width, int deltaSrc,
                                    const SkPMColor ctable[]);
 
     const RowProc       fRowProc;
     const SkPMColor*    fColorTable;      // Unowned pointer
     const int           fDeltaSrc;        // if bitsPerPixel % 8 == 0
                                           //     deltaSrc is bytesPerPixel
                                           // else
                                           //     deltaSrc is bitsPerPixel
     const SkImageInfo   fDstInfo;
-    void*               fDstRow;
-    const size_t        fDstRowBytes;
     int                 fCurrY;
 
     SkSwizzler(RowProc proc, const SkPMColor* ctable, int deltaSrc,
-               const SkImageInfo& info, void* dst, size_t rowBytes);
+               const SkImageInfo& info);
 
 };
 #endif // SkSwizzler_DEFINED