Revert "Revert of https://codereview.chromium.org/113823003/"

This reverts commit 68b4b32066ea0ba9dbb5d326a836f8a54297b7aa.

BUG=

Review URL: https://codereview.chromium.org/122293002

git-svn-id: http://skia.googlecode.com/svn/trunk@12842 2bbb7eff-a529-9590-31e7-b0007b416f81

include/core/Sk64.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2006 The Android Open Source Project
  *
@@ -6,7 +5,6 @@
  * found in the LICENSE file.
  */
 
-
 #ifndef Sk64_DEFINED
 #define Sk64_DEFINED
 
@@ -28,6 +26,11 @@ public:
     int64_t as64() const { return ((int64_t)fHi << 32) | fLo; }
     int64_t getLongLong() const { return this->as64(); }
 
+    void set64(int64_t value) {
+        fHi = (int32_t)(value >> 32);
+        fLo = (uint32_t)value;
+    }
+
     /** Returns non-zero if the Sk64 can be represented as a signed 32 bit integer
     */
     SkBool is32() const { return fHi == ((int32_t)fLo >> 31); }

include/core/SkBitmap.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2006 The Android Open Source Project
  *
@@ -6,17 +5,19 @@
  * found in the LICENSE file.
  */
 
-
 #ifndef SkBitmap_DEFINED
 #define SkBitmap_DEFINED
 
-#include "Sk64.h"
 #include "SkColor.h"
 #include "SkColorTable.h"
 #include "SkImageInfo.h"
 #include "SkPoint.h"
 #include "SkRefCnt.h"
 
+#ifdef SK_SUPPORT_LEGACY_SK64
+    #include "Sk64.h"
+#endif
+
 struct SkIRect;
 struct SkRect;
 class SkPaint;
@@ -149,19 +150,37 @@ public:
     */
     size_t getSafeSize() const ;
 
-    /** Return the byte size of the pixels, based on the height and rowBytes.
+    /**
-        This routine is slightly slower than getSize(), but does not truncate
+     *  Return the full size of the bitmap, in bytes.
-        the answer to 32bits.
+     */
-    */
+    int64_t computeSize64() const {
+        return sk_64_mul(fHeight, fRowBytes);
+    }
+
+    /**
+     *  Return the number of bytes from the pointer returned by getPixels()
+     *  to the end of the allocated space in the buffer. This may be smaller
+     *  than computeSize64() if there is any rowbytes padding beyond the width.
+     */
+    int64_t computeSafeSize64() const {
+        return ComputeSafeSize64((Config)fConfig, fWidth, fHeight, fRowBytes);
+    }
+
+#ifdef SK_SUPPORT_LEGACY_SK64
+    SK_ATTR_DEPRECATED("use getSize64()")
     Sk64 getSize64() const {
         Sk64 size;
-        size.setMul(fHeight, fRowBytes);
+        size.set64(this->computeSize64());
         return size;
     }
 
-    /** Same as getSafeSize(), but does not truncate the answer to 32bits.
+    SK_ATTR_DEPRECATED("use getSafeSize64()")
-    */
+    Sk64 getSafeSize64() const {
-    Sk64 getSafeSize64() const ;
+        Sk64 size;
+        size.set64(this->computeSafeSize64());
+        return size;
+    }
+#endif
 
     /** Returns true if this bitmap is marked as immutable, meaning that the
         contents of its pixels will not change for the lifetime of the bitmap.
@@ -217,7 +236,7 @@ public:
         return ComputeBytesPerPixel(c) >> 1;
     }
 
-    static Sk64 ComputeSize64(Config, int width, int height);
+    static int64_t ComputeSize64(Config, int width, int height);
     static size_t ComputeSize(Config, int width, int height);
 
     /**
@@ -678,10 +697,10 @@ private:
 
     /* Internal computations for safe size.
     */
