Remove support for inheriting the paint color from SkColorShader

BUG=skia:2453
R=reed@google.com, mtklein@google.com

Author: bsalomon@google.com

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

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

bench/ShaderMaskBench.cpp

@@ -44,8 +44,7 @@ public:
 
         fPaint.setAntiAlias(kBW != fq);
         fPaint.setLCDRenderText(kLCD == fq);
-        fPaint.setAlpha(isOpaque ? 0xFF : 0x80);
-        fPaint.setShader(new SkColorShader)->unref();
+        fPaint.setShader(new SkColorShader(isOpaque ? 0xFFFFFFFF : 0x80808080))->unref();
     }
 
 protected:

bench/valgrind.supp

@@ -81,134 +81,3 @@
    fun:_Z9tool_mainiPPc
    fun:(below main)
 }
-{
-   shadermask_LCD_* http://skbug.com/2453
-   Memcheck:Cond
-   fun:_ZL16SkMulDiv255Roundjj
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_ZN11SkBenchmark4drawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-   fun:main
-}
-{
-   shadermask_LCD_* 2 http://skbug.com/2453
-   Memcheck:Cond
-   fun:_ZL8SkMulS16ii
-   fun:_ZL16SkMulDiv255Roundjj
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_ZN11SkBenchmark4drawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-   fun:main
-}
-{
-   shadermask_LCD_* 3 http://skbug.com/2453
-   Memcheck:Cond
-   fun:_ZL23SkPremultiplyARGBInlinejjjj
-   fun:_Z18SkPreMultiplyColorj
-   fun:_ZL15SkColor2GrColorj
-   fun:_ZN12_GLOBAL__N_123skPaint2GrPaintNoShaderEP11SkGpuDeviceRK7SkPaintbbP7GrPaint
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_ZN11SkBenchmark4drawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-   fun:main
-}
-{
-   shadermask_LCD_* 4 http://skbug.com/2453
-   Memcheck:Cond
-   fun:_ZL16SkMulDiv255Roundjj
-   fun:_ZL23SkPremultiplyARGBInlinejjjj
-   fun:_Z18SkPreMultiplyColorj
-   fun:_ZL15SkColor2GrColorj
-   fun:_ZN12_GLOBAL__N_123skPaint2GrPaintNoShaderEP11SkGpuDeviceRK7SkPaintbbP7GrPaint
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_ZN11SkBenchmark4drawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-   fun:main
-}
-{
-   shadermask_LCD_* 5 http://skbug.com/2453
-   Memcheck:Cond
-   fun:_ZL16SkMulDiv255Roundjj
-   fun:_ZL23SkPremultiplyARGBInlinejjjj
-   fun:_Z18SkPreMultiplyColorj
-   fun:_ZL15SkColor2GrColorj
-   fun:_ZN12_GLOBAL__N_123skPaint2GrPaintNoShaderEP11SkGpuDeviceRK7SkPaintbbP7GrPaint
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_ZN11SkBenchmark4drawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-   fun:main
-}
-{
-   shadermask_LCD_* 6 http://skbug.com/2453
-   Memcheck:Cond
-   fun:_ZL8SkMulS16ii
-   fun:_ZL16SkMulDiv255Roundjj
-   fun:_ZL23SkPremultiplyARGBInlinejjjj
-   fun:_Z18SkPreMultiplyColorj
-   fun:_ZL15SkColor2GrColorj
-   fun:_ZN12_GLOBAL__N_123skPaint2GrPaintNoShaderEP11SkGpuDeviceRK7SkPaintbbP7GrPaint
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_ZN11SkBenchmark4drawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-}
-{
-   shadermask_LCD_* 7 http://skbug.com/2453
-   Memcheck:Cond
-   fun:_ZL12SkPackARGB32jjjj
-   fun:_ZL23SkPremultiplyARGBInlinejjjj
-   fun:_Z18SkPreMultiplyColorj
-   fun:_ZL15SkColor2GrColorj
-   fun:_ZN12_GLOBAL__N_123skPaint2GrPaintNoShaderEP11SkGpuDeviceRK7SkPaintbbP7GrPaint
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_ZN11SkBenchmark4drawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-   fun:main
-}
-{
-   shadermask_LCD_* 8 http://skbug.com/2453
-   Memcheck:Cond
-   fun:_ZL15GrColorPackRGBAjjjj
-   fun:_ZL15SkColor2GrColorj
-   fun:_ZN12_GLOBAL__N_123skPaint2GrPaintNoShaderEP11SkGpuDeviceRK7SkPaintbbP7GrPaint
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_ZN11SkBenchmark4drawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-   fun:main
-}
-{
-   shadermask_LCD_* Release http://skbug.com/2453
-   Memcheck:Cond
-   fun:_Z18SkPreMultiplyColorj
-   fun:_ZN12_GLOBAL__N_123skPaint2GrPaintNoShaderEP11SkGpuDeviceRK7SkPaintbbP7GrPaint
-   fun:_ZN12_GLOBAL__N_121skPaint2GrPaintShaderEP11SkGpuDeviceRK7SkPaintbP7GrPaint.constprop.127
-   fun:_ZN11SkGpuDevice8drawTextERK6SkDrawPKvmffRK7SkPaint
-   fun:_ZN8SkCanvas8drawTextEPKvmffRK7SkPaint
-   fun:_ZN15ShaderMaskBench6onDrawEiP8SkCanvas
-   fun:_Z9tool_mainiPPc
-   fun:(below main)
-}
-

