Bug 687188 - Skia radial gradients should use the 0/1 color stop values for clamping. r=jrmuziel
authorMatt Woodrow <mwoodrow@mozilla.com>
Fri, 28 Oct 2011 20:08:54 +1300
changeset 79363 2b40846bc3c85746fcb92ffabc09478b467054de
parent 79362 7a6b1c88a6cefce12de5ea30e3ebe3144f21fe37
child 79364 b79b68029f1de7fd719afdc8b93342957c6d414c
push id21389
push usermbrubeck@mozilla.com
push dateFri, 28 Oct 2011 18:19:50 +0000
treeherdermozilla-central@1c7e1db3645b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuziel
bugs687188
milestone10.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 687188 - Skia radial gradients should use the 0/1 color stop values for clamping. r=jrmuziel
gfx/skia/fix-gradient-clamp.patch
gfx/skia/src/effects/SkGradientShader.cpp
gfx/skia/update.sh
new file mode 100644
--- /dev/null
+++ b/gfx/skia/fix-gradient-clamp.patch
@@ -0,0 +1,224 @@
+diff --git a/gfx/skia/src/effects/SkGradientShader.cpp b/gfx/skia/src/effects/SkGradientShader.cpp
+--- a/gfx/skia/src/effects/SkGradientShader.cpp
++++ b/gfx/skia/src/effects/SkGradientShader.cpp
+@@ -165,16 +165,17 @@ private:
+ 
+     mutable uint16_t*   fCache16;   // working ptr. If this is NULL, we need to recompute the cache values
+     mutable SkPMColor*  fCache32;   // working ptr. If this is NULL, we need to recompute the cache values
+ 
+     mutable uint16_t*   fCache16Storage;    // storage for fCache16, allocated on demand
+     mutable SkMallocPixelRef* fCache32PixelRef;
+     mutable unsigned    fCacheAlpha;        // the alpha value we used when we computed the cache. larger than 8bits so we can store uninitialized value
+ 
++    static SkPMColor PremultiplyColor(SkColor c0, U8CPU alpha);
+     static void Build16bitCache(uint16_t[], SkColor c0, SkColor c1, int count);
+     static void Build32bitCache(SkPMColor[], SkColor c0, SkColor c1, int count,
+                                 U8CPU alpha);
+     void setCacheAlpha(U8CPU alpha) const;
+ 
+     typedef SkShader INHERITED;
+ };
+ 
+@@ -505,16 +506,31 @@ static inline U8CPU dither_fixed_to_8(Sk
+  *  For dithering with premultiply, we want to ceiling the alpha component,
+  *  to ensure that it is always >= any color component.
+  */
+ static inline U8CPU dither_ceil_fixed_to_8(SkFixed n) {
+     n >>= 8;
+     return ((n << 1) - (n | (n >> 8))) >> 8;
+ }
+ 
++SkPMColor Gradient_Shader::PremultiplyColor(SkColor c0, U8CPU paintAlpha)
++{
++    SkFixed a = SkMulDiv255Round(SkColorGetA(c0), paintAlpha);
++    SkFixed r = SkColorGetR(c0);
++    SkFixed g = SkColorGetG(c0);
++    SkFixed b = SkColorGetB(c0);
++    
++    a = SkIntToFixed(a) + 0x8000;
++    r = SkIntToFixed(r) + 0x8000;
++    g = SkIntToFixed(g) + 0x8000;
++    b = SkIntToFixed(b) + 0x8000;
++        
++    return SkPremultiplyARGBInline(a >> 16, r >> 16, g >> 16, b >> 16);
++}
++
+ void Gradient_Shader::Build32bitCache(SkPMColor cache[], SkColor c0, SkColor c1,
+                                       int count, U8CPU paintAlpha) {
+     SkASSERT(count > 1);
+ 
+     // need to apply paintAlpha to our two endpoints
+     SkFixed a = SkMulDiv255Round(SkColorGetA(c0), paintAlpha);
+     SkFixed da;
+     {
+@@ -606,24 +622,24 @@ const uint16_t* Gradient_Shader::getCach
+         }
+     }
+     return fCache16;
+ }
+ 
+ const SkPMColor* Gradient_Shader::getCache32() const {
+     if (fCache32 == NULL) {
+         // double the count for dither entries
+-        const int entryCount = kCache32Count * 2;
++        const int entryCount = kCache32Count * 2 + 2;
+         const size_t allocSize = sizeof(SkPMColor) * entryCount;
+ 
+         if (NULL == fCache32PixelRef) {
+             fCache32PixelRef = SkNEW_ARGS(SkMallocPixelRef,
+                                           (NULL, allocSize, NULL));
+         }
+-        fCache32 = (SkPMColor*)fCache32PixelRef->getAddr();
++        fCache32 = (SkPMColor*)fCache32PixelRef->getAddr() + 1;
+         if (fColorCount == 2) {
+             Build32bitCache(fCache32, fOrigColors[0], fOrigColors[1],
+                             kCache32Count, fCacheAlpha);
+         } else {
+             Rec* rec = fRecs;
+             int prevIndex = 0;
+             for (int i = 1; i < fColorCount; i++) {
+                 int nextIndex = SkFixedToFFFF(rec[i].fPos) >> (16 - kCache32Bits);
+@@ -637,28 +653,31 @@ const SkPMColor* Gradient_Shader::getCac
+             }
+             SkASSERT(prevIndex == kCache32Count - 1);
+         }
+ 
+         if (fMapper) {
+             SkMallocPixelRef* newPR = SkNEW_ARGS(SkMallocPixelRef,
+                                                  (NULL, allocSize, NULL));
+             SkPMColor* linear = fCache32;           // just computed linear data
+-            SkPMColor* mapped = (SkPMColor*)newPR->getAddr();    // storage for mapped data
++            SkPMColor* mapped = (SkPMColor*)newPR->getAddr() + 1;    // storage for mapped data
+             SkUnitMapper* map = fMapper;
+             for (int i = 0; i < kCache32Count; i++) {
+                 int index = map->mapUnit16((i << 8) | i) >> 8;
+                 mapped[i] = linear[index];
+                 mapped[i + kCache32Count] = linear[index + kCache32Count];
+             }
+             fCache32PixelRef->unref();
+             fCache32PixelRef = newPR;
+-            fCache32 = (SkPMColor*)newPR->getAddr();
++            fCache32 = (SkPMColor*)newPR->getAddr() + 1;
+         }
+     }
++    //Write the clamp colours into the first and last entries of fCache32
++    fCache32[-1] = PremultiplyColor(fOrigColors[0], fCacheAlpha);
++    fCache32[kCache32Count * 2] = PremultiplyColor(fOrigColors[fColorCount - 1], fCacheAlpha);
+     return fCache32;
+ }
+ 
+ /*
+  *  Because our caller might rebuild the same (logically the same) gradient
+  *  over and over, we'd like to return exactly the same "bitmap" if possible,
+  *  allowing the client to utilize a cache of our bitmap (e.g. with a GPU).
+  *  To do that, we maintain a private cache of built-bitmaps, based on our
+@@ -866,29 +885,37 @@ void Linear_Gradient::shadeSpan(int x, i
+             dx = dxStorage[0];
+         } else {
+             SkASSERT(fDstToIndexClass == kLinear_MatrixClass);
+             dx = SkScalarToFixed(fDstToIndex.getScaleX());
+         }
+ 
+         if (SkFixedNearlyZero(dx)) {
+             // we're a vertical gradient, so no change in a span
+-            unsigned fi = proc(fx);
+-            SkASSERT(fi <= 0xFFFF);
+-            // TODO: dither version
+-            sk_memset32(dstC, cache[fi >> (16 - kCache32Bits)], count);
++            if (proc == clamp_tileproc) {
++                if (fx < 0) {
++                    sk_memset32(dstC, cache[-1], count);
++                } else if (fx > 0xFFFF) {
++                    sk_memset32(dstC, cache[kCache32Count * 2], count);
++                } else {
++                    sk_memset32(dstC, cache[fx >> (16 - kCache32Bits)], count);
++                }
++            } else {
++                unsigned fi = proc(fx);
++                SkASSERT(fi <= 0xFFFF);
++                // TODO: dither version
++                sk_memset32(dstC, cache[fi >> (16 - kCache32Bits)], count);
++            }
+         } else if (proc == clamp_tileproc) {
+             SkClampRange range;
+             range.init(fx, dx, count, 0, 0xFF);
+ 
+             if ((count = range.fCount0) > 0) {
+-                sk_memset32_dither(dstC,
+-                                   cache[toggle + range.fV0],
+-                                   cache[(toggle ^ TOGGLE_MASK) + range.fV0],
+-                                   count);
++                // Do we really want to dither the clamp values?
++                sk_memset32(dstC, cache[-1], count);
+                 dstC += count;
+             }
+             if ((count = range.fCount1) > 0) {
+                 int unroll = count >> 3;
+                 fx = range.fFx1;
+                 for (int i = 0; i < unroll; i++) {
+                     NO_CHECK_ITER;  NO_CHECK_ITER;
+                     NO_CHECK_ITER;  NO_CHECK_ITER;
+@@ -897,20 +924,17 @@ void Linear_Gradient::shadeSpan(int x, i
+                 }
+                 if ((count &= 7) > 0) {
+                     do {
+                         NO_CHECK_ITER;
+                     } while (--count != 0);
+                 }
+             }
+             if ((count = range.fCount2) > 0) {
+-                sk_memset32_dither(dstC,
+-                                   cache[toggle + range.fV1],
+-                                   cache[(toggle ^ TOGGLE_MASK) + range.fV1],
+-                                   count);
++                sk_memset32(dstC, cache[kCache32Count * 2], count);
+             }
+         } else if (proc == mirror_tileproc) {
+             do {
+                 unsigned fi = mirror_8bits(fx >> 8);
+                 SkASSERT(fi <= 0xFF);
+                 fx += dx;
+                 *dstC++ = cache[toggle + fi];
+                 toggle ^= TOGGLE_MASK;
+@@ -1662,19 +1686,24 @@ public:
+             }
+             SkScalar b = (SkScalarMul(fDiff.fX, fx) +
+                          SkScalarMul(fDiff.fY, fy) - fStartRadius) * 2;
+             SkScalar db = (SkScalarMul(fDiff.fX, dx) +
+                           SkScalarMul(fDiff.fY, dy)) * 2;
+             if (proc == clamp_tileproc) {
+                 for (; count > 0; --count) {
+                     SkFixed t = two_point_radial(b, fx, fy, fSr2D2, foura, fOneOverTwoA, posRoot);
+-                    SkFixed index = SkClampMax(t, 0xFFFF);
+-                    SkASSERT(index <= 0xFFFF);
+-                    *dstC++ = cache[index >> (16 - kCache32Bits)];
++                    if (t < 0) {
++                      *dstC++ = cache[-1];
++                    } else if (t > 0xFFFF) {
++                      *dstC++ = cache[kCache32Count * 2];
++                    } else {
++                      SkASSERT(t <= 0xFFFF);
++                      *dstC++ = cache[t >> (16 - kCache32Bits)];
++                    }
+                     fx += dx;
+                     fy += dy;
+                     b += db;
+                 }
+             } else if (proc == mirror_tileproc) {
+                 for (; count > 0; --count) {
+                     SkFixed t = two_point_radial(b, fx, fy, fSr2D2, foura, fOneOverTwoA, posRoot);
+                     SkFixed index = mirror_tileproc(t);
+diff --git a/gfx/skia/update.sh b/gfx/skia/update.sh
+--- a/gfx/skia/update.sh
++++ b/gfx/skia/update.sh
+@@ -90,9 +90,10 @@ if [ -n "$rev" ]; then
+   version=$rev
+   sed -i "" "s/r[0-9][0-9][0-9][0-9]/r$version/" README_MOZILLA
+ else
+   echo "Remember to update README_MOZILLA with the version details."
+ fi
+ 
+ # Patch to get arm opts to build with frame pointers enabled. Bug 689069
+ patch -p3 < arm-opts.patch
++patch -p3
+ 
--- a/gfx/skia/src/effects/SkGradientShader.cpp
+++ b/gfx/skia/src/effects/SkGradientShader.cpp
@@ -165,16 +165,17 @@ private:
 
     mutable uint16_t*   fCache16;   // working ptr. If this is NULL, we need to recompute the cache values
     mutable SkPMColor*  fCache32;   // working ptr. If this is NULL, we need to recompute the cache values
 
     mutable uint16_t*   fCache16Storage;    // storage for fCache16, allocated on demand
     mutable SkMallocPixelRef* fCache32PixelRef;
     mutable unsigned    fCacheAlpha;        // the alpha value we used when we computed the cache. larger than 8bits so we can store uninitialized value
 
+    static SkPMColor PremultiplyColor(SkColor c0, U8CPU alpha);
     static void Build16bitCache(uint16_t[], SkColor c0, SkColor c1, int count);
     static void Build32bitCache(SkPMColor[], SkColor c0, SkColor c1, int count,
                                 U8CPU alpha);
     void setCacheAlpha(U8CPU alpha) const;
 
     typedef SkShader INHERITED;
 };
 
@@ -505,16 +506,31 @@ static inline U8CPU dither_fixed_to_8(Sk
  *  For dithering with premultiply, we want to ceiling the alpha component,
  *  to ensure that it is always >= any color component.
  */
 static inline U8CPU dither_ceil_fixed_to_8(SkFixed n) {
     n >>= 8;
     return ((n << 1) - (n | (n >> 8))) >> 8;
 }
 
