Fix GrAAConvexTessellator colinear point removal.

We weren't checking whether the new point in lineTo was backtracking or not.

Also:
Replace distance-to-line check with triangle area check. The previous check
was asymmetric. Given point sequence (a, b, c) it might make a different
decision than when given (c, b, a).

Compute normals late since we don't use them to detect colinear edges
anymore.

Rename SkPointPriv::SetOrhog -> SkPointPriv::MakeOrthog and return
computed value rather than take SkPoint* dst.

Bug: chromium:869172


Change-Id: I8da53edf1a2e6098f4199da57368ebb644866e4c
Reviewed-on: https://skia-review.googlesource.com/150682
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This commit is contained in:
Brian Salomon 2018-08-31 12:04:18 -04:00 committed by Skia Commit-Bot
parent d709a85751
commit 0235c648b7
7 changed files with 90 additions and 93 deletions

View File

@ -317,9 +317,8 @@ protected:
this->drawPath(canvas, i, &offset); this->drawPath(canvas, i, &offset);
} }
// Repro for crbug.com/472723 (Missing AA on portions of graphic with GPU rasterization)
{ {
canvas->translate(356.0f, 50.0f); // Repro for crbug.com/472723 (Missing AA on portions of graphic with GPU rasterization)
SkPaint p; SkPaint p;
p.setAntiAlias(true); p.setAntiAlias(true);
@ -334,7 +333,31 @@ protected:
p1.lineTo(59.4380493f, 364.671021f); p1.lineTo(59.4380493f, 364.671021f);
p1.lineTo(385.414276f, 690.647217f); p1.lineTo(385.414276f, 690.647217f);
p1.lineTo(386.121399f, 689.940125f); p1.lineTo(386.121399f, 689.940125f);
canvas->save();
canvas->translate(356.0f, 50.0f);
canvas->drawPath(p1, p); canvas->drawPath(p1, p);
canvas->restore();
// Repro for crbug.com/869172 (SVG path incorrectly simplified when using GPU
// Rasterization). This will only draw anything in the stroke-and-fill version.
SkPath p2;
p2.moveTo(10.f, 0.f);
p2.lineTo(38.f, 0.f);
p2.lineTo(66.f, 0.f);
p2.lineTo(94.f, 0.f);
p2.lineTo(122.f, 0.f);
p2.lineTo(150.f, 0.f);
p2.lineTo(150.f, 0.f);
p2.lineTo(122.f, 0.f);
p2.lineTo(94.f, 0.f);
p2.lineTo(66.f, 0.f);
p2.lineTo(38.f, 0.f);
p2.lineTo(10.f, 0.f);
p2.close();
canvas->save();
canvas->translate(0.0f, 500.0f);
canvas->drawPath(p2, p);
canvas->restore();
} }
} }

View File

@ -97,17 +97,9 @@ public:
static bool SetLengthFast(SkPoint* pt, float length); static bool SetLengthFast(SkPoint* pt, float length);
static void SetOrthog(SkPoint* pt, const SkPoint& vec, Side side = kLeft_Side) { static SkPoint MakeOrthog(const SkPoint& vec, Side side = kLeft_Side) {
// vec could be this SkASSERT(side == kRight_Side || side == kLeft_Side);
SkScalar tmp = vec.fX; return (side == kRight_Side) ? SkPoint{-vec.fY, vec.fX} : SkPoint{vec.fY, -vec.fX};
if (kRight_Side == side) {
pt->fX = -vec.fY;
pt->fY = tmp;
} else {
SkASSERT(kLeft_Side == side);
pt->fX = vec.fY;
pt->fY = -tmp;
}
} }
// counter-clockwise fan // counter-clockwise fan

View File

