AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix winding when splitting edges at out-of-bounds vertices

Browse Source 
Add GM repros and up triangulation verb count to match chromium's define
The GMs draw incorrectly in (base->p2) with the updated verb count, but
would draw fine w/o the increased verb count because a different path
renderer would be chosen.

This fixes a latent bug that was in the edge splitting code of the
triangulator that was exposed by https://skia-review.googlesource.com/c/skia/+/432196
Before that CL, intersections of two lines would be clamped to one of the
4 vertices of the 2 segments. In the CL linked, the clamping was adjusted
to clamp X and Y axes separately, so it increased the chance that a
clamped intersection would have its X or Y coord equal to line's vertex
but differ along the other coord (when they both equaled, they were
considered coincident and splitting that edge did nothing).

Splitting an edge at its intersection was intended to split (p0 to p1)
into new lines (p0 to v) and (v to p1) where p0 < v < p1 according to the
vertical or horizontal sorting that was imposed on the mesh. For a given
line segment and a clamped vertex, there are 8 ways the intersection
could be clamped (4 edges and 4 corners). If the edge has a positive
non-zero slope, a zero slope, or an infinite slope, in all cases the
clamped intersection will be sorted correctly and satisfy p0 < v < p1.

However, if the edge has negative slope
  vertical: p0.y<p1.y and p0.x>p1.x,
  horizontal: p0.x<p1.x and p0.y<p1.y
then intersections snapped to the primary sorting axis will be out of
order and produce a split such that v < p0 < p1 or p0 < p1 < v. This
was already detected, but it didn't update the winding of the new edge
to preserve the original winding from p0 to p1.

In these out-of-order cases, the intersection point is the top of both
the new and old edge, or the bottom of both the new and old edge. This
means that winding "top to bottom" on the new edge would go in the
opposite direction as the original winding from p0 to p1. Flipping the
winding on the new edge preserves the intended winding of the contour
while still allowing the edges/vertices to be sorted consistently.

This showed up as large gradients in the AA triangulator because w/o the
winding adjustment, the winding flip at the new edge would confuse the
border extractor that was used to compute insets and outsets for the 1px
coverage ramp. It would then use edges that were normally unrelated to
each and declare their line intersections as the "interior" with full
coverage. Obviously these could be anywhere so the 1px coverage ramp
would get smeared across that shape.

Bug: chromium:1257515
Change-Id: I015d6b4767db352e3eecfc53047958e74320268d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458057
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
Michael Ludwig 2021-10-11 16:41:32 -04:00 committed by SkCQ
parent 116d2e0e48
commit 6c8e2e832e
4 changed files with 99 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
gm/crbug_1257515.cpp Normal file
View File

