Alter SkXfermode's asFragmentProcessor & asXPFactory contracts

src/core/SkXfermode.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkXfermode.h"
 #include "SkXfermode_proccoeff.h"
 #include "SkColorPriv.h"
@@ skipping 632 matching lines @@
     { color_modeproc,       CANNOT_USE_COEFF,       CANNOT_USE_COEFF },
     { luminosity_modeproc,  CANNOT_USE_COEFF,       CANNOT_USE_COEFF },
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 bool SkXfermode::asMode(Mode* mode) const {
     return false;
 }
 
-bool SkXfermode::asFragmentProcessor(const GrFragmentProcessor**,
-                                     const GrFragmentProcessor*) const {
-    return false;
+const GrFragmentProcessor* SkXfermode::asFragmentProcessor(const GrFragmentProcessor*) const {
+    // This should never be called.
+    // TODO: make pure virtual in SkXfermode once Android update lands
+    SkASSERT(0);
+    return nullptr;
 }
 
-bool SkXfermode::asXPFactory(GrXPFactory**) const {
-    return false;
+GrXPFactory* SkXfermode::asXPFactory() const {
+    // This should never be called.
+    // TODO: make pure virtual in SkXfermode once Android update lands
+    SkASSERT(0);
+    return nullptr;
 }
 
 SkPMColor SkXfermode::xferColor(SkPMColor src, SkPMColor dst) const{
     // no-op. subclasses should override this
     return dst;
 }
 
 void SkXfermode::xfer32(SkPMColor* SK_RESTRICT dst,
                         const SkPMColor* SK_RESTRICT src, int count,
                         const SkAlpha* SK_RESTRICT aa) const {
@@ skipping 225 matching lines @@
             }
         }
     }
 }
 
 #if SK_SUPPORT_GPU
 #include "effects/GrCustomXfermode.h"
 #include "effects/GrPorterDuffXferProcessor.h"
 #include "effects/GrXfermodeFragmentProcessor.h"
 
-bool SkProcCoeffXfermode::asFragmentProcessor(const GrFragmentProcessor** fp,
-                                              const GrFragmentProcessor* dst) const {
-    if (fp) {
-        SkASSERT(dst);
-        *fp = GrXfermodeFragmentProcessor::CreateFromDstProcessor(dst, fMode);
-        SkASSERT(*fp || kSrc_Mode == fMode);
-    }
-    return true;
+const GrFragmentProcessor* SkProcCoeffXfermode::asFragmentProcessor(
+                                                            const GrFragmentProcessor* dst) const {
+    SkASSERT(dst);
+    return GrXfermodeFragmentProcessor::CreateFromDstProcessor(dst, fMode);
 }
 
-bool SkProcCoeffXfermode::asXPFactory(GrXPFactory** xp) const {
+GrXPFactory* SkProcCoeffXfermode::asXPFactory() const {
     if (CANNOT_USE_COEFF != fSrcCoeff) {
-        if (xp) {
-            *xp = GrPorterDuffXPFactory::Create(fMode);
-            SkASSERT(*xp);
-        }
-        return true;
+        SkASSERT(fMode <= kLastCoeffMode);

[egdaniel, 2016/02/08 15:32:11]

I feel like this assert assumes knowledge of exactly what GrPorterDuffXPFactory
will or will not accept. Would it not be safer to do something like:

"return SkAssertResult(GrPDXPF::Create(fMode));"

[robertphillips, 2016/02/08 15:54:53]

Done.

+        return GrPorterDuffXPFactory::Create(fMode);
     }
 
-    if (GrCustomXfermode::IsSupportedMode(fMode)) {
-        if (xp) {
-            *xp = GrCustomXfermode::CreateXPFactory(fMode);
-            SkASSERT(*xp);
-        }
-        return true;
-    }
-    return false;
+    SkASSERT(GrCustomXfermode::IsSupportedMode(fMode));
+    return GrCustomXfermode::CreateXPFactory(fMode);
 }
 #endif
 
 const char* SkXfermode::ModeName(Mode mode) {
     SkASSERT((unsigned) mode <= (unsigned)kLastMode);
     const char* gModeStrings[] = {
         "Clear", "Src", "Dst", "SrcOver", "DstOver", "SrcIn", "DstIn",
         "SrcOut", "DstOut", "SrcATop", "DstATop", "Xor", "Plus",
         "Modulate", "Screen", "Overlay", "Darken", "Lighten", "ColorDodge",
         "ColorBurn", "HardLight", "SoftLight", "Difference", "Exclusion",
@@ skipping 121 matching lines @@
     if (!xfer) {
         return SkXfermode::kOpaque_SrcColorOpacity == opacityType;
     }
 
     return xfer->isOpaque(opacityType);
 }
 
 SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_START(SkXfermode)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkProcCoeffXfermode)
 SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END