Working plumb of image scaling:

src/core/SkBitmapProcState.cpp

Index: src/core/SkBitmapProcState.cpp
diff --git a/src/core/SkBitmapProcState.cpp b/src/core/SkBitmapProcState.cpp
index a2c7f990c2d9b19fe5ee238de9eae68757a62c3b..d769aa7db05d514971435fb6fa9b36a3316ec66e 100644
--- a/src/core/SkBitmapProcState.cpp
+++ b/src/core/SkBitmapProcState.cpp
@@ -90,6 +90,10 @@ static bool valid_for_filtering(unsigned dimension) {
     return (dimension & ~0x3FFF) == 0;
 }
 
+void SkBitmapProcState::endContext() {
+    SkDELETE(fBitmapFilter);
+}
+
 bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) {
     if (fOrigBitmap.width() == 0 || fOrigBitmap.height() == 0) {
         return false;
@@ -109,8 +113,58 @@ bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) {
     }
 
     fBitmap = &fOrigBitmap;
-    if (fOrigBitmap.hasMipMap()) {
-        int shift = fOrigBitmap.extractMipLevel(&fMipBitmap,
+
+    // Check to see if the transformation matrix is scaling up, and if 
+    // the matrix is simple, and if the paint has high quality scaling 
+    // turned on.  If so, do the bitmap scale here and remove the scaling component from the matrix.
+
+    uint32_t hqUpsampleMask = SkPaint::kFilterBitmap_Flag
+                            | SkPaint::kHighQualityFilterBitmap_Flag
+                              ;
+    if (inv.getType() == (SkMatrix::kScale_Mask | SkMatrix::kTranslate_Mask) &&
+        (inv.getScaleX() < 1 || inv.getScaleY() < 1) &&
+        ((paint.getFlags() & hqUpsampleMask) == hqUpsampleMask) &&
+        fOrigBitmap.config() == SkBitmap::kARGB_8888_Config) {
+
+        // All the criteria are met; let's make a new bitmap.
+        fScaledBitmap.setConfig(SkBitmap::kARGB_8888_Config, fOrigBitmap.width() / inv.getScaleX(), fOrigBitmap.height() / inv.getScaleY());

[reed1, 2013/07/11 15:43:42]

nit: line-wrap

[reed1, 2013/07/11 15:49:08]

grrrr, this is tricky, since we have one predicate to decide if we should create
fScaledBitmap, and a (potentially) different one to decide if we should actually
create/use a HQ filter...

[humper, 2013/07/11 16:03:03]

Done.

+        fScaledBitmap.allocPixels();
+        fOrigBitmap.scale(&fScaledBitmap);
+        fBitmap = &fScaledBitmap;
+        
+        // Now get rid of the scale factor from the matrix
+        
+        if (m != &fUnitInvMatrix) {
+            fUnitInvMatrix = *m;
+            m = &fUnitInvMatrix;
+        }
+
+        // set the inv matrix type to translate-only; this should have
+        // the side-effect of disabling bitmap filtering also.
+
+        fUnitInvMatrix.setTranslate( 1/m->getScaleX() * m->getTranslateX(), 1/m->getScaleY() * m->getTranslateY() );

[reed1, 2013/07/11 15:43:42]

nit: line-wrap

[humper, 2013/07/11 16:03:03]

Done.

+    } else if (!fBitmap->hasMipMap()) {        
+        // Check to see if the transformation matrix is scaling *down*.
+        // If so, automatically build mipmaps
+    
+        SkPoint v1, v2;
+    
+        // guess if the matrix is scaling down by seeing what it does to two unit vectors.
+    
+        v1.fX = inv.getScaleX();
+        v1.fY = inv.getSkewY();
+        
+        v2.fX = inv.getSkewX();
+        v2.fY = inv.getScaleY();
+        
+        if (v1.fX * v1.fX + v1.fY * v1.fY > 1 ||
+            v2.fX * v2.fX + v2.fY * v2.fY > 1) {
+            fBitmap->buildMipMap();
+        }
+    } 
+    
+    if (fBitmap->hasMipMap()) {
+        int shift = fBitmap->extractMipLevel(&fMipBitmap,
                                                 SkScalarToFixed(m->getScaleX()),
                                                 SkScalarToFixed(m->getSkewY()));
 
@@ -156,20 +210,42 @@ bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) {
     fInvKyFractionalInt = SkScalarToFractionalInt(m->getSkewY());
 
     fAlphaScale = SkAlpha255To256(paint.getAlpha());
+    
+    fShaderProc32 = NULL;
+    fShaderProc16 = NULL;
+    fSampleProc32 = NULL;
+    fSampleProc16 = NULL;
 
     // pick-up filtering from the paint, but only if the matrix is
     // more complex than identity/translate (i.e. no need to pay the cost
     // of filtering if we're not scaled etc.).
     // note: we explicitly check inv, since m might be scaled due to unitinv
     //       trickery, but we don't want to see that for this test
-    fDoFilter = paint.isFilterBitmap() &&
-                (fInvType > SkMatrix::kTranslate_Mask &&
-                 valid_for_filtering(fBitmap->width() | fBitmap->height()));
-
-    fShaderProc32 = NULL;
-    fShaderProc16 = NULL;
-    fSampleProc32 = NULL;
-    fSampleProc16 = NULL;
+    
+    fFilterQuality = kNone_BitmapFilter;
+    
+    if (paint.isFilterBitmap()) {        
+        if ((paint.getFlags() & hqUpsampleMask) == hqUpsampleMask) {
+            // The paint wants high quality sampling, so let's try to do it.
+            fFilterQuality = kHQ_BitmapFilter;
+            fShaderProc32 = this->chooseBitmapFilterProc();
+        }
+        
+        if (!fShaderProc32) {
+            // if no shader proc is set, either the caller didn't ask
+            // for high quality filtering (so we didn't try at all),
+            // or our attempt to install an HQ sampler failed.  
+            // Attempt to use bilerp in either case.
+            
+            // Only install bilerp if the matrix is "interesting" and
+            // the image has a suitable size.
+            
+            if (fInvType > SkMatrix::kTranslate_Mask &&

[reed1, 2013/07/11 15:43:42]

Shouldn't we perform this (not scaled enough to justify any filtering) test
*before* we create a HQ proc?

[reed1, 2013/07/11 15:49:08]

Ah, I see that you are *re*calling this logic after you've (potentially) changed
fBitmap to point to a scaled version. Good. Perhaps we could refactor to make
this clearer (and also perhaps we should do this 2nd-pass logic in the miplevel
case as well.

[humper, 2013/07/11 16:03:03]

the valid_for_filtering function actually checks to make sure the image isn't
too huge, I think :)

However, you're right, we shouldn't install the HQ proc if the matrix is
translate only or simpler.  the chooseBitmapFilterProc() function in
SkBitmapFilter will actually do that by making sure that the matrix is either
affine or scale, so the right thing is actually happening.

+             valid_for_filtering(fBitmap->width() | fBitmap->height())) {
+                 fFilterQuality = kBilerp_BitmapFilter;
+             }
+        }
+    }
 
     fMatrixProc = this->chooseMatrixProc(trivial_matrix);
     if (NULL == fMatrixProc) {
@@ -185,7 +261,7 @@ bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) {
     if (fInvType <= (SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask)) {
         index |= 2;
     }
-    if (fDoFilter) {
+    if (fFilterQuality != kNone_BitmapFilter) {
         index |= 4;
     }
     // bits 3,4,5 encoding the source bitmap format
@@ -302,10 +378,6 @@ bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) {
         fShaderProc32 = this->chooseShaderProc32();
     }
 
-    if (NULL == fShaderProc32) {
-        fShaderProc32 = this->chooseBitmapFilterProc(paint);
-    }
-
     // see if our platform has any accelerated overrides
     this->platformProcs();
 
@@ -319,7 +391,7 @@ static void Clamp_S32_D32_nofilter_trans_shaderproc(const SkBitmapProcState& s,
     SkASSERT(((s.fInvType & ~SkMatrix::kTranslate_Mask)) == 0);
     SkASSERT(s.fInvKy == 0);
     SkASSERT(count > 0 && colors != NULL);
-    SkASSERT(!s.fDoFilter);
+    SkASSERT(s.fFilterQuality == kNone_BitmapFilter);
 
     const int maxX = s.fBitmap->width() - 1;
     const int maxY = s.fBitmap->height() - 1;
@@ -393,7 +465,7 @@ static void Repeat_S32_D32_nofilter_trans_shaderproc(const SkBitmapProcState& s,
     SkASSERT(((s.fInvType & ~SkMatrix::kTranslate_Mask)) == 0);
     SkASSERT(s.fInvKy == 0);
     SkASSERT(count > 0 && colors != NULL);
-    SkASSERT(!s.fDoFilter);
+    SkASSERT(s.fFilterQuality == kNone_BitmapFilter);

[reed1, 2013/07/11 15:43:42]

nit: skia likes to place the literal on the left (but not so important when the
var is const I guess)

 
     const int stopX = s.fBitmap->width();
     const int stopY = s.fBitmap->height();
@@ -439,7 +511,7 @@ static void S32_D32_constX_shaderproc(const SkBitmapProcState& s,
     int iY1   SK_INIT_TO_AVOID_WARNING;
     int iSubY SK_INIT_TO_AVOID_WARNING;
 
-    if (s.fDoFilter) {
+    if (s.fFilterQuality != SkBitmapProcState::kNone_BitmapFilter) {
         SkBitmapProcState::MatrixProc mproc = s.getMatrixProc();
         uint32_t xy[2];
 
@@ -520,7 +592,7 @@ static void S32_D32_constX_shaderproc(const SkBitmapProcState& s,
     const SkPMColor* row0 = s.fBitmap->getAddr32(0, iY0);
     SkPMColor color;
 
-    if (s.fDoFilter) {
+    if (s.fFilterQuality != SkBitmapProcState::kNone_BitmapFilter) {
         const SkPMColor* row1 = s.fBitmap->getAddr32(0, iY1);
 
         if (s.fAlphaScale < 256) {
@@ -576,7 +648,9 @@ SkBitmapProcState::ShaderProc32 SkBitmapProcState::chooseShaderProc32() {
     static const unsigned kMask = SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask;
 
     if (1 == fBitmap->width() && 0 == (fInvType & ~kMask)) {
-        if (!fDoFilter && fInvType <= SkMatrix::kTranslate_Mask && !this->setupForTranslate()) {
+        if (fFilterQuality == kNone_BitmapFilter && 
+            fInvType <= SkMatrix::kTranslate_Mask && 
+            !this->setupForTranslate()) {
             return DoNothing_shaderproc;
         }
         return S32_D32_constX_shaderproc;
@@ -588,7 +662,7 @@ SkBitmapProcState::ShaderProc32 SkBitmapProcState::chooseShaderProc32() {
     if (fInvType > SkMatrix::kTranslate_Mask) {
         return NULL;
     }
-    if (fDoFilter) {
+    if (fFilterQuality != kNone_BitmapFilter) {
         return NULL;
     }
 
@@ -684,9 +758,9 @@ void SkBitmapProcState::DebugMatrixProc(const SkBitmapProcState& state,
     //  scale -vs- affine
     //  filter -vs- nofilter
     if (state.fInvType <= (SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask)) {
-        proc = state.fDoFilter ? check_scale_filter : check_scale_nofilter;
+        proc = state.fFilterQuality != kNone_BitmapFilter ? check_scale_filter : check_scale_nofilter;
     } else {
-        proc = state.fDoFilter ? check_affine_filter : check_affine_nofilter;
+        proc = state.fFilterQuality != kNone_BitmapFilter ? check_affine_filter : check_affine_nofilter;
     }
     proc(bitmapXY, count, state.fBitmap->width(), state.fBitmap->height());
 }
@@ -721,7 +795,7 @@ int SkBitmapProcState::maxCountForBufferSize(size_t bufferSize) const {
         size >>= 2;
     }
 
-    if (fDoFilter) {
+    if (fFilterQuality != kNone_BitmapFilter) {
         size >>= 1;
     }