-    static Sk64 ComputeSafeSize64(Config   config,
+    static int64_t ComputeSafeSize64(Config   config,
-                                  uint32_t width,
+                                     uint32_t width,
-                                  uint32_t height,
+                                     uint32_t height,
-                                  size_t   rowBytes);
+                                     size_t   rowBytes);
     static size_t ComputeSafeSize(Config   config,
                                   uint32_t width,
                                   uint32_t height,

include/core/SkMath.h

@@ -35,6 +35,32 @@ int32_t SkSqrtBits(int32_t value, int bitBias);
  */
 #define SkSqrt32(n)         SkSqrtBits(n, 15)
 
+// 64bit -> 32bit utilities
+
+/**
+ *  Return true iff the 64bit value can exactly be represented in signed 32bits
+ */
+static inline bool sk_64_isS32(int64_t value) {
+    return (int32_t)value == value;
+}
+
+/**
+ *  Return the 64bit argument as signed 32bits, asserting in debug that the arg
+ *  exactly fits in signed 32bits. In the release build, no checks are preformed
+ *  and the return value if the arg does not fit is undefined.
+ */
+static inline int32_t sk_64_asS32(int64_t value) {
+    SkASSERT(sk_64_isS32(value));
+    return (int32_t)value;
+}
+
+// Handy util that can be passed two ints, and will automatically promote to
+// 64bits before the multiply, so the caller doesn't have to remember to cast
+// e.g. (int64_t)a * b;
+static inline int64_t sk_64_mul(int64_t a, int64_t b) {
+    return a * b;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 //! Returns the number of leading zero bits (0...32)

include/core/SkTypes.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2006 The Android Open Source Project
  *
@@ -6,7 +5,6 @@
  * found in the LICENSE file.
  */
 
-
 #ifndef SkTypes_DEFINED
 #define SkTypes_DEFINED
 
@@ -15,6 +13,8 @@
 #include "SkPostConfig.h"
 #include <stdint.h>
 
+//#define SK_SUPPORT_LEGACY_SK64
+
 /** \file SkTypes.h
 */
 

include/utils/SkRandom.h

@@ -10,9 +10,12 @@
 #ifndef SkRandom_DEFINED
 #define SkRandom_DEFINED
 
-#include "Sk64.h"
 #include "SkScalar.h"
 
+#ifdef SK_SUPPORT_LEGACY_SK64
+    #include "Sk64.h"
+#endif
+
 /** \class SkLCGRandom
 
     Utility class that implements pseudo random 32bit numbers using a fast
@@ -123,13 +126,21 @@ public:
         return this->nextUScalar1() <= fractionTrue;
     }
 
-    /** Return the next pseudo random number as a signed 64bit value.
+    /**
-    */
+     *  Return the next pseudo random number as a signed 64bit value.
+     */
+    int64_t next64() {
+        int64_t hi = this->nextS();
+        return (hi << 32) | this->nextU();
+    }
+
+#ifdef SK_SUPPORT_LEGACY_SK64
+    SK_ATTR_DEPRECATED("use next64()")
     void next64(Sk64* a) {
         SkASSERT(a);
         a->set(this->nextS(), this->nextU());
     }
-
+#endif
     /**
      *  Return the current seed. This allows the caller to later reset to the
      *  same seed (using setSeed) so it can generate the same sequence.
@@ -276,12 +287,21 @@ public:
         return this->nextUScalar1() <= fractionTrue;
     }
 
-    /** Return the next pseudo random number as a signed 64bit value.
+    /**
+     *  Return the next pseudo random number as a signed 64bit value.
      */
+    int64_t next64() {
+        int64_t hi = this->nextS();
+        return (hi << 32) | this->nextU();
+    }
+    
+#ifdef SK_SUPPORT_LEGACY_SK64
+    SK_ATTR_DEPRECATED("use next64()")
     void next64(Sk64* a) {
         SkASSERT(a);
         a->set(this->nextS(), this->nextU());
     }
+#endif
 
     /** Reset the random object.
      */

src/core/SkBitmap.cpp

@@ -24,10 +24,6 @@
 #include "SkPackBits.h"
 #include <new>
 
-static bool isPos32Bits(const Sk64& value) {
-    return !value.isNeg() && value.is32();
-}
-
 struct MipLevel {
     void*       fPixels;
     uint32_t    fRowBytes;
@@ -44,14 +40,12 @@ struct SkBitmap::MipMap : SkNoncopyable {
         if (levelCount < 0) {
             return NULL;
         }
-        Sk64 size;
+        int64_t size = (levelCount + 1) * sizeof(MipLevel);
-        size.setMul(levelCount + 1, sizeof(MipLevel));
+        size += sizeof(MipMap) + pixelSize;
-        size.add(sizeof(MipMap));
+        if (!sk_64_isS32(size)) {
-        size.add(SkToS32(pixelSize));
-        if (!isPos32Bits(size)) {
             return NULL;
         }
-        MipMap* mm = (MipMap*)sk_malloc_throw(size.get32());
+        MipMap* mm = (MipMap*)sk_malloc_throw(sk_64_asS32(size));
         mm->fRefCnt = 1;
         mm->fLevelCount = levelCount;
         return mm;
@@ -185,58 +179,49 @@ size_t SkBitmap::ComputeRowBytes(Config c, int width) {
         return 0;
     }
 
-    Sk64 rowBytes;
+    int64_t rowBytes = 0;
-    rowBytes.setZero();
 
     switch (c) {
         case kNo_Config:
             break;
         case kA8_Config:
         case kIndex8_Config:
-            rowBytes.set(width);
+            rowBytes = width;
             break;
         case kRGB_565_Config:
         case kARGB_4444_Config:
-            rowBytes.set(width);
+            rowBytes = width << 1;
-            rowBytes.shiftLeft(1);
             break;
         case kARGB_8888_Config:
-            rowBytes.set(width);
+            rowBytes = width << 2;
-            rowBytes.shiftLeft(2);
             break;
         default:
             SkDEBUGFAIL("unknown config");
             break;
     }
-    return isPos32Bits(rowBytes) ? rowBytes.get32() : 0;
+    return sk_64_isS32(rowBytes) ? sk_64_asS32(rowBytes) : 0;
 }
 
-Sk64 SkBitmap::ComputeSize64(Config c, int width, int height) {
+int64_t SkBitmap::ComputeSize64(Config config, int width, int height) {
-    Sk64 size;
+    int64_t rowBytes = sk_64_mul(ComputeBytesPerPixel(config), width);
-    size.setMul(SkToS32(SkBitmap::ComputeRowBytes(c, width)), height);
+    return rowBytes * height;
-    return size;
 }
 
 size_t SkBitmap::ComputeSize(Config c, int width, int height) {
-    Sk64 size = SkBitmap::ComputeSize64(c, width, height);
+    int64_t size = SkBitmap::ComputeSize64(c, width, height);
-    return isPos32Bits(size) ? size.get32() : 0;
+    return sk_64_isS32(size) ? sk_64_asS32(size) : 0;
 }
 
-Sk64 SkBitmap::ComputeSafeSize64(Config config,
+int64_t SkBitmap::ComputeSafeSize64(Config config,
-                                 uint32_t width,
+                                    uint32_t width,
-                                 uint32_t height,
+                                    uint32_t height,
-                                 size_t rowBytes) {
+                                    size_t rowBytes) {
-    Sk64 safeSize;
+    int64_t safeSize = 0;
-    safeSize.setZero();
     if (height > 0) {
-        // TODO: Handle the case where the return value from
+        int64_t lastRow = sk_64_mul(ComputeBytesPerPixel(config), width);
-        // ComputeRowBytes is more than 31 bits.
+        safeSize = sk_64_mul(height - 1, rowBytes) + lastRow;
-        safeSize.set(SkToS32(ComputeRowBytes(config, width)));
-        Sk64 sizeAllButLastRow;
-        sizeAllButLastRow.setMul(height - 1, SkToS32(rowBytes));
-        safeSize.add(sizeAllButLastRow);
     }
-    SkASSERT(!safeSize.isNeg());
+    SkASSERT(safeSize >= 0);
     return safeSize;
 }
 
@@ -244,8 +229,13 @@ size_t SkBitmap::ComputeSafeSize(Config config,
                                  uint32_t width,
                                  uint32_t height,
                                  size_t rowBytes) {
-    Sk64 safeSize = ComputeSafeSize64(config, width, height, rowBytes);
+    int64_t safeSize = ComputeSafeSize64(config, width, height, rowBytes);
-    return (safeSize.is32() ? safeSize.get32() : 0);
+    int32_t safeSize32 = (int32_t)safeSize;
+    
+    if (safeSize32 != safeSize) {
+        safeSize32 = 0;
+    }
+    return safeSize32;
 }
 
 void SkBitmap::getBounds(SkRect* bounds) const {
@@ -558,10 +548,6 @@ size_t SkBitmap::getSafeSize() const {
             ComputeRowBytes(this->config(), fWidth): 0);
 }
 
-Sk64 SkBitmap::getSafeSize64() const {
-    return ComputeSafeSize64(this->config(), fWidth, fHeight, fRowBytes);
-}
-
 bool SkBitmap::copyPixelsTo(void* const dst, size_t dstSize,
                             size_t dstRowBytes, bool preserveDstPad) const {
 

src/core/SkMallocPixelRef.cpp

@@ -64,13 +64,12 @@ SkMallocPixelRef* SkMallocPixelRef::NewAllocate(const SkImageInfo& info,
         rowBytes = minRB;
     }
 
-    Sk64 bigSize;
+    int64_t bigSize = (int64_t)info.fHeight * rowBytes;
-    bigSize.setMul(info.fHeight, rowBytes);
+    if (!sk_64_isS32(bigSize)) {
-    if (!bigSize.is32()) {
         return NULL;
     }
 
-    size_t size = bigSize.get32();
+    size_t size = sk_64_asS32(bigSize);
     void* addr = sk_malloc_flags(size, 0);
     if (NULL == addr) {
         return NULL;

src/core/SkMipMap.cpp

@@ -109,21 +109,15 @@ static void downsampleby2_proc4444(SkBitmap* dst, int x, int y,
     *dst->getAddr16(x >> 1, y >> 1) = (uint16_t)collaps4444(c >> 2);
 }
 
-static bool isPos32Bits(const Sk64& value) {
-    return !value.isNeg() && value.is32();
-}
-
 SkMipMap::Level* SkMipMap::AllocLevels(int levelCount, size_t pixelSize) {
     if (levelCount < 0) {
         return NULL;
     }
-    Sk64 size;
+    int64_t size = sk_64_mul(levelCount + 1, sizeof(Level)) + pixelSize;
-    size.setMul(levelCount + 1, sizeof(Level));
+    if (!sk_64_isS32(size)) {
-    size.add(SkToS32(pixelSize));
-    if (!isPos32Bits(size)) {
         return NULL;
     }
-    return (Level*)sk_malloc_throw(size.get32());
+    return (Level*)sk_malloc_throw(sk_64_asS32(size));
 }
 
 SkMipMap* SkMipMap::Build(const SkBitmap& src) {

src/core/SkRegion_path.cpp

@@ -107,8 +107,6 @@ bool SkRgnBuilder::init(int maxHeight, int maxTransitions, bool pathIsInverse) {
         return false;
     }
 
-    Sk64 count, size;
-
     if (pathIsInverse) {
         // allow for additional X transitions to "invert" each scanline
         // [ L' ... normal transitions ... R' ]
@@ -117,25 +115,25 @@ bool SkRgnBuilder::init(int maxHeight, int maxTransitions, bool pathIsInverse) {
     }
 
     // compute the count with +1 and +3 slop for the working buffer
-    count.setMul(maxHeight + 1, 3 + maxTransitions);
+    int64_t count = sk_64_mul(maxHeight + 1, 3 + maxTransitions);
 
     if (pathIsInverse) {
         // allow for two "empty" rows for the top and bottom
         //      [ Y, 1, L, R, S] == 5 (*2 for top and bottom)
-        count.add(10);
+        count += 10;
     }
 
-    if (!count.is32() || count.isNeg()) {
+    if (count < 0 || !sk_64_isS32(count)) {
         return false;
     }
-    fStorageCount = count.get32();
+    fStorageCount = sk_64_asS32(count);
 
-    size.setMul(fStorageCount, sizeof(SkRegion::RunType));
+    int64_t size = sk_64_mul(fStorageCount, sizeof(SkRegion::RunType));
-    if (!size.is32() || size.isNeg()) {
+    if (size < 0 || !sk_64_isS32(size)) {
         return false;
     }
 
-    fStorage = (SkRegion::RunType*)sk_malloc_flags(size.get32(), 0);
+    fStorage = (SkRegion::RunType*)sk_malloc_flags(sk_64_asS32(size), 0);
     if (NULL == fStorage) {
         return false;
     }

src/images/SkImageDecoder_libpng.cpp

@@ -607,13 +607,9 @@ bool SkPNGImageDecoder::getBitmapConfig(png_structp png_ptr, png_infop info_ptr,
 
     // sanity check for size
     {
-        Sk64 size;
+        int64_t size = sk_64_mul(origWidth, origHeight);
-        size.setMul(origWidth, origHeight);
-        if (size.isNeg() || !size.is32()) {
-            return false;
-        }
         // now check that if we are 4-bytes per pixel, we also don't overflow
-        if (size.get32() > (0x7FFFFFFF >> 2)) {
+        if (size < 0 || size > (0x7FFFFFFF >> 2)) {
             return false;
         }
     }

src/images/SkImageDecoder_libwebp.cpp

@@ -80,13 +80,12 @@ static bool webp_parse_header(SkStream* stream, int* width, int* height, int* al
 
     // sanity check for image size that's about to be decoded.
     {
-        Sk64 size;
+        int64_t size = sk_64_mul(*width, *height);
-        size.setMul(*width, *height);
+        if (!sk_64_isS32(size)) {
-        if (size.isNeg() || !size.is32()) {
             return false;
         }
         // now check that if we are 4-bytes per pixel, we also don't overflow
-        if (size.get32() > (0x7FFFFFFF >> 2)) {
+        if (sk_64_asS32(size) > (0x7FFFFFFF >> 2)) {
             return false;
         }
     }

tests/BitmapCopyTest.cpp

@@ -343,8 +343,8 @@ DEF_TEST(BitmapCopy, reporter) {
             SkBitmap tstSafeSize;
             tstSafeSize.setConfig(gPairs[i].fConfig, 100000000U,
                                   100000000U);
-            Sk64 safeSize = tstSafeSize.getSafeSize64();
+            int64_t safeSize = tstSafeSize.computeSafeSize64();
-            if (safeSize.isNeg()) {
+            if (safeSize < 0) {
                 SkString str;
                 str.printf("getSafeSize64() negative: %s",
                     getSkConfigName(tstSafeSize));
@@ -358,20 +358,20 @@ DEF_TEST(BitmapCopy, reporter) {
 
                 case SkBitmap::kA8_Config:
                 case SkBitmap::kIndex8_Config:
-                    if (safeSize.as64() != 0x2386F26FC10000LL) {
+                    if (safeSize != 0x2386F26FC10000LL) {
                         sizeFail = true;
                     }
                     break;
 
                 case SkBitmap::kRGB_565_Config:
                 case SkBitmap::kARGB_4444_Config:
-                    if (safeSize.as64() != 0x470DE4DF820000LL) {
+                    if (safeSize != 0x470DE4DF820000LL) {
                         sizeFail = true;
                     }
                     break;
 
                 case SkBitmap::kARGB_8888_Config:
-                    if (safeSize.as64() != 0x8E1BC9BF040000LL) {
+                    if (safeSize != 0x8E1BC9BF040000LL) {
                         sizeFail = true;
                     }
                     break;
@@ -381,7 +381,7 @@ DEF_TEST(BitmapCopy, reporter) {
             }
             if (sizeFail) {
                 SkString str;
-                str.printf("getSafeSize64() wrong size: %s",
+                str.printf("computeSafeSize64() wrong size: %s",
                     getSkConfigName(tstSafeSize));
                 reporter->reportFailed(str);
             }

tests/Sk64Test.cpp

@@ -7,6 +7,7 @@
 
 #include "Test.h"
 #include "TestClassDef.h"
+#include "Sk64.h"
 #include "SkRandom.h"
 #include <math.h>
 
@@ -92,8 +93,8 @@ void Sk64::UnitTestWithReporter(void* reporterParam) {
     }
 
     for (i = 0; i < 1000; i++) {
-        rand.next64(&a); //a.fHi >>= 1; // avoid overflow
+        a.set64(rand.next64());
-        rand.next64(&b); //b.fHi >>= 1; // avoid overflow
+        b.set64(rand.next64());
 
         if (!(i & 3))   // want to explicitly test these cases
         {