Revert of Make lines a special case in GrShape (patchset #5 id:120001 of https:/...

Reason for revert:
Assertion failures

Original issue's description:
> Make lines a special case in GrShape
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2108523002
>
> Committed: https://skia.googlesource.com/skia/+/c62318c748a1907649bd75382c4f4fd10533f2b3

TBR=robertphillips@google.com,egdaniel@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review-Url: https://codereview.chromium.org/2109613003

diff --git a/src/gpu/GrShape.cpp b/src/gpu/GrShape.cpp
index 5ffd32d..45ddb77 100644 (file)
--- a/src/gpu/GrShape.cpp
+++ b/src/gpu/GrShape.cpp
@@ -14,10 +14,10 @@ GrShape& GrShape::operator=(const GrShape& that) {
         case Type::kEmpty:
             break;
         case Type::kRRect:
-            fRRectData = that.fRRectData;
-            break;
-        case Type::kLine:
-            fLineData = that.fLineData;
+            fRRectData.fRRect = that.fRRectData.fRRect;
+            fRRectData.fDir = that.fRRectData.fDir;
+            fRRectData.fStart = that.fRRectData.fStart;
+            fRRectData.fInverted = that.fRRectData.fInverted;
             break;
         case Type::kPath:
             fPathData.fGenID = that.fPathData.fGenID;
@@ -29,29 +29,11 @@ GrShape& GrShape::operator=(const GrShape& that) {
     return *this;
 }
 
-SkRect GrShape::bounds() const {
+const SkRect& GrShape::bounds() const {
     static constexpr SkRect kEmpty = SkRect::MakeEmpty();
     switch (fType) {
         case Type::kEmpty:
             return kEmpty;
-        case Type::kLine: {
-            SkRect bounds;
-            if (fLineData.fPts[0].fX < fLineData.fPts[1].fX) {
-                bounds.fLeft = fLineData.fPts[0].fX;
-                bounds.fRight = fLineData.fPts[1].fX;
-            } else {
-                bounds.fLeft = fLineData.fPts[1].fX;
-                bounds.fRight = fLineData.fPts[0].fX;
-            }
-            if (fLineData.fPts[0].fY < fLineData.fPts[1].fY) {
-                bounds.fTop = fLineData.fPts[0].fY;
-                bounds.fBottom = fLineData.fPts[1].fY;
-            } else {
-                bounds.fTop = fLineData.fPts[1].fY;
-                bounds.fBottom = fLineData.fPts[0].fY;
-            }
-            return bounds;
-        }
         case Type::kRRect:
             return fRRectData.fRRect.getBounds();
         case Type::kPath:
@@ -61,13 +43,12 @@ SkRect GrShape::bounds() const {
     return kEmpty;
 }
 
-SkRect GrShape::styledBounds() const {
+void GrShape::styledBounds(SkRect* bounds) const {
     if (Type::kEmpty == fType && !fStyle.hasNonDashPathEffect()) {
-        return SkRect::MakeEmpty();
+        *bounds = SkRect::MakeEmpty();
+    } else {
+        fStyle.adjustBounds(bounds, this->bounds());
     }
-    SkRect bounds;
-    fStyle.adjustBounds(&bounds, this->bounds());
-    return bounds;
 }
 
 int GrShape::unstyledKeySize() const {
@@ -82,10 +63,6 @@ int GrShape::unstyledKeySize() const {
             SkASSERT(0 == SkRRect::kSizeInMemory % sizeof(uint32_t));
             // + 1 for the direction, start index, and inverseness.
             return SkRRect::kSizeInMemory / sizeof(uint32_t) + 1;
-        case Type::kLine:
-            GR_STATIC_ASSERT(2 * sizeof(uint32_t) == sizeof(SkPoint));
-            // 4 for the end points and 1 for the inverseness
-            return 5;
         case Type::kPath:
             if (0 == fPathData.fGenID) {
                 return -1;
@@ -117,11 +94,6 @@ void GrShape::writeUnstyledKey(uint32_t* key) const {
                 *key++ |= fRRectData.fStart;
                 SkASSERT(fRRectData.fStart < 8);
                 break;
-            case Type::kLine:
-                memcpy(key, fLineData.fPts, 2 * sizeof(SkPoint));
-                key += 4;
-                *key++ = fLineData.fInverted ? 1 : 0;
-                break;
             case Type::kPath:
                 SkASSERT(fPathData.fGenID);
                 *key++ = fPathData.fGenID;
@@ -187,10 +159,10 @@ GrShape::GrShape(const GrShape& that) : fStyle(that.fStyle) {
         case Type::kEmpty:
             break;
         case Type::kRRect:
-            fRRectData = that.fRRectData;
-            break;
-        case Type::kLine:
-            fLineData = that.fLineData;
+            fRRectData.fRRect = that.fRRectData.fRRect;
+            fRRectData.fDir = that.fRRectData.fDir;
+            fRRectData.fStart = that.fRRectData.fStart;
+            fRRectData.fInverted = that.fRRectData.fInverted;
             break;
         case Type::kPath:
             fPathData.fGenID = that.fPathData.fGenID;
@@ -294,14 +266,8 @@ void GrShape::attemptToSimplifyPath() {
     SkPath::Direction rrectDir;
     unsigned rrectStart;
     bool inverted = this->path().isInverseFillType();
-    SkPoint pts[2];
     if (this->path().isEmpty()) {
         this->changeType(Type::kEmpty);
-    } else if (this->path().isLine(pts)) {
-        this->changeType(Type::kLine);
-        fLineData.fPts[0] = pts[0];
-        fLineData.fPts[1] = pts[1];
-        fLineData.fInverted = inverted;
     } else if (this->path().isRRect(&rrect, &rrectDir, &rrectStart)) {
         this->changeType(Type::kRRect);
         fRRectData.fRRect = rrect;
@@ -347,8 +313,6 @@ void GrShape::attemptToSimplifyPath() {
         fInheritedKey.reset(0);
         if (Type::kRRect == fType) {
             this->attemptToSimplifyRRect();
-        } else if (Type::kLine == fType) {
-            this->attemptToSimplifyLine();
         }
     } else {
         if (fInheritedKey.count() || this->path().isVolatile()) {
@@ -357,8 +321,13 @@ void GrShape::attemptToSimplifyPath() {
             fPathData.fGenID = this->path().getGenerationID();
         }
         if (this->style().isSimpleFill()) {
-            this->path().close();
-            this->path().setIsVolatile(true);
+            // Filled paths are treated as though all their contours were closed.
+            // Since SkPath doesn't track individual contours, this will only close the last. :(
+            // There is no point in closing lines, though, since they loose their line-ness.
+            if (!this->path().isLine(nullptr)) {
+                this->path().close();
+                this->path().setIsVolatile(true);
+            }
         }
         if (!this->style().hasNonDashPathEffect()) {
             if (this->style().strokeRec().getStyle() == SkStrokeRec::kStroke_Style ||
@@ -399,21 +368,3 @@ void GrShape::attemptToSimplifyRRect() {
         fRRectData.fInverted = false;
     }
 }
-
-void GrShape::attemptToSimplifyLine() {
-    if (fStyle.isSimpleFill() && !fLineData.fInverted) {
-        this->changeType(Type::kEmpty);
-    } else {
-        // Only path effects could care about the order of the points. Otherwise canonicalize
-        // the point order
-        if (!fStyle.hasPathEffect()) {
-            SkPoint* pts = fLineData.fPts;
-            if (pts[1].fY < pts[0].fY || (pts[1].fY == pts[0].fY && pts[1].fX < pts[0].fX)) {
-                SkTSwap(pts[0], pts[1]);
-            }
-        } else if (fStyle.isDashed()) {
-            // Dashing ignores inverseness.
-            fLineData.fInverted = false;
-        }
-    }
-}

diff --git a/src/gpu/GrShape.h b/src/gpu/GrShape.h
index 8f3be58..87ee26e 100644 (file)
--- a/src/gpu/GrShape.h
+++ b/src/gpu/GrShape.h
@@ -152,19 +152,12 @@ public:
      * If the unstyled shape is a straight line segment, returns true and sets pts to the endpoints.
      * An inverse filled line path is still considered a line.
      */
-    bool asLine(SkPoint pts[2], bool* inverted) const {
-        if (fType != Type::kLine) {
-            return false;
-        }
-        if (pts) {
-            pts[0] = fLineData.fPts[0];
-            pts[1] = fLineData.fPts[1];
-        }
-        if (inverted) {
-            *inverted = fLineData.fInverted;
-        }
-        return true;
-    }
+     bool asLine(SkPoint pts[2]) const {
+         if (fType != Type::kPath) {
+             return false;
+         }
+         return this->path().isLine(pts);
+     }
 
     /** Returns the unstyled geometry as a path. */
     void asPath(SkPath* out) const {
@@ -182,16 +175,6 @@ public:
                     out->setFillType(kDefaultPathFillType);
                 }
                 break;
-            case Type::kLine:
-                out->reset();
-                out->moveTo(fLineData.fPts[0]);
-                out->lineTo(fLineData.fPts[1]);
-                if (fLineData.fInverted) {
-                    out->setFillType(kDefaultPathInverseFillType);
-                } else {
-                    out->setFillType(kDefaultPathFillType);
-                }
-                break;
             case Type::kPath:
                 *out = this->path();
                 break;
@@ -208,13 +191,13 @@ public:
      * Gets the bounds of the geometry without reflecting the shape's styling. This ignores
      * the inverse fill nature of the geometry.
      */
-    SkRect bounds() const;
+    const SkRect& bounds() const;
 
     /**
      * Gets the bounds of the geometry reflecting the shape's styling (ignoring inverse fill
      * status).
      */
-    SkRect styledBounds() const;
+    void styledBounds(SkRect* bounds) const;
 
     /**
      * Is this shape known to be convex, before styling is applied. An unclosed but otherwise
@@ -227,8 +210,6 @@ public:
                 return true;
             case Type::kRRect:
                 return true;
-            case Type::kLine:
-                return true;
             case Type::kPath:
                 // SkPath.isConvex() really means "is this path convex were it to be closed" and
                 // thus doesn't give the correct answer for stroked paths, hence we also check
@@ -250,9 +231,6 @@ public:
             case Type::kRRect:
                 ret = fRRectData.fInverted;
                 break;
-            case Type::kLine:
-                ret = fLineData.fInverted;
-                break;
             case Type::kPath:
                 ret = this->path().isInverseFillType();
                 break;
@@ -286,8 +264,6 @@ public:
                 return true;
             case Type::kRRect:
                 return true;
-            case Type::kLine:
-                return false;
             case Type::kPath:
                 // SkPath doesn't keep track of the closed status of each contour.
                 return SkPathPriv::IsClosedSingleContour(this->path());
@@ -306,8 +282,6 @@ public:
                     return SkPath::kLine_SegmentMask;
                 }
                 return SkPath::kLine_SegmentMask | SkPath::kConic_SegmentMask;
-            case Type::kLine:
-                return SkPath::kLine_SegmentMask;
             case Type::kPath:
                 return this->path().getSegmentMasks();
         }
@@ -333,7 +307,6 @@ private:
     enum class Type {
         kEmpty,
         kRRect,
-        kLine,
         kPath,
     };
 
@@ -383,7 +356,6 @@ private:
 
     void attemptToSimplifyPath();
     void attemptToSimplifyRRect();
-    void attemptToSimplifyLine();
 
     // Defaults to use when there is no distinction between even/odd and winding fills.
     static constexpr SkPath::FillType kDefaultPathFillType = SkPath::kEvenOdd_FillType;
@@ -448,10 +420,6 @@ private:
             // Gen ID of the original path (fPath may be modified)
             int32_t                     fGenID;
         } fPathData;
-        struct {
-            SkPoint                     fPts[2];
-            bool                        fInverted;
-        } fLineData;
     };
     GrStyle                     fStyle;
     SkAutoSTArray<8, uint32_t>  fInheritedKey;

diff --git a/src/gpu/GrSoftwarePathRenderer.cpp b/src/gpu/GrSoftwarePathRenderer.cpp
index 45a4b78..983bd20 100644 (file)
--- a/src/gpu/GrSoftwarePathRenderer.cpp
+++ b/src/gpu/GrSoftwarePathRenderer.cpp
@@ -38,7 +38,8 @@ bool get_shape_and_clip_bounds(int width, int height,
         *devShapeBounds = SkIRect::MakeWH(width, height);
         return false;
     }
-    SkRect shapeBounds = shape.styledBounds();
+    SkRect shapeBounds;
+    shape.styledBounds(&shapeBounds);
     if (!shapeBounds.isEmpty()) {
         SkRect shapeSBounds;
         matrix.mapRect(&shapeSBounds, shapeBounds);

diff --git a/src/gpu/batches/GrAADistanceFieldPathRenderer.cpp b/src/gpu/batches/GrAADistanceFieldPathRenderer.cpp
index eb6a67d..b178a68 100644 (file)
--- a/src/gpu/batches/GrAADistanceFieldPathRenderer.cpp
+++ b/src/gpu/batches/GrAADistanceFieldPathRenderer.cpp
@@ -109,7 +109,8 @@ bool GrAADistanceFieldPathRenderer::onCanDrawPath(const CanDrawPathArgs& args) c
     // scaled to have bounds within 2.0f*kLargeMIP by 2.0f*kLargeMIP
     // the goal is to accelerate rendering of lots of small paths that may be scaling
     SkScalar maxScale = args.fViewMatrix->getMaxScale();
-    SkRect bounds = args.fShape->styledBounds();
+    SkRect bounds;
+    args.fShape->styledBounds(&bounds);
     SkScalar maxDim = SkMaxScalar(bounds.width(), bounds.height());
 
     return maxDim <= kMediumMIP && maxDim * maxScale <= 2.0f*kLargeMIP;

diff --git a/src/gpu/batches/GrDashLinePathRenderer.cpp b/src/gpu/batches/GrDashLinePathRenderer.cpp
index e7ef240..8dba7b9 100644 (file)
--- a/src/gpu/batches/GrDashLinePathRenderer.cpp
+++ b/src/gpu/batches/GrDashLinePathRenderer.cpp
@@ -13,10 +13,7 @@
 
 bool GrDashLinePathRenderer::onCanDrawPath(const CanDrawPathArgs& args) const {
     SkPoint pts[2];
-    bool inverted;
-    if (args.fShape->style().isDashed() && args.fShape->asLine(pts, &inverted)) {
-        // We should never have an inverse dashed case.
-        SkASSERT(!inverted);
+    if (args.fShape->style().isDashed() && args.fShape->asLine(pts)) {
         return GrDashingEffect::CanDrawDashLine(pts, args.fShape->style(), *args.fViewMatrix);
     }
     return false;
@@ -37,7 +34,7 @@ bool GrDashLinePathRenderer::onDrawPath(const DrawPathArgs& args) {
         aaMode = GrDashingEffect::AAMode::kNone;
     }
     SkPoint pts[2];
-    SkAssertResult(args.fShape->asLine(pts, nullptr));
+    SkAssertResult(args.fShape->asLine(pts));
     SkAutoTUnref<GrDrawBatch> batch(GrDashingEffect::CreateDashLineBatch(args.fColor,
                                                                          *args.fViewMatrix,
                                                                          pts,

diff --git a/tests/GrShapeTest.cpp b/tests/GrShapeTest.cpp
index ca907a1..1ee9bbc 100644 (file)
--- a/tests/GrShapeTest.cpp
+++ b/tests/GrShapeTest.cpp
@@ -177,9 +177,10 @@ private:
         CheckBounds(r, fAppliedPE, fAppliedPE.bounds());
         CheckBounds(r, fAppliedPEThenStroke, fAppliedPEThenStroke.bounds());
         CheckBounds(r, fAppliedFull, fAppliedFull.bounds());
-        SkRect styledBounds = fBase.styledBounds();
+        SkRect styledBounds;
+        fBase.styledBounds(&styledBounds);
         CheckBounds(r, fAppliedFull, styledBounds);
-        styledBounds = fAppliedPE.styledBounds();
+        fAppliedPE.styledBounds(&styledBounds);
         CheckBounds(r, fAppliedFull, styledBounds);
 
         // Check that the same path is produced when style is applied by GrShape and GrStyle.
@@ -371,10 +372,8 @@ static void check_equivalence(skiatest::Reporter* r, const GrShape& a, const GrS
     }
     REPORTER_ASSERT(r, a.bounds() == b.bounds());
     REPORTER_ASSERT(r, a.segmentMask() == b.segmentMask());
-    // Init these to suppress warnings.
-    SkPoint pts[4] {{0, 0,}, {0, 0}, {0, 0}, {0, 0}} ;
-    bool invertedLine[2] {true, true};
-    REPORTER_ASSERT(r, a.asLine(pts, &invertedLine[0]) == b.asLine(pts + 2, &invertedLine[1]));
+    SkPoint pts[4];
+    REPORTER_ASSERT(r, a.asLine(pts) == b.asLine(pts + 2));
     // mayBeInverseFilledAfterStyling() is allowed to differ if one has a arbitrary PE and the other
     // doesn't (since the PE can set any fill type on its output path).
     // Moreover, dash style explicitly ignores inverseness. So if one is dashed but not the other
@@ -384,11 +383,8 @@ static void check_equivalence(skiatest::Reporter* r, const GrShape& a, const GrS
         REPORTER_ASSERT(r, a.mayBeInverseFilledAfterStyling() ==
                            b.mayBeInverseFilledAfterStyling());
     }
-    if (a.asLine(nullptr, nullptr)) {
+    if (a.asLine(pts)) {
         REPORTER_ASSERT(r, pts[2] == pts[0] && pts[3] == pts[1]);
-        REPORTER_ASSERT(r, ignoreInversenessDifference || invertedLine[0] == invertedLine[1]);
-        REPORTER_ASSERT(r, invertedLine[0] == a.inverseFilled());
-        REPORTER_ASSERT(r, invertedLine[1] == b.inverseFilled());
     }
     REPORTER_ASSERT(r, ignoreInversenessDifference || a.inverseFilled() == b.inverseFilled());
 }
@@ -1353,114 +1349,6 @@ void test_rrect(skiatest::Reporter* r, const SkRRect& rrect) {
     }
 }
 
-void test_lines(skiatest::Reporter* r) {
-    static constexpr SkPoint kA { 1,  1};
-    static constexpr SkPoint kB { 5, -9};
-    static constexpr SkPoint kC {-3, 17};
-
-    SkPath lineAB;
-    lineAB.moveTo(kA);
-    lineAB.lineTo(kB);
-
-    SkPath lineBA;
-    lineBA.moveTo(kB);
-    lineBA.lineTo(kA);
-
-    SkPath lineAC;
-    lineAC.moveTo(kB);
-    lineAC.lineTo(kC);
-
-    SkPath invLineAB = lineAB;
-    invLineAB.setFillType(SkPath::kInverseEvenOdd_FillType);
-
-    SkPaint fill;
-    SkPaint stroke;
-    stroke.setStyle(SkPaint::kStroke_Style);
-    stroke.setStrokeWidth(2.f);
-    SkPaint hairline;
-    hairline.setStyle(SkPaint::kStroke_Style);
-    hairline.setStrokeWidth(0.f);
-    SkPaint dash = stroke;
-    dash.setPathEffect(make_dash());
-
-    TestCase fillAB(lineAB, fill, r);
-    TestCase fillEmpty(SkPath(), fill, r);
-    fillAB.compare(r, fillEmpty, TestCase::kAllSame_ComparisonExpecation);
-    REPORTER_ASSERT(r, !fillAB.baseShape().asLine(nullptr, nullptr));
-
-    TestCase strokeAB(lineAB, stroke, r);
-    TestCase strokeBA(lineBA, stroke, r);
-    TestCase strokeAC(lineAC, stroke, r);
-
-    TestCase hairlineAB(lineAB, hairline, r);
-    TestCase hairlineBA(lineBA, hairline, r);
-    TestCase hairlineAC(lineAC, hairline, r);
-
-    TestCase dashAB(lineAB, dash, r);
-    TestCase dashBA(lineBA, dash, r);
-    TestCase dashAC(lineAC, dash, r);
-
-    strokeAB.compare(r, fillAB, TestCase::kAllDifferent_ComparisonExpecation);
-
-    strokeAB.compare(r, strokeBA, TestCase::kAllSame_ComparisonExpecation);
-    strokeAB.compare(r, strokeAC, TestCase::kAllDifferent_ComparisonExpecation);
-
-    hairlineAB.compare(r, hairlineBA, TestCase::kAllSame_ComparisonExpecation);
-    hairlineAB.compare(r, hairlineAC, TestCase::kAllDifferent_ComparisonExpecation);
-
-    dashAB.compare(r, dashBA, TestCase::kAllDifferent_ComparisonExpecation);
-    dashAB.compare(r, dashAC, TestCase::kAllDifferent_ComparisonExpecation);
-
-    strokeAB.compare(r, hairlineAB, TestCase::kSameUpToStroke_ComparisonExpecation);
-
-    // One of dashAB or dashBA should have the same line as strokeAB. It depends upon how
-    // GrShape canonicalizes line endpoints (when it can, i.e. when not dashed).
-    bool canonicalizeAsAB;
-    SkPoint canonicalPts[2] {kA, kB};
-    // Init these to suppress warnings.
-    bool inverted = true;
-    SkPoint pts[2] {{0, 0}, {0, 0}};
-    REPORTER_ASSERT(r, strokeAB.baseShape().asLine(pts, &inverted) && !inverted);
-    if (pts[0] == kA && pts[1] == kB) {
-        canonicalizeAsAB = true;
-    } else if (pts[1] == kA && pts[0] == kB) {
-        canonicalizeAsAB = false;
-        SkTSwap(canonicalPts[0], canonicalPts[1]);
-    } else {
-        ERRORF(r, "Should return pts (a,b) or (b, a)");
-        return;
-    };
-
-    strokeAB.compare(r, canonicalizeAsAB ? dashAB : dashBA,
-                     TestCase::kSameUpToPE_ComparisonExpecation);
-    REPORTER_ASSERT(r, strokeAB.baseShape().asLine(pts, &inverted) && !inverted &&
-                       pts[0] == canonicalPts[0] && pts[1] == canonicalPts[1]);
-    REPORTER_ASSERT(r, hairlineAB.baseShape().asLine(pts, &inverted) && !inverted &&
-                       pts[0] == canonicalPts[0] && pts[1] == canonicalPts[1]);
-    REPORTER_ASSERT(r, dashAB.baseShape().asLine(pts, &inverted) && !inverted &&
-                       pts[0] == kA && pts[1] == kB);
-    REPORTER_ASSERT(r, dashBA.baseShape().asLine(pts, &inverted) && !inverted &&
-                       pts[0] == kB && pts[1] == kA);
-
-
-    TestCase strokeInvAB(invLineAB, stroke, r);
-    TestCase hairlineInvAB(invLineAB, hairline, r);
-    TestCase dashInvAB(invLineAB, dash, r);
-    strokeInvAB.compare(r, strokeAB, TestCase::kAllDifferent_ComparisonExpecation);
-    hairlineInvAB.compare(r, hairlineAB, TestCase::kAllDifferent_ComparisonExpecation);
-    // Dashing ignores inverse.
-    dashInvAB.compare(r, dashAB, TestCase::kAllSame_ComparisonExpecation);
-
-    REPORTER_ASSERT(r, strokeInvAB.baseShape().asLine(pts, &inverted) && inverted &&
-                       pts[0] == canonicalPts[0] && pts[1] == canonicalPts[1]);
-    REPORTER_ASSERT(r, hairlineInvAB.baseShape().asLine(pts, &inverted) && inverted &&
-                       pts[0] == canonicalPts[0] && pts[1] == canonicalPts[1]);
-    // Dashing ignores inverse.
-    REPORTER_ASSERT(r, dashInvAB.baseShape().asLine(pts, &inverted) && !inverted &&
-                       pts[0] == kA && pts[1] == kB);
-
-}
-
 DEF_TEST(GrShape, reporter) {
     for (auto r : { SkRect::MakeWH(10, 20),
                     SkRect::MakeWH(-10, -20),
@@ -1487,7 +1375,7 @@ DEF_TEST(GrShape, reporter) {
         test_path_effect_fails(reporter, r);
         test_make_hairline_path_effect(reporter, r, true);
         GrShape shape(r);
-        REPORTER_ASSERT(reporter, !shape.asLine(nullptr, nullptr));
+        REPORTER_ASSERT(reporter, !shape.asLine(nullptr));
     }
 
     for (auto rr : { SkRRect::MakeRect(SkRect::MakeWH(10, 10)),
@@ -1515,7 +1403,7 @@ DEF_TEST(GrShape, reporter) {
         test_path_effect_fails(reporter, rr);
         test_make_hairline_path_effect(reporter, rr, true);
         GrShape shape(rr);
-        REPORTER_ASSERT(reporter, !shape.asLine(nullptr, nullptr));
+        REPORTER_ASSERT(reporter, !shape.asLine(nullptr));
     }
 
     struct TestPath {
@@ -1571,16 +1459,15 @@ DEF_TEST(GrShape, reporter) {
             // These tests all assume that the original GrShape for fill and stroke will be the
             // same.
             // However, that is not the case in special cases (e.g. an unclosed rect becomes a RRect
-            // GrShape with a fill style but becomes a Path GrShape when stroked). Similarly, a path
-            // that is a line becomes empty when filled but is special-cased as a line when stroked.
-            if (testPath.fIsRRectForFill == testPath.fIsRRectForStroke && !testPath.fIsLine) {
+            // GrShape with a fill style but becomes a Path GrShape when stroked).
+            if (testPath.fIsRRectForFill == testPath.fIsRRectForStroke) {
                 test_basic(reporter, path);
                 test_null_dash(reporter, path);
                 test_path_effect_makes_rrect(reporter, path);
             }
             test_scale(reporter, path);
             // This test uses a stroking paint, hence use of fIsRRectForStroke
-            test_volatile_path(reporter, path, testPath.fIsRRectForStroke || testPath.fIsLine);
+            test_volatile_path(reporter, path, testPath.fIsRRectForStroke);
             test_dash_fill(reporter, path);
             // Test modifying various stroke params.
             test_stroke_param<SkPath, SkScalar>(
@@ -1596,8 +1483,7 @@ DEF_TEST(GrShape, reporter) {
             test_unknown_path_effect(reporter, path);
             test_path_effect_makes_empty_shape(reporter, path);
             test_path_effect_fails(reporter, path);
-            test_make_hairline_path_effect(reporter, path, testPath.fIsRRectForStroke ||
-                                                           testPath.fIsLine);
+            test_make_hairline_path_effect(reporter, path, testPath.fIsRRectForStroke);
         }
     }
 
@@ -1630,14 +1516,14 @@ DEF_TEST(GrShape, reporter) {
             strokePathCase.compare(reporter, strokeRRectCase,
                                    TestCase::kAllSame_ComparisonExpecation);
         }
+        REPORTER_ASSERT(reporter, testPath.fIsLine == fillPathCase.baseShape().asLine(nullptr));
+        REPORTER_ASSERT(reporter, testPath.fIsLine == strokePathCase.baseShape().asLine(nullptr));
     }
 
     // Test a volatile empty path.
     test_volatile_path(reporter, SkPath(), true);
 
     test_empty_shape(reporter);
-
-    test_lines(reporter);
 }
 
 #endif