+SkPMColor Gradient_Shader::PremultiplyColor(SkColor c0, U8CPU paintAlpha)
+{
+    SkFixed a = SkMulDiv255Round(SkColorGetA(c0), paintAlpha);
+    SkFixed r = SkColorGetR(c0);
+    SkFixed g = SkColorGetG(c0);
+    SkFixed b = SkColorGetB(c0);
+    
+    a = SkIntToFixed(a) + 0x8000;
+    r = SkIntToFixed(r) + 0x8000;
+    g = SkIntToFixed(g) + 0x8000;
+    b = SkIntToFixed(b) + 0x8000;
+        
+    return SkPremultiplyARGBInline(a >> 16, r >> 16, g >> 16, b >> 16);
+}
+
 void Gradient_Shader::Build32bitCache(SkPMColor cache[], SkColor c0, SkColor c1,
                                       int count, U8CPU paintAlpha) {
     SkASSERT(count > 1);
 
     // need to apply paintAlpha to our two endpoints
     SkFixed a = SkMulDiv255Round(SkColorGetA(c0), paintAlpha);
     SkFixed da;
     {
@@ -606,24 +622,24 @@ const uint16_t* Gradient_Shader::getCach
         }
     }
     return fCache16;
 }
 
 const SkPMColor* Gradient_Shader::getCache32() const {
     if (fCache32 == NULL) {
         // double the count for dither entries
-        const int entryCount = kCache32Count * 2;
+        const int entryCount = kCache32Count * 2 + 2;
         const size_t allocSize = sizeof(SkPMColor) * entryCount;
 
         if (NULL == fCache32PixelRef) {
             fCache32PixelRef = SkNEW_ARGS(SkMallocPixelRef,
                                           (NULL, allocSize, NULL));
         }
-        fCache32 = (SkPMColor*)fCache32PixelRef->getAddr();
+        fCache32 = (SkPMColor*)fCache32PixelRef->getAddr() + 1;
         if (fColorCount == 2) {
             Build32bitCache(fCache32, fOrigColors[0], fOrigColors[1],
                             kCache32Count, fCacheAlpha);
         } else {
             Rec* rec = fRecs;
             int prevIndex = 0;
             for (int i = 1; i < fColorCount; i++) {
                 int nextIndex = SkFixedToFFFF(rec[i].fPos) >> (16 - kCache32Bits);
@@ -637,28 +653,31 @@ const SkPMColor* Gradient_Shader::getCac
             }
             SkASSERT(prevIndex == kCache32Count - 1);
         }
 
         if (fMapper) {
             SkMallocPixelRef* newPR = SkNEW_ARGS(SkMallocPixelRef,
                                                  (NULL, allocSize, NULL));
             SkPMColor* linear = fCache32;           // just computed linear data
-            SkPMColor* mapped = (SkPMColor*)newPR->getAddr();    // storage for mapped data
+            SkPMColor* mapped = (SkPMColor*)newPR->getAddr() + 1;    // storage for mapped data
             SkUnitMapper* map = fMapper;
             for (int i = 0; i < kCache32Count; i++) {
                 int index = map->mapUnit16((i << 8) | i) >> 8;
                 mapped[i] = linear[index];
                 mapped[i + kCache32Count] = linear[index + kCache32Count];
             }
             fCache32PixelRef->unref();
             fCache32PixelRef = newPR;
-            fCache32 = (SkPMColor*)newPR->getAddr();
+            fCache32 = (SkPMColor*)newPR->getAddr() + 1;
         }
     }
+    //Write the clamp colours into the first and last entries of fCache32
+    fCache32[-1] = PremultiplyColor(fOrigColors[0], fCacheAlpha);
+    fCache32[kCache32Count * 2] = PremultiplyColor(fOrigColors[fColorCount - 1], fCacheAlpha);
     return fCache32;
 }
 
 /*
  *  Because our caller might rebuild the same (logically the same) gradient
  *  over and over, we'd like to return exactly the same "bitmap" if possible,
  *  allowing the client to utilize a cache of our bitmap (e.g. with a GPU).
  *  To do that, we maintain a private cache of built-bitmaps, based on our
@@ -866,29 +885,37 @@ void Linear_Gradient::shadeSpan(int x, i
             dx = dxStorage[0];
         } else {
             SkASSERT(fDstToIndexClass == kLinear_MatrixClass);
             dx = SkScalarToFixed(fDstToIndex.getScaleX());
         }
 
         if (SkFixedNearlyZero(dx)) {
             // we're a vertical gradient, so no change in a span
-            unsigned fi = proc(fx);
-            SkASSERT(fi <= 0xFFFF);
-            // TODO: dither version
-            sk_memset32(dstC, cache[fi >> (16 - kCache32Bits)], count);
+            if (proc == clamp_tileproc) {
+                if (fx < 0) {
+                    sk_memset32(dstC, cache[-1], count);
+                } else if (fx > 0xFFFF) {
+                    sk_memset32(dstC, cache[kCache32Count * 2], count);
+                } else {
+                    sk_memset32(dstC, cache[fx >> (16 - kCache32Bits)], count);
+                }
+            } else {
+                unsigned fi = proc(fx);
+                SkASSERT(fi <= 0xFFFF);
+                // TODO: dither version
+                sk_memset32(dstC, cache[fi >> (16 - kCache32Bits)], count);
+            }
         } else if (proc == clamp_tileproc) {
             SkClampRange range;
             range.init(fx, dx, count, 0, 0xFF);
 
             if ((count = range.fCount0) > 0) {
-                sk_memset32_dither(dstC,
-                                   cache[toggle + range.fV0],
-                                   cache[(toggle ^ TOGGLE_MASK) + range.fV0],
-                                   count);
+                // Do we really want to dither the clamp values?
+                sk_memset32(dstC, cache[-1], count);
                 dstC += count;
             }
             if ((count = range.fCount1) > 0) {
                 int unroll = count >> 3;
                 fx = range.fFx1;
                 for (int i = 0; i < unroll; i++) {
                     NO_CHECK_ITER;  NO_CHECK_ITER;
                     NO_CHECK_ITER;  NO_CHECK_ITER;
@@ -897,20 +924,17 @@ void Linear_Gradient::shadeSpan(int x, i
                 }
                 if ((count &= 7) > 0) {
                     do {
                         NO_CHECK_ITER;
                     } while (--count != 0);
                 }
             }
             if ((count = range.fCount2) > 0) {
-                sk_memset32_dither(dstC,
-                                   cache[toggle + range.fV1],
-                                   cache[(toggle ^ TOGGLE_MASK) + range.fV1],
-                                   count);
+                sk_memset32(dstC, cache[kCache32Count * 2], count);
             }
         } else if (proc == mirror_tileproc) {
             do {
                 unsigned fi = mirror_8bits(fx >> 8);
                 SkASSERT(fi <= 0xFF);
                 fx += dx;
                 *dstC++ = cache[toggle + fi];
                 toggle ^= TOGGLE_MASK;
@@ -1662,19 +1686,24 @@ public:
             }
             SkScalar b = (SkScalarMul(fDiff.fX, fx) +
                          SkScalarMul(fDiff.fY, fy) - fStartRadius) * 2;
             SkScalar db = (SkScalarMul(fDiff.fX, dx) +
                           SkScalarMul(fDiff.fY, dy)) * 2;
             if (proc == clamp_tileproc) {
                 for (; count > 0; --count) {
                     SkFixed t = two_point_radial(b, fx, fy, fSr2D2, foura, fOneOverTwoA, posRoot);
-                    SkFixed index = SkClampMax(t, 0xFFFF);
-                    SkASSERT(index <= 0xFFFF);
-                    *dstC++ = cache[index >> (16 - kCache32Bits)];
+                    if (t < 0) {
+                      *dstC++ = cache[-1];
+                    } else if (t > 0xFFFF) {
+                      *dstC++ = cache[kCache32Count * 2];
+                    } else {
+                      SkASSERT(t <= 0xFFFF);
+                      *dstC++ = cache[t >> (16 - kCache32Bits)];
+                    }
                     fx += dx;
                     fy += dy;
                     b += db;
                 }
             } else if (proc == mirror_tileproc) {
                 for (; count > 0; --count) {
                     SkFixed t = two_point_radial(b, fx, fy, fSr2D2, foura, fOneOverTwoA, posRoot);
                     SkFixed index = mirror_tileproc(t);
--- a/gfx/skia/update.sh
+++ b/gfx/skia/update.sh
@@ -88,11 +88,13 @@ fi
 
 if [ -n "$rev" ]; then
   version=$rev
   sed -i "" "s/r[0-9][0-9][0-9][0-9]/r$version/" README_MOZILLA
 else
   echo "Remember to update README_MOZILLA with the version details."
 fi
 
-# Patch to get arm opts to build with frame pointers enabled. Bug 689069
+# Bug 689069 - Patch to get arm opts to build with frame pointers enabled.
 patch -p3 < arm-opts.patch
+# Bug 687188 - Skia radial gradients should use the 0/1 color stop values for clamping.
+patch -p3 < fix-gradient-clamp.patch