include/core/SkColorShader.h

@@ -19,16 +19,11 @@
 */
 class SK_API SkColorShader : public SkShader {
 public:
-    /** Create a ColorShader that will inherit its color from the Paint
-        at draw time.
-    */
-    SkColorShader();
-
     /** Create a ColorShader that ignores the color in the paint, and uses the
         specified color. Note: like all shaders, at draw time the paint's alpha
         will be respected, and is applied to the specified color.
     */
-    SkColorShader(SkColor c);
+    explicit SkColorShader(SkColor c);
 
     virtual bool isOpaque() const SK_OVERRIDE;
 
@@ -65,7 +60,7 @@ public:
                                  SkMatrix* outMatrix,
                                  TileMode xy[2]) const SK_OVERRIDE;
 
-    virtual GradientType asAGradient(GradientInfo* info) const SK_OVERRIDE;
+    virtual GradientType asAGradient(GradientInfo* info) const SK_OVERRIDE; 
 
     SK_TO_STRING_OVERRIDE()
     SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkColorShader)
@@ -76,7 +71,6 @@ protected:
 
 private:
     SkColor     fColor;         // ignored if fInheritColor is true
-    SkBool8     fInheritColor;
 
     typedef SkShader INHERITED;
 };

include/core/SkPicture.h

@@ -326,13 +326,14 @@ protected:
     // V23: SkPaint::FilterLevel became a real enum
     // V24: SkTwoPointConicalGradient now has fFlipped flag for gradient flipping
     // V25: SkDashPathEffect now only writes phase and interval array when flattening
+    // V26: Removed boolean from SkColorShader for inheriting color from SkPaint.
 
     // Note: If the picture version needs to be increased then please follow the
     // steps to generate new SKPs in (only accessible to Googlers): http://goo.gl/qATVcw
 
     // Only SKPs within the min/current picture version range (inclusive) can be read.
     static const uint32_t MIN_PICTURE_VERSION = 19;
-    static const uint32_t CURRENT_PICTURE_VERSION = 25;
+    static const uint32_t CURRENT_PICTURE_VERSION = 26;
 
     mutable uint32_t      fUniqueID;
 

src/core/SkBlitter.cpp