@ -253,7 +253,7 @@ void GrPathUtils::QuadUVMatrix::set(const SkPoint qPts[3]) {
// when looking from the point 0 down the line we want positive // when looking from the point 0 down the line we want positive
// distances to be to the left. This matches the non-degenerate // distances to be to the left. This matches the non-degenerate
// case. // case.
SkPointPriv::SetOrthog(&lineVec, lineVec, SkPointPriv::kLeft_Side); lineVec = SkPointPriv::MakeOrthog(lineVec, SkPointPriv::kLeft_Side);
// first row // first row
fM[0] = 0; fM[0] = 0;
fM[1] = 0; fM[1] = 0;
@ -498,9 +498,9 @@ void convert_noninflect_cubic_to_quads(const SkPoint p[4],
if (constrainWithinTangents && if (constrainWithinTangents &&
!is_point_within_cubic_tangents(p[0], ab, dc, p[3], dir, cAvg)) { !is_point_within_cubic_tangents(p[0], ab, dc, p[3], dir, cAvg)) {
// choose a new cAvg that is the intersection of the two tangent lines. // choose a new cAvg that is the intersection of the two tangent lines.
SkPointPriv::SetOrthog(&ab, ab); ab = SkPointPriv::MakeOrthog(ab);
SkScalar z0 = -ab.dot(p[0]); SkScalar z0 = -ab.dot(p[0]);
SkPointPriv::SetOrthog(&dc, dc); dc = SkPointPriv::MakeOrthog(dc);
SkScalar z1 = -dc.dot(p[3]); SkScalar z1 = -dc.dot(p[3]);
cAvg.fX = ab.fY * z1 - z0 * dc.fY; cAvg.fX = ab.fY * z1 - z0 * dc.fY;
cAvg.fY = z0 * dc.fX - ab.fX * z1; cAvg.fY = z0 * dc.fX - ab.fX * z1;

View File

@ -147,7 +147,7 @@ static bool compute_vectors(SegmentArray* segments,
for (int p = 0; p < n; ++p) { for (int p = 0; p < n; ++p) {
segb.fNorms[p] = segb.fPts[p] - *prevPt; segb.fNorms[p] = segb.fPts[p] - *prevPt;
segb.fNorms[p].normalize(); segb.fNorms[p].normalize();
SkPointPriv::SetOrthog(&segb.fNorms[p], segb.fNorms[p], normSide); segb.fNorms[p] = SkPointPriv::MakeOrthog(segb.fNorms[p], normSide);
prevPt = &segb.fPts[p]; prevPt = &segb.fPts[p];
} }
if (Segment::kLine == segb.fType) { if (Segment::kLine == segb.fType) {
@ -206,7 +206,7 @@ static void update_degenerate_test(DegenerateTestData* data, const SkPoint& pt)
if (SkPointPriv::DistanceToSqd(pt, data->fFirstPoint) > kCloseSqd) { if (SkPointPriv::DistanceToSqd(pt, data->fFirstPoint) > kCloseSqd) {
data->fLineNormal = pt - data->fFirstPoint; data->fLineNormal = pt - data->fFirstPoint;
data->fLineNormal.normalize(); data->fLineNormal.normalize();
SkPointPriv::SetOrthog(&data->fLineNormal, data->fLineNormal); data->fLineNormal = SkPointPriv::MakeOrthog(data->fLineNormal);
data->fLineC = -data->fLineNormal.dot(data->fFirstPoint); data->fLineC = -data->fLineNormal.dot(data->fFirstPoint);
data->fStage = DegenerateTestData::kLine; data->fStage = DegenerateTestData::kLine;
} }

View File

@ -59,10 +59,14 @@ static bool duplicate_pt(const SkPoint& p0, const SkPoint& p1) {
return distSq < kCloseSqd; return distSq < kCloseSqd;
} }
static SkScalar abs_dist_from_line(const SkPoint& p0, const SkVector& v, const SkPoint& test) { static bool points_are_colinear_and_b_is_middle(const SkPoint& a, const SkPoint& b,
SkPoint testV = test - p0; const SkPoint& c) {
SkScalar dist = testV.fX * v.fY - testV.fY * v.fX; // 'area' is twice the area of the triangle with corners a, b, and c.
return SkScalarAbs(dist); SkScalar area = a.fX * (b.fY - c.fY) + b.fX * (c.fY - a.fY) + c.fX * (a.fY - b.fY);
if (SkScalarAbs(area) >= 2 * kCloseSqd) {
return false;
}
return (a - b).dot(b - c) >= 0;
} }
int GrAAConvexTessellator::addPt(const SkPoint& pt, int GrAAConvexTessellator::addPt(const SkPoint& pt,
@ -142,6 +146,27 @@ void GrAAConvexTessellator::rewind() {
#endif #endif
} }
void GrAAConvexTessellator::computeNormals() {
auto normalToVector = [this](SkVector v) {
SkVector n = SkPointPriv::MakeOrthog(v, fSide);
SkAssertResult(n.normalize());
SkASSERT(SkScalarNearlyEqual(1.0f, n.length()));
return n;
};
// Check the cross product of the final trio
fNorms.append(fPts.count());
fNorms[0] = fPts[1] - fPts[0];
fNorms.top() = fPts[0] - fPts.top();
SkScalar cross = SkPoint::CrossProduct(fNorms[0], fNorms.top());
fSide = (cross > 0.0f) ? SkPointPriv::kRight_Side : SkPointPriv::kLeft_Side;
fNorms[0] = normalToVector(fNorms[0]);
for (int cur = 1; cur < fNorms.count() - 1; ++cur) {
fNorms[cur] = normalToVector(fPts[cur + 1] - fPts[cur]);
}
fNorms.top() = normalToVector(fNorms.top());
}
void GrAAConvexTessellator::computeBisectors() { void GrAAConvexTessellator::computeBisectors() {
fBisectors.setCount(fNorms.count()); fBisectors.setCount(fNorms.count());
@ -149,11 +174,8 @@ void GrAAConvexTessellator::computeBisectors() {
for (int cur = 0; cur < fBisectors.count(); prev = cur, ++cur) { for (int cur = 0; cur < fBisectors.count(); prev = cur, ++cur) {
fBisectors[cur] = fNorms[cur] + fNorms[prev]; fBisectors[cur] = fNorms[cur] + fNorms[prev];
if (!fBisectors[cur].normalize()) { if (!fBisectors[cur].normalize()) {
SkASSERT(SkPointPriv::kLeft_Side == fSide || SkPointPriv::kRight_Side == fSide); fBisectors[cur] = SkPointPriv::MakeOrthog(fNorms[cur], (SkPointPriv::Side)-fSide) +
SkPointPriv::SetOrthog(&fBisectors[cur], fNorms[cur], (SkPointPriv::Side)-fSide); SkPointPriv::MakeOrthog(fNorms[prev], fSide);
SkVector other;
SkPointPriv::SetOrthog(&other, fNorms[prev], fSide);
fBisectors[cur] += other;
SkAssertResult(fBisectors[cur].normalize()); SkAssertResult(fBisectors[cur].normalize());
} else { } else {
fBisectors[cur].negate(); // make the bisector face in fBisectors[cur].negate(); // make the bisector face in
@ -357,8 +379,6 @@ bool GrAAConvexTessellator::extractFromPath(const SkMatrix& m, const SkPath& pat
// Presumptive inner ring: 6*numPts + 6 // Presumptive inner ring: 6*numPts + 6
fIndices.setReserve(18*path.countPoints() + 6); fIndices.setReserve(18*path.countPoints() + 6);
fNorms.setReserve(path.countPoints());
// TODO: is there a faster way to extract the points from the path? Perhaps // TODO: is there a faster way to extract the points from the path? Perhaps
// get all the points via a new entry point, transform them all in bulk // get all the points via a new entry point, transform them all in bulk
// and then walk them to find duplicates? // and then walk them to find duplicates?
@ -393,48 +413,24 @@ bool GrAAConvexTessellator::extractFromPath(const SkMatrix& m, const SkPath& pat
// check if last point is a duplicate of the first point. If so, remove it. // check if last point is a duplicate of the first point. If so, remove it.
if (duplicate_pt(fPts[this->numPts()-1], fPts[0])) { if (duplicate_pt(fPts[this->numPts()-1], fPts[0])) {
this->popLastPt(); this->popLastPt();
fNorms.pop();
} }
SkASSERT(fPts.count() == fNorms.count()+1); // Remove any lingering colinear points where the path wraps around
if (this->numPts() >= 3) { bool noRemovalsToDo = false;
if (abs_dist_from_line(fPts.top(), fNorms.top(), fPts[0]) < kClose) { while (!noRemovalsToDo && this->numPts() >= 3) {
// The last point is on the line from the second to last to the first point. if (points_are_colinear_and_b_is_middle(fPts[fPts.count() - 2], fPts.top(), fPts[0])) {
this->popLastPt(); this->popLastPt();
fNorms.pop(); } else if (points_are_colinear_and_b_is_middle(fPts.top(), fPts[0], fPts[1])) {
} this->popFirstPtShuffle();
*fNorms.push() = fPts[0] - fPts.top();
SkDEBUGCODE(SkScalar len =) SkPoint::Normalize(&fNorms.top());
SkASSERT(len > 0.0f);
SkASSERT(fPts.count() == fNorms.count());
}
if (this->numPts() >= 3 && abs_dist_from_line(fPts[0], fNorms.top(), fPts[1]) < kClose) {
// The first point is on the line from the last to the second.
this->popFirstPtShuffle();
fNorms.removeShuffle(0);
fNorms[0] = fPts[1] - fPts[0];
SkDEBUGCODE(SkScalar len =) SkPoint::Normalize(&fNorms[0]);
SkASSERT(len > 0.0f);
SkASSERT(SkScalarNearlyEqual(1.0f, fNorms[0].length()));
}
if (this->numPts() >= 3) {
// Check the cross product of the final trio
SkScalar cross = SkPoint::CrossProduct(fNorms[0], fNorms.top());
if (cross > 0.0f) {
fSide = SkPointPriv::kRight_Side;
} else { } else {
fSide = SkPointPriv::kLeft_Side; noRemovalsToDo = true;
}
// Make all the normals face outwards rather than along the edge
for (int cur = 0; cur < fNorms.count(); ++cur) {
SkPointPriv::SetOrthog(&fNorms[cur], fNorms[cur], fSide);
SkASSERT(SkScalarNearlyEqual(1.0f, fNorms[cur].length()));
} }
}
// Compute the normals and bisectors.
SkASSERT(fNorms.empty());
if (this->numPts() >= 3) {
this->computeNormals();
this->computeBisectors(); this->computeBisectors();
} else if (this->numPts() == 2) { } else if (this->numPts() == 2) {
// We've got two points, so we're degenerate. // We've got two points, so we're degenerate.
@ -445,13 +441,11 @@ bool GrAAConvexTessellator::extractFromPath(const SkMatrix& m, const SkPath& pat
// For stroking, we still need to process the degenerate path, so fix it up // For stroking, we still need to process the degenerate path, so fix it up
fSide = SkPointPriv::kLeft_Side; fSide = SkPointPriv::kLeft_Side;
// Make all the normals face outwards rather than along the edge fNorms.append(2);
for (int cur = 0; cur < fNorms.count(); ++cur) { fNorms[0] = SkPointPriv::MakeOrthog(fPts[1] - fPts[0], fSide);
SkPointPriv::SetOrthog(&fNorms[cur], fNorms[cur], fSide); fNorms[0].normalize();
SkASSERT(SkScalarNearlyEqual(1.0f, fNorms[cur].length())); fNorms[1] = -fNorms[0];
} SkASSERT(SkScalarNearlyEqual(1.0f, fNorms[0].length()));
fNorms.push_back(SkPoint::Make(-fNorms[0].fX, -fNorms[0].fY));
// we won't actually use the bisectors, so just push zeroes // we won't actually use the bisectors, so just push zeroes
fBisectors.push_back(SkPoint::Make(0.0, 0.0)); fBisectors.push_back(SkPoint::Make(0.0, 0.0));
fBisectors.push_back(SkPoint::Make(0.0, 0.0)); fBisectors.push_back(SkPoint::Make(0.0, 0.0));
@ -839,7 +833,7 @@ void GrAAConvexTessellator::Ring::computeNormals(const GrAAConvexTessellator& te
fPts[cur].fNorm = tess.point(fPts[next].fIndex) - tess.point(fPts[cur].fIndex); fPts[cur].fNorm = tess.point(fPts[next].fIndex) - tess.point(fPts[cur].fIndex);
SkPoint::Normalize(&fPts[cur].fNorm); SkPoint::Normalize(&fPts[cur].fNorm);
SkPointPriv::SetOrthog(&fPts[cur].fNorm, fPts[cur].fNorm, tess.side()); fPts[cur].fNorm = SkPointPriv::MakeOrthog(fPts[cur].fNorm, tess.side());
} }
} }
@ -848,13 +842,9 @@ void GrAAConvexTessellator::Ring::computeBisectors(const GrAAConvexTessellator&
for (int cur = 0; cur < fPts.count(); prev = cur, ++cur) { for (int cur = 0; cur < fPts.count(); prev = cur, ++cur) {
fPts[cur].fBisector = fPts[cur].fNorm + fPts[prev].fNorm; fPts[cur].fBisector = fPts[cur].fNorm + fPts[prev].fNorm;
if (!fPts[cur].fBisector.normalize()) { if (!fPts[cur].fBisector.normalize()) {
SkASSERT(SkPointPriv::kLeft_Side == tess.side() || fPts[cur].fBisector =
SkPointPriv::kRight_Side == tess.side()); SkPointPriv::MakeOrthog(fPts[cur].fNorm, (SkPointPriv::Side)-tess.side()) +
SkPointPriv::SetOrthog(&fPts[cur].fBisector, fPts[cur].fNorm, SkPointPriv::MakeOrthog(fPts[prev].fNorm, tess.side());
(SkPointPriv::Side)-tess.side());
SkVector other;
SkPointPriv::SetOrthog(&other, fPts[prev].fNorm, tess.side());
fPts[cur].fBisector += other;
SkAssertResult(fPts[cur].fBisector.normalize()); SkAssertResult(fPts[cur].fBisector.normalize());
} else { } else {
fPts[cur].fBisector.negate(); // make the bisector face in fPts[cur].fBisector.negate(); // make the bisector face in
@ -904,11 +894,10 @@ void GrAAConvexTessellator::lineTo(const SkPoint& p, CurveState curve) {
return; return;
} }
SkASSERT(fPts.count() <= 1 || fPts.count() == fNorms.count()+1); if (this->numPts() >= 2 &&
if (this->numPts() >= 2 && abs_dist_from_line(fPts.top(), fNorms.top(), p) < kClose) { points_are_colinear_and_b_is_middle(fPts[fPts.count() - 2], fPts.top(), p)) {
// The old last point is on the line from the second to last to the new point // The old last point is on the line from the second to last to the new point
this->popLastPt(); this->popLastPt();
fNorms.pop();
// double-check that the new last point is not a duplicate of the new point. In an ideal // double-check that the new last point is not a duplicate of the new point. In an ideal
// world this wouldn't be necessary (since it's only possible for non-convex paths), but // world this wouldn't be necessary (since it's only possible for non-convex paths), but
// floating point precision issues mean it can actually happen on paths that were // floating point precision issues mean it can actually happen on paths that were
@ -919,12 +908,6 @@ void GrAAConvexTessellator::lineTo(const SkPoint& p, CurveState curve) {
} }
SkScalar initialRingCoverage = (SkStrokeRec::kFill_Style == fStyle) ? 0.5f : 1.0f; SkScalar initialRingCoverage = (SkStrokeRec::kFill_Style == fStyle) ? 0.5f : 1.0f;
this->addPt(p, 0.0f, initialRingCoverage, false, curve); this->addPt(p, 0.0f, initialRingCoverage, false, curve);
if (this->numPts() > 1) {
*fNorms.push() = fPts.top() - fPts[fPts.count()-2];
SkDEBUGCODE(SkScalar len =) SkPoint::Normalize(&fNorms.top());
SkASSERT(len > 0.0f);
SkASSERT(SkScalarNearlyEqual(1.0f, fNorms.top().length()));
}
} }
void GrAAConvexTessellator::lineTo(const SkMatrix& m, SkPoint p, CurveState curve) { void GrAAConvexTessellator::lineTo(const SkMatrix& m, SkPoint p, CurveState curve) {

View File

@ -230,6 +230,7 @@ private:
// return false on failure/degenerate path // return false on failure/degenerate path
bool extractFromPath(const SkMatrix& m, const SkPath& path); bool extractFromPath(const SkMatrix& m, const SkPath& path);
void computeBisectors(); void computeBisectors();
void computeNormals();
void fanRing(const Ring& ring); void fanRing(const Ring& ring);

View File

@ -554,15 +554,13 @@ static void bloat_quad(const SkPoint qpts[3], const SkMatrix* toDevice,
SkASSERT(ab.length() > 0 && cb.length() > 0); SkASSERT(ab.length() > 0 && cb.length() > 0);
ab.normalize(); ab.normalize();
SkVector abN; SkVector abN = SkPointPriv::MakeOrthog(ab, SkPointPriv::kLeft_Side);
SkPointPriv::SetOrthog(&abN, ab, SkPointPriv::kLeft_Side);
if (abN.dot(ac) > 0) { if (abN.dot(ac) > 0) {
abN.negate(); abN.negate();
} }
cb.normalize(); cb.normalize();
SkVector cbN; SkVector cbN = SkPointPriv::MakeOrthog(cb, SkPointPriv::kLeft_Side);
SkPointPriv::SetOrthog(&cbN, cb, SkPointPriv::kLeft_Side);
if (cbN.dot(ac) < 0) { if (cbN.dot(ac) < 0) {
cbN.negate(); cbN.negate();
} }