Fix GrShape to preserve inverseness of rrects for strokes but not dashes.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2051263003
Review-Url: https://codereview.chromium.org/2051263003
diff --git a/src/gpu/GrShape.cpp b/src/gpu/GrShape.cpp
index 11b2d38..7be7e7a 100644
--- a/src/gpu/GrShape.cpp
+++ b/src/gpu/GrShape.cpp
@@ -281,16 +281,16 @@
static inline bool rrect_path_is_inverse_filled(const SkPath& path, const SkStrokeRec& strokeRec,
const SkPathEffect* pe) {
- // Dashing doesn't use the path fill type. Dashing only works with stroking
+ // This is currently imitating the questionable behavior of the sw-rasterizer. Inverseness is
+ // respected for stroking but not dashing + stroking. (We make no assumptions about arbitrary
+ // path effects and preserve the path's inverseness.)
+ // skbug.com/5421
if (pe && pe->asADash(nullptr)) {
- pe = nullptr;
- }
-
- SkStrokeRec::Style style = strokeRec.getStyle();
- if (!pe && (SkStrokeRec::kStroke_Style == style || SkStrokeRec::kHairline_Style == style)) {
- // stroking ignores the path fill rule.
+ SkDEBUGCODE(SkStrokeRec::Style style = strokeRec.getStyle();)
+ SkASSERT(SkStrokeRec::kStroke_Style == style || SkStrokeRec::kHairline_Style == style);
return false;
}
+
return path.isInverseFillType();
}
diff --git a/src/gpu/GrShape.h b/src/gpu/GrShape.h
index 93896ce..6da1a41 100644
--- a/src/gpu/GrShape.h
+++ b/src/gpu/GrShape.h
@@ -293,13 +293,9 @@
SkASSERT(Type::kRRect == fType);
SkASSERT(!fInheritedKey.count());
if (fRRectIsInverted) {
- if (!fStyle.hasNonDashPathEffect()) {
- SkStrokeRec::Style recStyle = fStyle.strokeRec().getStyle();
- if (SkStrokeRec::kStroke_Style == recStyle ||
- SkStrokeRec::kHairline_Style == recStyle) {
- // stroking ignores the path fill rule.
- fRRectIsInverted = false;
- }
+ if (fStyle.isDashed()) {
+ // Dashing ignores the inverseness (currently). skbug.com/5421
+ fRRectIsInverted = false;
}
} else if (fRRect.isEmpty()) {
fType = Type::kEmpty;
diff --git a/tests/GrShapeTest.cpp b/tests/GrShapeTest.cpp
index 5fa1db9..04e2af6 100644
--- a/tests/GrShapeTest.cpp
+++ b/tests/GrShapeTest.cpp
@@ -281,21 +281,15 @@
a.asPath(&pathA);
b.asPath(&pathB);
- // Having a fill style or non-dash path effect can prevent 'a' but not 'b' from turning an
- // inverse fill type into a non-inverse fill type.
+ // Having a dash path effect can allow 'a' but not 'b' to turn a inverse fill type into a
+ // non-inverse fill type (or vice versa).
bool ignoreInversenessDifference = false;
if (pathA.isInverseFillType() != pathB.isInverseFillType()) {
const GrShape* s1 = pathA.isInverseFillType() ? &a : &b;
const GrShape* s2 = pathA.isInverseFillType() ? &b : &a;
- SkStrokeRec::Style style1 = s1->style().strokeRec().getStyle();
- SkStrokeRec::Style style2 = s2->style().strokeRec().getStyle();
- bool canDropInverse1 = !s1->style().hasNonDashPathEffect() &&
- (SkStrokeRec::kStroke_Style == style1 ||
- SkStrokeRec::kHairline_Style == style1);
- bool canDropInverse2 = !s2->style().hasNonDashPathEffect() &&
- (SkStrokeRec::kStroke_Style == style2 ||
- SkStrokeRec::kHairline_Style == style2);
- ignoreInversenessDifference = !canDropInverse1 && canDropInverse2;
+ bool canDropInverse1 = s1->style().isDashed();
+ bool canDropInverse2 = s2->style().isDashed();
+ ignoreInversenessDifference = (canDropInverse1 != canDropInverse2);
}
if (allowSameRRectButDiffStartAndDir) {
@@ -1094,32 +1088,50 @@
}
}
- static const SkPath::Direction kDir = SkPath::kCW_Direction; // arbitrary
- const GrShape& exampleFillCase = shapes[index(false, kDir, 0, kFill, false)];
+ // Get the keys for some example shape instances that we'll use for comparision against the
+ // rest.
+ static constexpr SkPath::Direction kExamplesDir = SkPath::kCW_Direction;
+ static constexpr unsigned kExamplesStart = 0;
+ const GrShape& exampleFillCase = shapes[index(false, kExamplesDir, kExamplesStart, kFill,
+ false)];
Key exampleFillCaseKey;
make_key(&exampleFillCaseKey, exampleFillCase);
- const GrShape& exampleStrokeAndFillCase = shapes[index(false, kDir, 0, kStrokeAndFill, false)];
+ const GrShape& exampleStrokeAndFillCase = shapes[index(false, kExamplesDir, kExamplesStart,
+ kStrokeAndFill, false)];
Key exampleStrokeAndFillCaseKey;
make_key(&exampleStrokeAndFillCaseKey, exampleStrokeAndFillCase);
- const GrShape& exampleInvFillCase = shapes[index(true, kDir, 0, kFill, false)];
+ const GrShape& exampleInvFillCase = shapes[index(true, kExamplesDir, kExamplesStart, kFill,
+ false)];
Key exampleInvFillCaseKey;
make_key(&exampleInvFillCaseKey, exampleInvFillCase);
- const GrShape& exampleInvStrokeAndFillCase =
- shapes[index(true, kDir, 0, kStrokeAndFill, false)];
+ const GrShape& exampleInvStrokeAndFillCase = shapes[index(true, kExamplesDir, kExamplesStart,
+ kStrokeAndFill, false)];
Key exampleInvStrokeAndFillCaseKey;
make_key(&exampleInvStrokeAndFillCaseKey, exampleInvStrokeAndFillCase);
- const GrShape& exampleStrokeCase = shapes[index(false, kDir, 0, kStroke, false)];
+ const GrShape& exampleStrokeCase = shapes[index(false, kExamplesDir, kExamplesStart, kStroke,
+ false)];
Key exampleStrokeCaseKey;
make_key(&exampleStrokeCaseKey, exampleStrokeCase);
- const GrShape& exampleHairlineCase = shapes[index(false, kDir, 0, kHairline, false)];
+ const GrShape& exampleInvStrokeCase = shapes[index(true, kExamplesDir, kExamplesStart, kStroke,
+ false)];
+ Key exampleInvStrokeCaseKey;
+ make_key(&exampleInvStrokeCaseKey, exampleInvStrokeCase);
+
+ const GrShape& exampleHairlineCase = shapes[index(false, kExamplesDir, kExamplesStart,
+ kHairline, false)];
Key exampleHairlineCaseKey;
make_key(&exampleHairlineCaseKey, exampleHairlineCase);
+ const GrShape& exampleInvHairlineCase = shapes[index(true, kExamplesDir, kExamplesStart,
+ kHairline, false)];
+ Key exampleInvHairlineCaseKey;
+ make_key(&exampleInvHairlineCaseKey, exampleInvHairlineCase);
+
// These are dummy initializations to suppress warnings.
SkRRect queryRR = SkRRect::MakeEmpty();
SkPath::Direction queryDir = SkPath::kCW_Direction;
@@ -1160,19 +1172,37 @@
REPORTER_ASSERT(r, 0 == queryStart);
REPORTER_ASSERT(r, !queryInverted);
+ REPORTER_ASSERT(r, exampleInvHairlineCase.asRRect(&queryRR, &queryDir, &queryStart,
+ &queryInverted));
+ REPORTER_ASSERT(r, queryRR == rrect);
+ REPORTER_ASSERT(r, SkPath::kCW_Direction == queryDir);
+ REPORTER_ASSERT(r, 0 == queryStart);
+ REPORTER_ASSERT(r, queryInverted);
+
REPORTER_ASSERT(r, exampleStrokeCase.asRRect(&queryRR, &queryDir, &queryStart, &queryInverted));
REPORTER_ASSERT(r, queryRR == rrect);
REPORTER_ASSERT(r, SkPath::kCW_Direction == queryDir);
REPORTER_ASSERT(r, 0 == queryStart);
REPORTER_ASSERT(r, !queryInverted);
+ REPORTER_ASSERT(r, exampleInvStrokeCase.asRRect(&queryRR, &queryDir, &queryStart,
+ &queryInverted));
+ REPORTER_ASSERT(r, queryRR == rrect);
+ REPORTER_ASSERT(r, SkPath::kCW_Direction == queryDir);
+ REPORTER_ASSERT(r, 0 == queryStart);
+ REPORTER_ASSERT(r, queryInverted);
+
// Remember that the key reflects the geometry before styling is applied.
REPORTER_ASSERT(r, exampleFillCaseKey != exampleInvFillCaseKey);
REPORTER_ASSERT(r, exampleFillCaseKey == exampleStrokeAndFillCaseKey);
REPORTER_ASSERT(r, exampleFillCaseKey != exampleInvStrokeAndFillCaseKey);
REPORTER_ASSERT(r, exampleFillCaseKey == exampleStrokeCaseKey);
+ REPORTER_ASSERT(r, exampleFillCaseKey != exampleInvStrokeCaseKey);
REPORTER_ASSERT(r, exampleFillCaseKey == exampleHairlineCaseKey);
+ REPORTER_ASSERT(r, exampleFillCaseKey != exampleInvHairlineCaseKey);
REPORTER_ASSERT(r, exampleInvStrokeAndFillCaseKey == exampleInvFillCaseKey);
+ REPORTER_ASSERT(r, exampleInvStrokeAndFillCaseKey == exampleInvStrokeCaseKey);
+ REPORTER_ASSERT(r, exampleInvStrokeAndFillCaseKey == exampleInvHairlineCaseKey);
for (bool inverted : {false, true}) {
for (SkPath::Direction dir : {SkPath::kCW_Direction, SkPath::kCCW_Direction}) {
@@ -1204,13 +1234,13 @@
dash)];
TestCase e(strokeCase, r);
- TestCase f(exampleStrokeCase, r);
TestCase g(hairlineCase, r);
- TestCase h(exampleHairlineCase, r);
- // Both hairline and stroke shapes must respect the dashing and both
- // ignore inverseness.
+ // Both hairline and stroke shapes must respect the dashing.
if (dash) {
+ // Dashing always ignores the inverseness. skbug.com/5421
+ TestCase f(exampleStrokeCase, r);
+ TestCase h(exampleHairlineCase, r);
unsigned expectedStart = canonicalize_rrect_start(start, rrect);
REPORTER_ASSERT(r, strokeCase.style().pathEffect());
REPORTER_ASSERT(r, hairlineCase.style().pathEffect());
@@ -1238,6 +1268,8 @@
g.compare(r, h, TestCase::kAllDifferent_ComparisonExpecation);
}
} else {
+ TestCase f(inverted ? exampleInvStrokeCase : exampleStrokeCase, r);
+ TestCase h(inverted ? exampleInvHairlineCase : exampleHairlineCase, r);
REPORTER_ASSERT(r, !strokeCase.style().pathEffect());
REPORTER_ASSERT(r, !hairlineCase.style().pathEffect());
e.compare(r, f, TestCase::kAllSame_ComparisonExpecation);
@@ -1385,6 +1417,7 @@
test_make_hairline_path_effect(reporter, path, testPath.fIsRRectForStroke);
}
}
+
for (auto testPath : paths) {
const SkPath& path = testPath.fPath;