diff options
author | Brian Osman <brianosman@google.com> | 2023-04-19 13:19:17 -0400 |
---|---|---|
committer | SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> | 2023-04-19 18:31:11 +0000 |
commit | cee2c5ed5fae2aecbbb39b5241bb9cb679953df1 (patch) | |
tree | 3edc2a7d6220c96249b5efcac52dbe60476c02f7 | |
parent | 0e1ff430811e6361c1cc5041b209dc3e09bbbbe0 (diff) | |
download | skia-cee2c5ed5fae2aecbbb39b5241bb9cb679953df1.tar.gz |
Always run color space math in full-float within Ganesh
With HDR in particular, some of the intermediate values can have
magnitudes that break down badly with half. This CL fixes some HDR bugs
on Android. I'm hoping that the performance impact is minimal, and it
lets us remove a bunch of complication around a caps bit.
NOTE: There is additional cleanup to happen, but I'm splitting this CL
in two parts, so we can cherry-pick this bit to Android U.
Bug: b/270744068
Change-Id: Ie16f4aa66e01c6d881fb522efd5fb8467887f826
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/674517
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Reck <jreck@google.com>
Reviewed-by: Nolan Scobie <nscobie@google.com>
Commit-Queue: Nolan Scobie <nscobie@google.com>
-rw-r--r-- | src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp | 47 |
1 files changed, 20 insertions, 27 deletions
diff --git a/src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp b/src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp index d3c4c654f0..601213b5dc 100644 --- a/src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp +++ b/src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp @@ -150,19 +150,19 @@ void GrGLSLShaderBuilder::appendColorGamutXform(SkString* out, auto emitTFFunc = [=](const char* name, GrGLSLProgramDataManager::UniformHandle uniform, skcms_TFType tfType) { - const GrShaderVar gTFArgs[] = { GrShaderVar("x", SkSLType::kHalf) }; + const GrShaderVar gTFArgs[] = { GrShaderVar("x", SkSLType::kFloat) }; const char* coeffs = uniformHandler->getUniformCStr(uniform); SkString body; // Temporaries to make evaluation line readable. We always use the sRGBish names, so the // PQ and HLG math is confusing. - body.appendf("half G = %s[0];", coeffs); - body.appendf("half A = %s[1];", coeffs); - body.appendf("half B = %s[2];", coeffs); - body.appendf("half C = %s[3];", coeffs); - body.appendf("half D = %s[4];", coeffs); - body.appendf("half E = %s[5];", coeffs); - body.appendf("half F = %s[6];", coeffs); - body.append("half s = sign(x);"); + body.appendf("float G = %s[0];", coeffs); + body.appendf("float A = %s[1];", coeffs); + body.appendf("float B = %s[2];", coeffs); + body.appendf("float C = %s[3];", coeffs); + body.appendf("float D = %s[4];", coeffs); + body.appendf("float E = %s[5];", coeffs); + body.appendf("float F = %s[6];", coeffs); + body.append("float s = sign(x);"); body.append("x = abs(x);"); switch (tfType) { case skcms_TFType_sRGBish: @@ -183,7 +183,7 @@ void GrGLSLShaderBuilder::appendColorGamutXform(SkString* out, } body.append("return s * x;"); SkString funcName = this->getMangledFunctionName(name); - this->emitFunction(SkSLType::kHalf, funcName.c_str(), {gTFArgs, std::size(gTFArgs)}, + this->emitFunction(SkSLType::kFloat, funcName.c_str(), {gTFArgs, std::size(gTFArgs)}, body.c_str()); return funcName; }; @@ -202,42 +202,35 @@ void GrGLSLShaderBuilder::appendColorGamutXform(SkString* out, SkString gamutXformFuncName; if (colorXformHelper->applyGamutXform()) { - const GrShaderVar gGamutXformArgs[] = { GrShaderVar("color", SkSLType::kHalf4) }; + const GrShaderVar gGamutXformArgs[] = { GrShaderVar("color", SkSLType::kFloat4) }; const char* xform = uniformHandler->getUniformCStr(colorXformHelper->gamutXformUniform()); SkString body; body.appendf("color.rgb = (%s * color.rgb);", xform); body.append("return color;"); gamutXformFuncName = this->getMangledFunctionName("gamut_xform"); - this->emitFunction(SkSLType::kHalf4, gamutXformFuncName.c_str(), + this->emitFunction(SkSLType::kFloat4, gamutXformFuncName.c_str(), {gGamutXformArgs, std::size(gGamutXformArgs)}, body.c_str()); } // Now define a wrapper function that applies all the intermediate steps { - // Some GPUs require full float to get results that are as accurate as expected/required. - // Most GPUs work just fine with half float. Strangely, the GPUs that have this bug - // (Mali G series) only require us to promote the type of a few temporaries here -- - // the helper functions above can always be written to use half. - bool useFloat = fProgramBuilder->shaderCaps()->fColorSpaceMathNeedsFloat; - - const GrShaderVar gColorXformArgs[] = { - GrShaderVar("color", useFloat ? SkSLType::kFloat4 : SkSLType::kHalf4)}; + const GrShaderVar gColorXformArgs[] = { GrShaderVar("color", SkSLType::kFloat4) }; SkString body; if (colorXformHelper->applyUnpremul()) { body.append("color = unpremul(color);"); } if (colorXformHelper->applySrcTF()) { - body.appendf("color.r = %s(half(color.r));", srcTFFuncName.c_str()); - body.appendf("color.g = %s(half(color.g));", srcTFFuncName.c_str()); - body.appendf("color.b = %s(half(color.b));", srcTFFuncName.c_str()); + body.appendf("color.r = %s(color.r);", srcTFFuncName.c_str()); + body.appendf("color.g = %s(color.g);", srcTFFuncName.c_str()); + body.appendf("color.b = %s(color.b);", srcTFFuncName.c_str()); } if (colorXformHelper->applyGamutXform()) { - body.appendf("color = %s(half4(color));", gamutXformFuncName.c_str()); + body.appendf("color = %s(color);", gamutXformFuncName.c_str()); } if (colorXformHelper->applyDstTF()) { - body.appendf("color.r = %s(half(color.r));", dstTFFuncName.c_str()); - body.appendf("color.g = %s(half(color.g));", dstTFFuncName.c_str()); - body.appendf("color.b = %s(half(color.b));", dstTFFuncName.c_str()); + body.appendf("color.r = %s(color.r);", dstTFFuncName.c_str()); + body.appendf("color.g = %s(color.g);", dstTFFuncName.c_str()); + body.appendf("color.b = %s(color.b);", dstTFFuncName.c_str()); } if (colorXformHelper->applyPremul()) { body.append("color.rgb *= color.a;"); |