@@ -928,8 +928,9 @@ SkBlitter* SkBlitter::Choose(const SkBitmap& device,
     if (NULL == shader) {
         if (mode) {
             // xfermodes (and filters) require shaders for our current blitters
-            shader = SkNEW(SkColorShader);
+            shader = SkNEW_ARGS(SkColorShader, (paint->getColor()));
             paint.writable()->setShader(shader)->unref();
+            paint.writable()->setAlpha(0xFF);
         } else if (cf) {
             // if no shader && no xfermode, we just apply the colorfilter to
             // our color and move on.

src/core/SkShader.cpp

@@ -203,37 +203,29 @@ void SkShader::toString(SkString* str) const {
 #include "SkColorShader.h"
 #include "SkUtils.h"
 
-SkColorShader::SkColorShader()
-    : fColor()
-    , fInheritColor(true) {
-}
-
 SkColorShader::SkColorShader(SkColor c)
-    : fColor(c)
-    , fInheritColor(false) {
+    : fColor(c) {
 }
 
 bool SkColorShader::isOpaque() const {
-    if (fInheritColor) {
-        return true; // using paint's alpha
-    }
     return SkColorGetA(fColor) == 255;
 }
 
 SkColorShader::SkColorShader(SkReadBuffer& b) : INHERITED(b) {
-    fInheritColor = b.readBool();
-    if (fInheritColor) {
-        return;
+    // V25_COMPATIBILITY_CODE We had a boolean to make the color shader inherit the paint's
+    // color. We don't support that any more.
+    if (b.pictureVersion() < 26 && 0 != b.pictureVersion()) {
+        if (b.readBool()) {
+            SkDEBUGFAIL("We shouldn't have pictures that recorded the inherited case.");
+            fColor = SK_ColorWHITE;
+            return;
+        }
     }
     fColor = b.readColor();
 }
 
 void SkColorShader::flatten(SkWriteBuffer& buffer) const {
     this->INHERITED::flatten(buffer);
-    buffer.writeBool(fInheritColor);
-    if (fInheritColor) {
-        return;
-    }
     buffer.writeColor(fColor);
 }
 
@@ -260,16 +252,11 @@ SkColorShader::ColorShaderContext::ColorShaderContext(const SkColorShader& shade
                                                       const SkMatrix& matrix)
     : INHERITED(shader, device, paint, matrix)
 {
+    SkColor color;
     unsigned a;
 
-    SkColor color;
-    if (shader.fInheritColor) {
-        color = paint.getColor();
-        a = SkColorGetA(color);
-    } else {
-        color = shader.fColor;
-        a = SkAlphaMul(SkColorGetA(color), SkAlpha255To256(paint.getAlpha()));
-    }
+    color = shader.fColor;
+    a = SkAlphaMul(SkColorGetA(color), SkAlpha255To256(paint.getAlpha()));
 
     unsigned r = SkColorGetR(color);
     unsigned g = SkColorGetG(color);
@@ -327,12 +314,8 @@ SkShader::GradientType SkColorShader::asAGradient(GradientInfo* info) const {
 void SkColorShader::toString(SkString* str) const {
     str->append("SkColorShader: (");
 
-    if (fInheritColor) {
-        str->append("Color: inherited from paint");
-    } else {
-        str->append("Color: ");
-        str->appendHex(fColor);
-    }
+    str->append("Color: ");
+    str->appendHex(fColor);
 
     this->INHERITED::toString(str);
 

tests/ShaderOpacityTest.cpp

@@ -103,11 +103,6 @@ static void test_color(skiatest::Reporter* reporter)
     REPORTER_ASSERT(reporter, colorShader2.isOpaque());
     SkColorShader colorShader3(SkColorSetARGB(0x7F,0,0,0));
     REPORTER_ASSERT(reporter, !colorShader3.isOpaque());
-
-    // with inherrited color, shader must declare itself as opaque,
-    // since lack of opacity will depend solely on the paint
-    SkColorShader colorShader4;
-    REPORTER_ASSERT(reporter, colorShader4.isOpaque());
 }
 
 DEF_TEST(ShaderOpacity, reporter) {