simplify bitmap scaler and cache

BUG=skia:

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

6 files changed, 45 insertions, 148 deletions

diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp
index 2b11b81fd7..9f985e5a3e 100644
--- a/src/core/SkBitmapCache.cpp
+++ b/src/core/SkBitmapCache.cpp
@@ -48,26 +48,26 @@ static unsigned gBitmapKeyNamespaceLabel;
 
 struct BitmapKey : public SkResourceCache::Key {
 public:
-    BitmapKey(uint32_t genID, SkScalar sx, SkScalar sy, const SkIRect& bounds)
+    BitmapKey(uint32_t genID, int width, int height, const SkIRect& bounds)
         : fGenID(genID)
-        , fScaleX(sx)
-        , fScaleY(sy)
+        , fWidth(width)
+        , fHeight(height)
         , fBounds(bounds)
     {
         this->init(&gBitmapKeyNamespaceLabel, SkMakeResourceCacheSharedIDForBitmap(genID),
-                   sizeof(fGenID) + sizeof(fScaleX) + sizeof(fScaleY) + sizeof(fBounds));
+                   sizeof(fGenID) + sizeof(fWidth) + sizeof(fHeight) + sizeof(fBounds));
     }
 
-    uint32_t    fGenID;
-    SkScalar    fScaleX;
-    SkScalar    fScaleY;
-    SkIRect     fBounds;
+    const uint32_t  fGenID;
+    const int       fWidth;
+    const int       fHeight;
+    const SkIRect   fBounds;
 };
 
 struct BitmapRec : public SkResourceCache::Rec {
-    BitmapRec(uint32_t genID, SkScalar scaleX, SkScalar scaleY, const SkIRect& bounds,
+    BitmapRec(uint32_t genID, int width, int height, const SkIRect& bounds,
               const SkBitmap& result)
-        : fKey(genID, scaleX, scaleY, bounds)
+        : fKey(genID, width, height, bounds)
         , fBitmap(result)
     {}
 
@@ -97,25 +97,25 @@ private:
 #define CHECK_LOCAL(localCache, localName, globalName, ...) \
     ((localCache) ? localCache->localName(__VA_ARGS__) : SkResourceCache::globalName(__VA_ARGS__))
 
-bool SkBitmapCache::Find(const SkBitmap& src, SkScalar invScaleX, SkScalar invScaleY, SkBitmap* result,
-                         SkResourceCache* localCache) {
-    if (0 == invScaleX || 0 == invScaleY) {
+bool SkBitmapCache::FindWH(const SkBitmap& src, int width, int height, SkBitmap* result,
+                           SkResourceCache* localCache) {
+    if (0 == width || 0 == height) {
         // degenerate, and the key we use for mipmaps
         return false;
     }
-    BitmapKey key(src.getGenerationID(), invScaleX, invScaleY, get_bounds_from_bitmap(src));
+    BitmapKey key(src.getGenerationID(), width, height, get_bounds_from_bitmap(src));
 
     return CHECK_LOCAL(localCache, find, Find, key, BitmapRec::Finder, result);
 }
 
-void SkBitmapCache::Add(const SkBitmap& src, SkScalar invScaleX, SkScalar invScaleY,
-                        const SkBitmap& result, SkResourceCache* localCache) {
-    if (0 == invScaleX || 0 == invScaleY) {
+void SkBitmapCache::AddWH(const SkBitmap& src, int width, int height,
+                          const SkBitmap& result, SkResourceCache* localCache) {
+    if (0 == width || 0 == height) {
         // degenerate, and the key we use for mipmaps
         return;
     }
     SkASSERT(result.isImmutable());
-    BitmapRec* rec = new BitmapRec(src.getGenerationID(), invScaleX, invScaleY,
+    BitmapRec* rec = new BitmapRec(src.getGenerationID(), width, height,
                                    get_bounds_from_bitmap(src), result);
     CHECK_LOCAL(localCache, add, Add, rec);
     src.pixelRef()->notifyAddedToCache();
diff --git a/src/core/SkBitmapCache.h b/src/core/SkBitmapCache.h
index 3e541eed2c..12be6fad35 100644
--- a/src/core/SkBitmapCache.h
+++ b/src/core/SkBitmapCache.h
@@ -27,17 +27,17 @@ public:
     static SkBitmap::Allocator* GetAllocator();
 
     /**
-     *  Search based on the src bitmap and inverse scales in X and Y. If found, returns true and
+     *  Search based on the src bitmap and scaled width/height. If found, returns true and
      *  result will be set to the matching bitmap with its pixels already locked.
      */
-    static bool Find(const SkBitmap& src, SkScalar invScaleX, SkScalar invScaleY, SkBitmap* result,
-                     SkResourceCache* localCache = nullptr);
+    static bool FindWH(const SkBitmap& src, int width, int height, SkBitmap* result,
+                       SkResourceCache* localCache = nullptr);
 
     /*
      *  result must be marked isImmutable()
      */
-    static void Add(const SkBitmap& src, SkScalar invScaleX, SkScalar invScaleY,
-            const SkBitmap& result, SkResourceCache* localCache = nullptr);
+    static void AddWH(const SkBitmap& src, int width, int height, const SkBitmap& result,
+                      SkResourceCache* localCache = nullptr);
 
     /**
      *  Search based on the bitmap's genID and subset. If found, returns true and
diff --git a/src/core/SkBitmapController.cpp b/src/core/SkBitmapController.cpp
index e4b4199c69..3c3e69413a 100644
--- a/src/core/SkBitmapController.cpp
+++ b/src/core/SkBitmapController.cpp
@@ -10,6 +10,9 @@
 #include "SkMatrix.h"
 #include "SkTemplates.h"
 
+// RESIZE_LANCZOS3 is another good option, but chrome prefers mitchell at the moment
+#define kHQ_RESIZE_METHOD   SkBitmapScaler::RESIZE_MITCHELL
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 static bool valid_for_drawing(const SkBitmap& bm) {
@@ -117,31 +120,28 @@ bool SkDefaultBitmapControllerState::processHQRequest(const SkBitmap& origBitmap
         return false; // no need for HQ
     }
     
-    SkScalar trueDestWidth  = origBitmap.width() / invScaleX;
-    SkScalar trueDestHeight = origBitmap.height() / invScaleY;
-    SkScalar roundedDestWidth = SkScalarRoundToScalar(trueDestWidth);
-    SkScalar roundedDestHeight = SkScalarRoundToScalar(trueDestHeight);
+    const int dstW = SkScalarRoundToScalar(origBitmap.width() / invScaleX);
+    const int dstH = SkScalarRoundToScalar(origBitmap.height() / invScaleY);
     
-    if (!SkBitmapCache::Find(origBitmap, roundedDestWidth, roundedDestHeight, &fResultBitmap)) {
+    if (!SkBitmapCache::FindWH(origBitmap, dstW, dstH, &fResultBitmap)) {
         SkAutoPixmapUnlock src;
         if (!origBitmap.requestLock(&src)) {
             return false;
         }
-        if (!SkBitmapScaler::Resize(&fResultBitmap, src.pixmap(), SkBitmapScaler::RESIZE_BEST,
-                                    roundedDestWidth, roundedDestHeight,
-                                    SkResourceCache::GetAllocator())) {
+        if (!SkBitmapScaler::Resize(&fResultBitmap, src.pixmap(), kHQ_RESIZE_METHOD,
+                                    dstW, dstH, SkResourceCache::GetAllocator())) {
             return false; // we failed to create fScaledBitmap
         }
         
         SkASSERT(fResultBitmap.getPixels());
         fResultBitmap.setImmutable();
-        SkBitmapCache::Add(origBitmap, roundedDestWidth, roundedDestHeight, fResultBitmap);
+        SkBitmapCache::AddWH(origBitmap, dstW, dstH, fResultBitmap);
     }
     
     SkASSERT(fResultBitmap.getPixels());
     
-    fInvMatrix.postScale(roundedDestWidth / origBitmap.width(),
-                         roundedDestHeight / origBitmap.height());
+    fInvMatrix.postScale(SkIntToScalar(dstW) / origBitmap.width(),
+                         SkIntToScalar(dstH) / origBitmap.height());
     fQuality = kLow_SkFilterQuality;
     return true;
 }
diff --git a/src/core/SkBitmapScaler.cpp b/src/core/SkBitmapScaler.cpp
index ff053a9a41..962fdce375 100644
--- a/src/core/SkBitmapScaler.cpp
+++ b/src/core/SkBitmapScaler.cpp
@@ -62,10 +62,10 @@ SkResizeFilter::SkResizeFilter(SkBitmapScaler::ResizeMethod method,
                                const SkRect& destSubset,
                                const SkConvolutionProcs& convolveProcs) {
 
-    // method will only ever refer to an "algorithm method".
-    SkASSERT((SkBitmapScaler::RESIZE_FIRST_ALGORITHM_METHOD <= method) &&
-             (method <= SkBitmapScaler::RESIZE_LAST_ALGORITHM_METHOD));
+    SkASSERT(method >= SkBitmapScaler::RESIZE_FirstMethod &&
+             method <= SkBitmapScaler::RESIZE_LastMethod);
 
+    fBitmapFilter = nullptr;
     switch(method) {
         case SkBitmapScaler::RESIZE_BOX:
             fBitmapFilter = new SkBoxFilter;
@@ -82,10 +82,6 @@ SkResizeFilter::SkResizeFilter(SkBitmapScaler::ResizeMethod method,
         case SkBitmapScaler::RESIZE_LANCZOS3:
             fBitmapFilter = new SkLanczosFilter;
             break;
-        default:
-            // NOTREACHED:
-            fBitmapFilter = new SkMitchellFilter(1.f / 3.f, 1.f / 3.f);
-            break;
     }
 
 
@@ -214,43 +210,8 @@ void SkResizeFilter::computeFilters(int srcSize,
   }
 }
 
-static SkBitmapScaler::ResizeMethod ResizeMethodToAlgorithmMethod(
-                                    SkBitmapScaler::ResizeMethod method) {
-    // Convert any "Quality Method" into an "Algorithm Method"
-    if (method >= SkBitmapScaler::RESIZE_FIRST_ALGORITHM_METHOD &&
-    method <= SkBitmapScaler::RESIZE_LAST_ALGORITHM_METHOD) {
-        return method;
-    }
-    // The call to SkBitmapScalerGtv::Resize() above took care of
-    // GPU-acceleration in the cases where it is possible. So now we just
-    // pick the appropriate software method for each resize quality.
-    switch (method) {
-        // Users of RESIZE_GOOD are willing to trade a lot of quality to
-        // get speed, allowing the use of linear resampling to get hardware
-        // acceleration (SRB). Hence any of our "good" software filters
-        // will be acceptable, so we use a triangle.
-        case SkBitmapScaler::RESIZE_GOOD:
-            return SkBitmapScaler::RESIZE_TRIANGLE;
-        // Users of RESIZE_BETTER are willing to trade some quality in order
-        // to improve performance, but are guaranteed not to devolve to a linear
-        // resampling. In visual tests we see that Hamming-1 is not as good as
-        // Lanczos-2, however it is about 40% faster and Lanczos-2 itself is
-        // about 30% faster than Lanczos-3. The use of Hamming-1 has been deemed
-        // an acceptable trade-off between quality and speed.
-        case SkBitmapScaler::RESIZE_BETTER:
-            return SkBitmapScaler::RESIZE_HAMMING;
-        default:
-#ifdef SK_HIGH_QUALITY_IS_LANCZOS
-            return SkBitmapScaler::RESIZE_LANCZOS3;
-#else
-            return SkBitmapScaler::RESIZE_MITCHELL;
-#endif
-    }
-}
-
 bool SkBitmapScaler::Resize(SkBitmap* resultPtr, const SkPixmap& source, ResizeMethod method,
-                            float destWidth, float destHeight,
-                            SkBitmap::Allocator* allocator) {
+                            int destWidth, int destHeight, SkBitmap::Allocator* allocator) {
     if (nullptr == source.addr() || source.colorType() != kN32_SkColorType ||
         source.width() < 1 || source.height() < 1)
     {
@@ -258,27 +219,13 @@ bool SkBitmapScaler::Resize(SkBitmap* resultPtr, const SkPixmap& source, ResizeM
     }
 
     if (destWidth < 1 || destHeight < 1) {
-        // todo: seems like we could handle negative dstWidth/Height, since that
-        // is just a negative scale (flip)
         return false;
     }
 
     SkConvolutionProcs convolveProcs= { 0, nullptr, nullptr, nullptr, nullptr };
     PlatformConvolutionProcs(&convolveProcs);
 
-    SkRect destSubset = { 0, 0, destWidth, destHeight };
-
-    // Ensure that the ResizeMethod enumeration is sound.
-    SkASSERT(((RESIZE_FIRST_QUALITY_METHOD <= method) &&
-      (method <= RESIZE_LAST_QUALITY_METHOD)) ||
-      ((RESIZE_FIRST_ALGORITHM_METHOD <= method) &&
-      (method <= RESIZE_LAST_ALGORITHM_METHOD)));
-
-    method = ResizeMethodToAlgorithmMethod(method);
-
-    // Check that we deal with an "algorithm methods" from this point onward.
-    SkASSERT((SkBitmapScaler::RESIZE_FIRST_ALGORITHM_METHOD <= method) &&
-      (method <= SkBitmapScaler::RESIZE_LAST_ALGORITHM_METHOD));
+    SkRect destSubset = SkRect::MakeIWH(destWidth, destHeight);
 
     SkResizeFilter filter(method, source.width(), source.height(),
                         destWidth, destHeight, destSubset, convolveProcs);
diff --git a/src/core/SkBitmapScaler.h b/src/core/SkBitmapScaler.h
index 1b7eef5952..03feec3e12 100644
--- a/src/core/SkBitmapScaler.h
+++ b/src/core/SkBitmapScaler.h
@@ -19,68 +19,18 @@
 class SK_API SkBitmapScaler {
 public:
     enum ResizeMethod {
-        // Quality Methods
-        //
-        // Those enumeration values express a desired quality/speed tradeoff.
-        // They are translated into an algorithm-specific method that depends
-        // on the capabilities (CPU, GPU) of the underlying platform.
-        // It is possible for all three methods to be mapped to the same
-        // algorithm on a given platform.
-
-        // Good quality resizing. Fastest resizing with acceptable visual quality.
-        // This is typically intended for use during interactive layouts
-        // where slower platforms may want to trade image quality for large
-        // increase in resizing performance.
-        //
-        // For example the resizing implementation may devolve to linear
-        // filtering if this enables GPU acceleration to be used.
-        //
-        // Note that the underlying resizing method may be determined
-        // on the fly based on the parameters for a given resize call.
-        // For example an implementation using a GPU-based linear filter
-        // in the common case may still use a higher-quality software-based
-        // filter in cases where using the GPU would actually be slower - due
-        // to too much latency - or impossible - due to image format or size
-        // constraints.
-        RESIZE_GOOD,
-
-        // Medium quality resizing. Close to high quality resizing (better
-        // than linear interpolation) with potentially some quality being
-        // traded-off for additional speed compared to RESIZE_BEST.
-        //
-        // This is intended, for example, for generation of large thumbnails
-        // (hundreds of pixels in each dimension) from large sources, where
-        // a linear filter would produce too many artifacts but where
-        // a RESIZE_HIGH might be too costly time-wise.
-        RESIZE_BETTER,
-
-        // High quality resizing. The algorithm is picked to favor image quality.
-        RESIZE_BEST,
-
-        //
-        // Algorithm-specific enumerations
-        //
-
-        // Box filter. This is a weighted average of all of the pixels touching
-        // the destination pixel. For enlargement, this is nearest neighbor.
-        //
-        // You probably don't want this, it is here for testing since it is easy to
-        // compute. Use RESIZE_LANCZOS3 instead.
         RESIZE_BOX,
         RESIZE_TRIANGLE,
         RESIZE_LANCZOS3,
         RESIZE_HAMMING,
         RESIZE_MITCHELL,
 
-        // enum aliases for first and last methods by algorithm or by quality.
-        RESIZE_FIRST_QUALITY_METHOD = RESIZE_GOOD,
-        RESIZE_LAST_QUALITY_METHOD = RESIZE_BEST,
-        RESIZE_FIRST_ALGORITHM_METHOD = RESIZE_BOX,
-        RESIZE_LAST_ALGORITHM_METHOD = RESIZE_MITCHELL,
+        RESIZE_FirstMethod = RESIZE_BOX,
+        RESIZE_LastMethod = RESIZE_MITCHELL,
     };
 
     static bool Resize(SkBitmap* result, const SkPixmap& src, ResizeMethod method,
-                       float dest_width, float dest_height, SkBitmap::Allocator* = nullptr);
+                       int dest_width, int dest_height, SkBitmap::Allocator* = nullptr);
 
      /** Platforms can also optionally overwrite the convolution functions
         if we have SIMD versions of them.
diff --git a/tests/SkResourceCacheTest.cpp b/tests/SkResourceCacheTest.cpp
index 6314863e50..cf8327fcde 100644
--- a/tests/SkResourceCacheTest.cpp
+++ b/tests/SkResourceCacheTest.cpp
@@ -21,9 +21,9 @@ static bool is_in_scaled_image_cache(const SkBitmap& orig,
                                      SkScalar xScale,
                                      SkScalar yScale) {
     SkBitmap scaled;
-    float roundedImageWidth = SkScalarRoundToScalar(orig.width() * xScale);
-    float roundedImageHeight = SkScalarRoundToScalar(orig.height() * yScale);
-    return SkBitmapCache::Find(orig, roundedImageWidth, roundedImageHeight, &scaled);
+    int width = SkScalarRoundToInt(orig.width() * xScale);
+    int height = SkScalarRoundToInt(orig.height() * yScale);
+    return SkBitmapCache::FindWH(orig, width, height, &scaled);
 }
 
 // Draw a scaled bitmap, then return true if it has been cached.