SkBitmapRegionCodec needs to use the rowBytes on the pixel ref

tools/SkBitmapRegionCodec.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 "SkBitmapRegionCodec.h"
 #include "SkAndroidCodec.h"
 #include "SkCodecPriv.h"
 #include "SkCodecTools.h"
+#include "SkPixelRef.h"
 
 SkBitmapRegionCodec::SkBitmapRegionCodec(SkAndroidCodec* codec)
     : INHERITED(codec->getInfo().width(), codec->getInfo().height())
     , fCodec(codec)
 {}
 
 bool SkBitmapRegionCodec::decodeRegion(SkBitmap* bitmap, SkBitmap::Allocator* allocator,
         const SkIRect& desiredSubset, int sampleSize, SkColorType dstColorType,
         bool requireUnpremul) {
 
@@ skipping 64 matching lines @@
         scaledOutY = outY / sampleSize;
         // We need to be safe here because getSupportedSubset() may have modified the subset.
         const int extraX = SkTMax(0, desiredSubset.width() - outX - subset.width());
         const int extraY = SkTMax(0, desiredSubset.height() - outY - subset.height());
         const int scaledExtraX = extraX / sampleSize;
         const int scaledExtraY = extraY / sampleSize;
         scaledOutWidth += scaledOutX + scaledExtraX;
         scaledOutHeight += scaledOutY + scaledExtraY;
     }
     SkImageInfo outInfo = decodeInfo.makeWH(scaledOutWidth, scaledOutHeight);
-    bitmap->setInfo(outInfo, outInfo.minRowBytes());
+    bitmap->setInfo(outInfo);
     if (!bitmap->tryAllocPixels(allocator, colorTable.get())) {
         SkCodecPrintf("Error: Could not allocate pixels.\n");
         return false;
     }
 
     // Zero the bitmap if the region is not completely within the image.
     // TODO (msarett): Can we make this faster by implementing it to only
     //                 zero parts of the image that we won't overwrite with
     //                 pixels?
     // TODO (msarett): This could be skipped if memory is zero initialized.
     //                 This would matter if this code is moved to Android and
     //                 uses Android bitmaps.
     if (SubsetType::kPartiallyInside_SubsetType == type) {
         void* pixels = bitmap->getPixels();
         size_t bytes = outInfo.getSafeSize(bitmap->rowBytes());
         memset(pixels, 0, bytes);
     }
 
     // Decode into the destination bitmap
     SkAndroidCodec::AndroidOptions options;
     options.fSampleSize = sampleSize;
     options.fSubset = ⊂
     options.fColorPtr = colorPtr;
     options.fColorCount = colorCountPtr;
     void* dst = bitmap->getAddr(scaledOutX, scaledOutY);
-    size_t rowBytes = bitmap->rowBytes();
+
+    // FIXME: skbug.com/4538
+    // It is important that we use the rowBytes on the pixelRef.  They may not be
+    // set properly on the bitmap.
+    SkPixelRef* pr = SkRef(bitmap->pixelRef());

[scroggo, 2015/11/05 22:45:56]

I just want to double check that the ref counting is correct here...

Say pixelRef has a refCount of RC. After this call, it is RC + 1

+    size_t rowBytes = pr->rowBytes();
+    bitmap->setInfo(outInfo, rowBytes);

[scroggo, 2015/11/05 22:45:56]

Behind our backs, this decrements the refCount, so it is back to RC

+    bitmap->setPixelRef(pr)->unref();

[scroggo, 2015/11/05 22:45:56]

setPixelRef increments it again (RC + 1) and then we immediately unref it (RC).

lgtm

[msarett, 2015/11/05 22:49:05]

Looks correct to me as well.

     SkCodec::Result result = fCodec->getAndroidPixels(decodeInfo, dst, rowBytes, &options);
     if (SkCodec::kSuccess != result && SkCodec::kIncompleteInput != result) {
         SkCodecPrintf("Error: Could not get pixels.\n");
         return false;
     }
 
     return true;
 }
 
 bool SkBitmapRegionCodec::conversionSupported(SkColorType colorType) {
     // FIXME: Call virtual function when it lands.
     SkImageInfo info = SkImageInfo::Make(0, 0, colorType, fCodec->getInfo().alphaType(),
             fCodec->getInfo().profileType());
     return conversion_possible(info, fCodec->getInfo());
 }