Increase precision for two point gradient coordinates.
This can be seen in these failing gold images:
https://gold.skia.org/detail?test=gradients_local_perspective_nodither&digest=52ef69138d81fcb58f4439b2d5903f45
Zooming in on the 2nd from the bottom, rightmost gradient shows a
solid red line in the blue border region (due to overflow and then
being clamped to the wrong border). The 2nd rightmost gradient in
the same row also shows extra red at the bottom from similar overflow.
The former error appears now because the GPU pipeline didn't previously
render that gradient at all. The latter appears now because the refactor
for gradients changed where clamping of texture coordinates occurred.
Bug: skia:
Change-Id: If2a886eaae4e8817a205d9bb77cf092b97d27c84
Reviewed-on: https://skia-review.googlesource.com/155610
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/gpu/gradients/GrTwoPointConicalGradientLayout.cpp b/src/gpu/gradients/GrTwoPointConicalGradientLayout.cpp
index 291c867..68c28dc 100644
--- a/src/gpu/gradients/GrTwoPointConicalGradientLayout.cpp
+++ b/src/gpu/gradients/GrTwoPointConicalGradientLayout.cpp
@@ -43,25 +43,25 @@
kDefault_GrSLPrecision, "focalParams");
SkString sk_TransformedCoords2D_0 = fragBuilder->ensureCoords2D(args.fTransformedCoords[0]);
fragBuilder->codeAppendf(
- "half2 p = half2(%s);\nhalf t = -1.0;\nhalf v = 1.0;\n@switch (%d) {\n case "
- "1:\n {\n half r0_2 = %s.y;\n t = r0_2 - p.y * p.y;\n "
- " if (t >= 0.0) {\n t = float(p.x) + sqrt(float(t));\n "
- " } else {\n v = -1.0;\n }\n }\n "
- "break;\n case 0:\n {\n half r0 = %s.x;\n @if (%s) "
- "{\n t = length(p) - r0;\n } else {\n t = "
- "-length(p) - r0;\n ",
+ "float2 p = %s;\nfloat t = -1.0;\nhalf v = 1.0;\n@switch (%d) {\n case 1:\n "
+ " {\n half r0_2 = %s.y;\n t = float(float(r0_2) - p.y * "
+ "p.y);\n if (t >= 0.0) {\n t = p.x + sqrt(t);\n "
+ " } else {\n v = -1.0;\n }\n }\n break;\n "
+ " case 0:\n {\n half r0 = %s.x;\n @if (%s) {\n "
+ " t = length(p) - float(r0);\n } else {\n t = "
+ "-length(p) - float(r0);\n",
sk_TransformedCoords2D_0.c_str(), (int)_outer.type(),
args.fUniformHandler->getUniformCStr(fFocalParamsVar),
args.fUniformHandler->getUniformCStr(fFocalParamsVar),
(_outer.isRadiusIncreasing() ? "true" : "false"));
fragBuilder->codeAppendf(
- " }\n }\n break;\n case 2:\n {\n half invR1 = "
- "%s.x;\n half fx = %s.y;\n half x_t = -1.0;\n @if "
- "(%s) {\n x_t = dot(p, p) / p.x;\n } else if (%s) {\n "
- " x_t = length(p) - p.x * invR1;\n } else {\n "
- "half temp = p.x * p.x - p.y * p.y;\n if (temp >= 0.0) {\n "
- " @if (%s || !%s) {\n x_t = "
- "half(-sqrt(float(temp)) - float(p.x * invR1",
+ " }\n }\n break;\n case 2:\n {\n "
+ "half invR1 = %s.x;\n half fx = %s.y;\n float x_t = -1.0;\n "
+ " @if (%s) {\n x_t = dot(p, p) / p.x;\n } else "
+ "if (%s) {\n x_t = length(p) - p.x * float(invR1);\n } "
+ "else {\n float temp = p.x * p.x - p.y * p.y;\n if "
+ "(temp >= 0.0) {\n @if (%s || !%s) {\n "
+ "x_t = -sqrt(temp) - p.x * float",
args.fUniformHandler->getUniformCStr(fFocalParamsVar),
args.fUniformHandler->getUniformCStr(fFocalParamsVar),
(_outer.isFocalOnCircle() ? "true" : "false"),
@@ -69,22 +69,22 @@
(_outer.isSwapped() ? "true" : "false"),
(_outer.isRadiusIncreasing() ? "true" : "false"));
fragBuilder->codeAppendf(
- "));\n } else {\n x_t = "
- "half(sqrt(float(temp)) - float(p.x * invR1));\n }\n "
- " }\n }\n @if (!%s) {\n if (float(x_t) <= "
- "0.0) {\n v = -1.0;\n }\n }\n "
- " @if (%s) {\n @if (%s) {\n t = x_t;\n "
- " } else {\n t = x_t + fx;\n }\n "
- " } else {\n @if (%s) {",
+ "(invR1);\n } else {\n x_t = sqrt(temp) "
+ "- p.x * float(invR1);\n }\n }\n }\n "
+ " @if (!%s) {\n if (x_t <= 0.0) {\n v = "
+ "-1.0;\n }\n }\n @if (%s) {\n "
+ "@if (%s) {\n t = x_t;\n } else {\n "
+ " t = x_t + float(fx);\n }\n } else {\n "
+ " @if (%s) {\n ",
(_outer.isWellBehaved() ? "true" : "false"),
(_outer.isRadiusIncreasing() ? "true" : "false"),
(_outer.isNativelyFocal() ? "true" : "false"),
(_outer.isNativelyFocal() ? "true" : "false"));
fragBuilder->codeAppendf(
- "\n t = -x_t;\n } else {\n t "
- "= -x_t + fx;\n }\n }\n @if (%s) {\n "
- " t = 1.0 - t;\n }\n }\n break;\n}\n%s = half4(t, v, "
- "0.0, 0.0);\n",
+ " t = -x_t;\n } else {\n t = -x_t + "
+ "float(fx);\n }\n }\n @if (%s) {\n "
+ " t = 1.0 - t;\n }\n }\n break;\n}\n%s = "
+ "half4(half(t), v, 0.0, 0.0);\n",
(_outer.isSwapped() ? "true" : "false"), args.fOutputColor);
}
diff --git a/src/gpu/gradients/GrTwoPointConicalGradientLayout.fp b/src/gpu/gradients/GrTwoPointConicalGradientLayout.fp
index f7d9860..73fac71 100644
--- a/src/gpu/gradients/GrTwoPointConicalGradientLayout.fp
+++ b/src/gpu/gradients/GrTwoPointConicalGradientLayout.fp
@@ -32,8 +32,14 @@
}
void main() {
- half2 p = sk_TransformedCoords2D[0];
- half t = -1;
+ // p typed as a float2 is intentional; while a half2 is adequate for most normal cases in the
+ // two point conic gradient's coordinate system, when the gradient is composed with a local
+ // perspective matrix, certain out-of-bounds regions become ill behaved on mobile devices.
+ // On desktops, they are properly clamped after the fact, but on many Adreno GPUs the
+ // calculations of t and x_t below overflow and produce an incorrect interpolant (which then
+ // renders the wrong border color sporadically). Increasing precition alleviates that issue.
+ float2 p = sk_TransformedCoords2D[0];
+ float t = -1;
half v = 1; // validation flag, set to negative to discard fragment later
@switch(type) {
@@ -60,13 +66,13 @@
half invR1 = focalParams.x;
half fx = focalParams.y;
- half x_t = -1;
+ float x_t = -1;
@if (isFocalOnCircle) {
x_t = dot(p, p) / p.x;
} else if (isWellBehaved) {
x_t = length(p) - p.x * invR1;
} else {
- half temp = p.x * p.x - p.y * p.y;
+ float temp = p.x * p.x - p.y * p.y;
// Only do sqrt if temp >= 0; this is significantly slower than checking temp >= 0
// in the if statement that checks r(t) >= 0. But GPU may break if we sqrt a