@ -0,0 +1,83 @@
/*
* Copyright 2021 Google LLC
*
* 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/SkPathBuilder.h"
DEF_SIMPLE_GM(crbug_1257515, canvas, 1139, 400) {
// <svg width="1139" height="400" viewBox="0 0 1139 400">
// <g transform="translate(46,60) scale(1 1)">
// <path fill="none" d="M 45.125 102.53701800000002
// L 135.375 162.666156 L 225.625 116.622276
// L 315.875 121.52087700000001 L 406.125 134.632899
// L 496.375 192.317736 L 586.625 138.82944899999998
// L 676.875 234.212031 L 767.125 207.082926 L 857.375 128.083857
// L 947.625 127.95689999999999 L 1037.875 113.956785"
// stroke="red" stroke-width="2" stroke-linejoin="round" stroke-linecap="round">
// </path>
// </g>
// </svg>
SkPathBuilder b;
b.moveTo(45.125f, 102.53701800000002f)
.lineTo(135.375f, 162.666156f)
.lineTo(225.625f, 116.622276f)
.lineTo(315.875f, 121.52087700000001f)
.lineTo(406.125f, 134.632899f)
.lineTo(496.375f, 192.317736f)
.lineTo(586.625f, 138.82944899999998f)
.lineTo(676.875f, 234.212031f)
.lineTo(767.125f, 207.082926f)
.lineTo(857.375f, 128.083857f)
.lineTo(947.625f, 127.95689999999999f)
.lineTo(1037.875f, 113.956785f);
SkPaint p;
p.setColor(SK_ColorRED);
p.setStrokeWidth(2.f);
p.setStyle(SkPaint::kStroke_Style);
p.setStrokeCap(SkPaint::kRound_Cap);
p.setStrokeJoin(SkPaint::kRound_Join);
p.setAntiAlias(true);
canvas->save();
canvas->translate(-50.f, -200.f);
canvas->scale(2.f, 2.f);
canvas->drawPath(b.detach(), p);
canvas->restore();
// <svg width="1148" height="700" viewBox="0 0 1148 700">
// <path fill="none" d="M 129.5307 587.5728 L 232.4748 617.037 L 335.4189 624.8472
// L 438.3631 630.5933 L 541.3073 625.1138 L 644.2513 626.8717
// L 747.1955 629.9542 L 850.1396 629.6956 L 953.0838 616.4909
// L 1056.028 613.8181"
// stroke="rgba(47,136,255,1)" stroke-width="3"
// stroke-linecap="butt" stroke-linejoin="bevel" stroke-miterlimit="10">
// </path>
// </svg>
b.moveTo(128.5307f, 587.5728f)
.lineTo(232.4748f, 617.037f)
.lineTo(335.4189f, 624.8472f)
.lineTo(438.3631f, 630.5933f)
.lineTo(541.3073f, 625.1138f)
.lineTo(644.2513f, 626.8717f)
.lineTo(747.1955f, 629.9542f)
.lineTo(850.1396f, 629.6956f)
.lineTo(953.0838f, 616.4909f)
.lineTo(1056.028f, 613.8181f);
p.setColor(SkColorSetARGB(255, 47, 136, 255));
p.setStrokeWidth(3.f);
p.setStrokeCap(SkPaint::kButt_Cap);
p.setStrokeJoin(SkPaint::kBevel_Join);
p.setStrokeMiter(10.f);
canvas->save();
canvas->translate(-300.f, -900.f);
canvas->scale(2.f, 2.f);
canvas->drawPath(b.detach(), p);
canvas->restore();
}

1
gn/gm.gni
View File

@ -115,6 +115,7 @@ gm_sources = [
"$_gm/crbug_1174186.cpp",
"$_gm/crbug_1174354.cpp",
"$_gm/crbug_1177833.cpp",
"$_gm/crbug_1257515.cpp",
"$_gm/crbug_224618.cpp",
"$_gm/crbug_691386.cpp",
"$_gm/crbug_788500.cpp",

11
src/gpu/geometry/GrTriangulator.cpp
View File

@ -888,15 +888,26 @@ bool GrTriangulator::splitEdge(Edge* edge, Vertex* v, EdgeList* activeEdges, Ver
Vertex* top;
Vertex* bottom;
int winding = edge->fWinding;
// Theoretically, and ideally, the edge betwee p0 and p1 is being split by v, and v is "between"
// the segment end points according to c. This is equivalent to p0 < v < p1. Unfortunately, if
// v was clamped/rounded this relation doesn't always hold.
if (c.sweep_lt(v->fPoint, edge->fTop->fPoint)) {
// Actually "v < p0 < p1": update 'edge' to be v->p1 and add v->p0. We flip the winding on
// the new edge so that it winds as if it were p0->v.
top = v;
bottom = edge->fTop;
winding *= -1;
this->setTop(edge, v, activeEdges, current, c);
} else if (c.sweep_lt(edge->fBottom->fPoint, v->fPoint)) {
// Actually "p0 < p1 < v": update 'edge' to be p0->v and add p1->v. We flip the winding on
// the new edge so that it winds as if it were v->p1.
top = edge->fBottom;
bottom = v;
winding *= -1;
this->setBottom(edge, v, activeEdges, current, c);
} else {
// The ideal case, "p0 < v < p1": update 'edge' to be p0->v and add v->p1. Original winding
// is valid for both edges.
top = v;
bottom = edge->fBottom;
this->setBottom(edge, v, activeEdges, current, c);

4
src/gpu/ops/TriangulatingPathRenderer.cpp
View File

@ -33,7 +33,11 @@
#include <cstdio>
#ifndef GR_AA_TESSELLATOR_MAX_VERB_COUNT
#if defined(SK_BUILD_FOR_FUZZER)
#define GR_AA_TESSELLATOR_MAX_VERB_COUNT 10
#else
#define GR_AA_TESSELLATOR_MAX_VERB_COUNT 100
#endif
#endif
/*