Remove inout params from saturation helper func in XferEffect.

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"
@@ skipping 1269 matching lines @@
             SkString getSatBody;
             getSatBody.printf("\treturn max(max(color.r, color.g), color.b) - "
                               "min(min(color.r, color.g), color.b);\n");
             builder->emitFunction(GrGLShaderBuilder::kFragment_ShaderType,
                                   kFloat_GrSLType,
                                   "saturation",
                                   SK_ARRAY_COUNT(getSatArgs), getSatArgs,
                                   getSatBody.c_str(),
                                   &getFunction);
 
-            // Emit a helper that sets the saturation given sorted input channels
+            // Emit a helper that sets the saturation given sorted input channels. This used
+            // to use inout params for min, mid, and max components but that seems to cause
+            // problems on PowerVR drivers. So instead it returns a vec3 where r, g ,b are the
+            // adjusted min, mid, and max inputs, respectively.
             SkString helperFunction;
             GrGLShaderVar helperArgs[] = {
                 GrGLShaderVar("minComp", kFloat_GrSLType),
                 GrGLShaderVar("midComp", kFloat_GrSLType),
                 GrGLShaderVar("maxComp", kFloat_GrSLType),
                 GrGLShaderVar("sat", kFloat_GrSLType),
             };
-            helperArgs[0].setTypeModifier(GrGLShaderVar::kInOut_TypeModifier);
-            helperArgs[1].setTypeModifier(GrGLShaderVar::kInOut_TypeModifier);
-            helperArgs[2].setTypeModifier(GrGLShaderVar::kInOut_TypeModifier);
             SkString helperBody;
-            helperBody.append("\tif (minComp < maxComp) {\n"
-                              "\t\tmidComp = sat * (midComp - minComp) / (maxComp - minComp);\n"
-                              "\t\tmaxComp = sat;\n"
+            helperBody.printf("\tif (minComp < maxComp) {\n"
+                              "\t\tvec3 result;\n"
+                              "\t\tresult.r = 0.0;\n"
+                              "\t\tresult.g = sat * (midComp - minComp) / (maxComp - minComp);\n"
+                              "\t\tresult.b = sat;\n"
+                              "\t\treturn result;\n"
                               "\t} else {\n"
-                              "\t\tmidComp = maxComp = 0.0;\n"
-                              "\t}\n"
-                              "\tminComp = 0.0;\n");
+                              "\t\treturn %s;\n"
+                              "\t}\n",

[robertphillips, 2013/04/29 17:04:02]

Why not just put this in the string?

[bsalomon, 2013/04/29 17:07:02]

Laziness I suppose. I can never remember whether the .0s are required here.
(just checked now and they're not).

[bsalomon, 2013/04/29 17:12:22]

Done.

+                              GrGLSLZerosVecf(3));
             builder->emitFunction(GrGLShaderBuilder::kFragment_ShaderType,
-                                  kVoid_GrSLType,
+                                  kVec3f_GrSLType,
                                   "set_saturation_helper",
                                   SK_ARRAY_COUNT(helperArgs), helperArgs,
                                   helperBody.c_str(),
                                   &helperFunction);
 
             GrGLShaderVar setSatArgs[] = {
                 GrGLShaderVar("hueLumColor", kVec3f_GrSLType),
                 GrGLShaderVar("satColor", kVec3f_GrSLType),
             };
             const char* helpFunc = helperFunction.c_str();
             SkString setSatBody;
             setSatBody.appendf("\tfloat sat = %s(satColor);\n"
                                "\tif (hueLumColor.r <= hueLumColor.g) {\n"
                                "\t\tif (hueLumColor.g <= hueLumColor.b) {\n"
-                               "\t\t\t%s(hueLumColor.r, hueLumColor.g, hueLumColor.b, sat);\n"
+                               "\t\t\thueLumColor.rgb = %s(hueLumColor.r, hueLumColor.g, hueLumColor.b, sat);\n"
                                "\t\t} else if (hueLumColor.r <= hueLumColor.b) {\n"
-                               "\t\t\t%s(hueLumColor.r, hueLumColor.b, hueLumColor.g, sat);\n"
+                               "\t\t\thueLumColor.rbg = %s(hueLumColor.r, hueLumColor.b, hueLumColor.g, sat);\n"
                                "\t\t} else {\n"
-                               "\t\t\t%s(hueLumColor.b, hueLumColor.r, hueLumColor.g, sat);\n"
+                               "\t\t\thueLumColor.brg = %s(hueLumColor.b, hueLumColor.r, hueLumColor.g, sat);\n"
                                "\t\t}\n"
                                "\t} else if (hueLumColor.r <= hueLumColor.b) {\n"
-                               "\t\t%s(hueLumColor.g, hueLumColor.r, hueLumColor.b, sat);\n"
+                               "\t\thueLumColor.grb = %s(hueLumColor.g, hueLumColor.r, hueLumColor.b, sat);\n"
                                "\t} else if (hueLumColor.g <= hueLumColor.b) {\n"
-                               "\t\t%s(hueLumColor.g, hueLumColor.b, hueLumColor.r, sat);\n"
+                               "\t\thueLumColor.gbr = %s(hueLumColor.g, hueLumColor.b, hueLumColor.r, sat);\n"
                                "\t} else {\n"
-                               "\t\t%s(hueLumColor.b, hueLumColor.g, hueLumColor.r, sat);\n"
+                               "\t\thueLumColor.bgr = %s(hueLumColor.b, hueLumColor.g, hueLumColor.r, sat);\n"
                                "\t}\n"
-                               "\treturn hueLumColor;",
+                               "\treturn hueLumColor;\n",
                                getFunction.c_str(), helpFunc, helpFunc, helpFunc, helpFunc,
                                helpFunc, helpFunc);
             builder->emitFunction(GrGLShaderBuilder::kFragment_ShaderType,
                                   kVec3f_GrSLType,
                                   "set_saturation",
                                   SK_ARRAY_COUNT(setSatArgs), setSatArgs,
                                   setSatBody.c_str(),
                                   setSatFunction);
 
         }
@@ skipping 619 matching lines @@
     return proc16;
 }
 
 SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_START(SkXfermode)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkProcCoeffXfermode)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkClearXfermode)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkSrcXfermode)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkDstInXfermode)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkDstOutXfermode)
 SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END