Don't use AA convex path renderer if direction can't be determined
Without the changes to GrShape and GrAAConvexPathRenderer, this new GM
would not draw the red circle at the center when using Ganesh. Raster
drew the red circle. The difference came about because they checked
convexity after transforming by the view matrix, and Ganesh checked
before.
With this particular path construction and the very large scale factor
on the CTM, the convexity calculator in local space computed small
enough edge vectors that it thought the contour backtracked on itself
instead of winding in a consistent direction.
The GrAAConvexPathRenderer would claim it could draw the convex path,
but later when actually preparing to draw, it would do nothing if it
turned out the convex path didn't have a direction. This CL updates it
so that the convex path renderer's canDraw() function matches what it
is actually able to draw.
There is the separate issue of the convexity/direction checking
precision.
Bug: chromium:996140
Change-Id: I6f2af2d9ba6752663d97c8573deb2d767a3f1245
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/238121
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/gm/crbug_996140.cpp b/gm/crbug_996140.cpp
new file mode 100644
index 0000000..d2d2b6d
--- /dev/null
+++ b/gm/crbug_996140.cpp
@@ -0,0 +1,79 @@
+/*
+ * Copyright 2019 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "gm/gm.h"
+#include "include/core/SkCanvas.h"
+#include "include/core/SkColor.h"
+#include "include/core/SkPaint.h"
+#include "include/core/SkPath.h"
+
+/*
+ * Canvas example. Expected large blue stroked circle, white middle, small red circle.
+ * GPU-accelerated canvas produces large blue stroked circle, white middle, NO red circle.
+ *
+ * 1: var c = document.getElementById("myCanvas");
+ * 2: var ctx = c.getContext("2d");
+ * 3: ctx.beginPath();
+ * 4: ctx.scale(203.20, 203.20);
+ * 5: ctx.translate(-14.55, -711.51);
+ * 6: ctx.fillStyle = "red";
+ * 7: ctx.strokeStyle = "blue";
+ * 8: //ctx.lineWidth = 1/203.20;
+ * 9: ctx.arc(19.221, 720-6.76,0.0295275590551181,0,2*Math.PI);
+ * 10: ctx.stroke();
+ * 11: ctx.fill();
+ * 12: ctx.closePath();
+*/
+DEF_SIMPLE_GM_BG(crbug_996140, canvas, 300, 300, SK_ColorWHITE) {
+ // Specific parameters taken from the canvas minimum working example
+ SkScalar cx = 19.221f;
+ SkScalar cy = 720-6.76f;
+ SkScalar radius = 0.0295275590551181f;
+
+ SkScalar s = 203.20f;
+ SkScalar tx = -14.55f;
+ SkScalar ty = -711.51f;
+
+ // 0: The test canvas was 1920x574 and the circle was located in the bottom left, but that's
+ // not necessary to reproduce the problem, so translate to make a smaller GM.
+ canvas->translate(-800, -200);
+
+ // 3: ctx.beginPath();
+ SkPath path;
+
+ // 4: ctx.scale(203.20, 203.20);
+ canvas->scale(s, s);
+ // 5: ctx.translate(-14.55, -711.51);
+ canvas->translate(tx, ty);
+
+ // 6: ctx.fillStyle = "red";
+ SkPaint fill;
+ fill.setColor(SK_ColorRED);
+ fill.setStyle(SkPaint::kFill_Style);
+ fill.setAntiAlias(true);
+
+ // 7: ctx.strokeStyle = "blue";
+ SkPaint stroke;
+ stroke.setColor(SK_ColorBLUE);
+ stroke.setStrokeWidth(1.f);
+ stroke.setStyle(SkPaint::kStroke_Style);
+ stroke.setAntiAlias(true);
+
+ // 9: ctx.arc(19.221, 720-6.76,0.0295275590551181,0,2*Math.PI);
+ // This matches how Canvas prepares an arc(x, y, radius, 0, 2pi) call
+ SkRect boundingBox = SkRect::MakeLTRB(cx - radius, cy - radius, cx + radius, cy + radius);
+ path.arcTo(boundingBox, 0, 180.f, false);
+ path.arcTo(boundingBox, 180.f, 180.f, false);
+
+ // 12: ctx.closePath();
+ // path.close();
+
+ // 10: ctx.stroke(); (NOT NEEDED TO REPRODUCE FAILING RED CIRCLE)
+ canvas->drawPath(path, stroke);
+ // 11: ctx.fill()
+ canvas->drawPath(path, fill);
+}
diff --git a/gn/gm.gni b/gn/gm.gni
index 8df253b..9909947 100644
--- a/gn/gm.gni
+++ b/gn/gm.gni
@@ -115,6 +115,7 @@
"$_gm/crbug_938592.cpp",
"$_gm/crbug_946965.cpp",
"$_gm/crbug_947055.cpp",
+ "$_gm/crbug_996140.cpp",
"$_gm/croppedrects.cpp",
"$_gm/crosscontextimage.cpp",
"$_gm/cubicpaths.cpp",
diff --git a/src/gpu/ccpr/GrOctoBounds.cpp b/src/gpu/ccpr/GrOctoBounds.cpp
index a510fd3..72aa6f0 100644
--- a/src/gpu/ccpr/GrOctoBounds.cpp
+++ b/src/gpu/ccpr/GrOctoBounds.cpp
@@ -123,7 +123,11 @@
void GrOctoBounds::validateBoundsAreTight(const std::function<void(
bool cond, const char* file, int line, const char* code)>& validateFn) const {
- constexpr static float epsilon = 1e-3f;
+ // The octobounds calculated in GrCCPerFlushResources::renderShapeInAtlas use FMAs to compute
+ // M * (x,y) and T45 * M * (x,y) in parallel. This leads to a not-insignificant floating point
+ // difference between (T45 * M * (x,y)) stored in fBounds45, and T45 * (M * (x,y)) calculated
+ // here from fBounds with the Get_xy45 functions.
+ constexpr static float epsilon = 1e-2f;
float l=fBounds.left(), l45=fBounds45.left();
float t=fBounds.top(), t45=fBounds45.top();
diff --git a/src/gpu/geometry/GrShape.h b/src/gpu/geometry/GrShape.h
index f7627e6..09af83f 100644
--- a/src/gpu/geometry/GrShape.h
+++ b/src/gpu/geometry/GrShape.h
@@ -336,6 +336,32 @@
return false;
}
+ /**
+ * Does the shape have a known winding direction. Some degenerate convex shapes may not have
+ * a computable direction, but this is not always a requirement for path renderers so it is
+ * kept separate from knownToBeConvex().
+ */
+ bool knownDirection() const {
+ switch (fType) {
+ case Type::kEmpty:
+ return true;
+ case Type::kInvertedEmpty:
+ return true;
+ case Type::kRRect:
+ return true;
+ case Type::kArc:
+ return true;
+ case Type::kLine:
+ return true;
+ case Type::kPath:
+ // Assuming this is called after knownToBeConvex(), this should just be relying on
+ // cached convexity and direction and will be cheap.
+ return !SkPathPriv::CheapIsFirstDirection(this->path(),
+ SkPathPriv::kUnknown_FirstDirection);
+ }
+ return false;
+ }
+
/** Is the pre-styled geometry inverse filled? */
bool inverseFilled() const {
bool ret = false;
diff --git a/src/gpu/ops/GrAAConvexPathRenderer.cpp b/src/gpu/ops/GrAAConvexPathRenderer.cpp
index 5021aa7..4aa860f 100644
--- a/src/gpu/ops/GrAAConvexPathRenderer.cpp
+++ b/src/gpu/ops/GrAAConvexPathRenderer.cpp
@@ -219,19 +219,21 @@
}
}
-static inline bool get_direction(const SkPath& path, const SkMatrix& m,
- SkPathPriv::FirstDirection* dir) {
- if (!SkPathPriv::CheapComputeFirstDirection(path, dir)) {
- return false;
- }
+static inline SkPathPriv::FirstDirection get_direction(const SkPath& path, const SkMatrix& m) {
+ // At this point, we've already returned true from canDraw(), which checked that the path's
+ // direction could be determined, so this should just be fetching the cached direction.
+ SkPathPriv::FirstDirection dir;
+ SkAssertResult(SkPathPriv::CheapComputeFirstDirection(path, &dir));
+
// check whether m reverses the orientation
SkASSERT(!m.hasPerspective());
SkScalar det2x2 = m.get(SkMatrix::kMScaleX) * m.get(SkMatrix::kMScaleY) -
m.get(SkMatrix::kMSkewX) * m.get(SkMatrix::kMSkewY);
if (det2x2 < 0) {
- *dir = SkPathPriv::OppositeFirstDirection(*dir);
+ dir = SkPathPriv::OppositeFirstDirection(dir);
}
- return true;
+
+ return dir;
}
static inline void add_line_to_segment(const SkPoint& pt,
@@ -281,11 +283,7 @@
// line paths. We detect paths that are very close to a line (zero area) and
// draw nothing.
DegenerateTestData degenerateData;
- SkPathPriv::FirstDirection dir;
- // get_direction can fail for some degenerate paths.
- if (!get_direction(path, m, &dir)) {
- return false;
- }
+ SkPathPriv::FirstDirection dir = get_direction(path, m);
for (;;) {
SkPoint pts[4];
@@ -661,9 +659,12 @@
GrPathRenderer::CanDrawPath
GrAAConvexPathRenderer::onCanDrawPath(const CanDrawPathArgs& args) const {
+ // This check requires convexity and known direction, since the direction is used to build
+ // the geometry segments. Degenerate convex paths will fall through to some other path renderer.
if (args.fCaps->shaderCaps()->shaderDerivativeSupport() &&
(GrAAType::kCoverage == args.fAAType) && args.fShape->style().isSimpleFill() &&
- !args.fShape->inverseFilled() && args.fShape->knownToBeConvex()) {
+ !args.fShape->inverseFilled() && args.fShape->knownToBeConvex() &&
+ args.fShape->knownDirection()) {
return CanDrawPath::kYes;
}
return CanDrawPath::kNo;
