Make lines a special case in GrShape

src/gpu/GrShape.cpp

 /*
  * Copyright 2016 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "GrShape.h"
 
 GrShape& GrShape::operator=(const GrShape& that) {
     fStyle = that.fStyle;
     this->changeType(that.fType, Type::kPath == that.fType ? &that.path() : nullptr);
     switch (fType) {
         case Type::kEmpty:
             break;
         case Type::kRRect:
-            fRRectData.fRRect = that.fRRectData.fRRect;
-            fRRectData.fDir = that.fRRectData.fDir;
-            fRRectData.fStart = that.fRRectData.fStart;
-            fRRectData.fInverted = that.fRRectData.fInverted;
+            fRRectData = that.fRRectData;
             break;
+        case Type::kLine:
+            fLineData = that.fLineData;

[robertphillips, 2016/06/28 17:04:05]

break here ?!

[bsalomon, 2016/06/28 17:13:02]

Yikes, done.

         case Type::kPath:
             fPathData.fGenID = that.fPathData.fGenID;
             break;
     }
     fInheritedKey.reset(that.fInheritedKey.count());
     sk_careful_memcpy(fInheritedKey.get(), that.fInheritedKey.get(),
                       sizeof(uint32_t) * fInheritedKey.count());
     return *this;
 }
 
-const SkRect& GrShape::bounds() 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:
             return this->path().getBounds();
     }
     SkFAIL("Unknown shape type");
     return kEmpty;
 }
 
-void GrShape::styledBounds(SkRect* bounds) const {
+SkRect GrShape::styledBounds() const {
     if (Type::kEmpty == fType && !fStyle.hasNonDashPathEffect()) {
-        *bounds = SkRect::MakeEmpty();
-    } else {
-        fStyle.adjustBounds(bounds, this->bounds());
+        return SkRect::MakeEmpty();
     }
+    SkRect bounds;
+    fStyle.adjustBounds(&bounds, this->bounds());
+    return bounds;
 }
 
 int GrShape::unstyledKeySize() const {
     if (fInheritedKey.count()) {
         return fInheritedKey.count();
     }
     switch (fType) {
         case Type::kEmpty:
             return 1;
         case Type::kRRect:
             SkASSERT(!fInheritedKey.count());
             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;
             } else {
                 // The key is the path ID and fill type.
                 return 2;
             }
     }
     SkFAIL("Should never get here.");
     return 0;
@@ skipping 11 matching lines @@
                 *key++ = 1;
                 break;
             case Type::kRRect:
                 fRRectData.fRRect.writeToMemory(key);
                 key += SkRRect::kSizeInMemory / sizeof(uint32_t);
                 *key = (fRRectData.fDir == SkPath::kCCW_Direction) ? (1 << 31) : 0;
                 *key |= fRRectData.fInverted ? (1 << 30) : 0;
                 *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;
                 // We could canonicalize the fill rule for paths that don't differentiate between
                 // even/odd or winding fill (e.g. convex).
                 *key++ = this->path().getFillType();
                 break;
         }
     }
     SkASSERT(key - origKey == this->unstyledKeySize());
@@ skipping 45 matching lines @@
     }
 }
 
 GrShape::GrShape(const GrShape& that) : fStyle(that.fStyle) {
     const SkPath* thatPath = Type::kPath == that.fType ? &that.fPathData.fPath : nullptr;
     this->initType(that.fType, thatPath);
     switch (fType) {
         case Type::kEmpty:
             break;
         case Type::kRRect:
-            fRRectData.fRRect = that.fRRectData.fRRect;
-            fRRectData.fDir = that.fRRectData.fDir;
-            fRRectData.fStart = that.fRRectData.fStart;
-            fRRectData.fInverted = that.fRRectData.fInverted;
+            fRRectData = that.fRRectData;
             break;
+        case Type::kLine:
+            fLineData = that.fLineData;

[robertphillips, 2016/06/28 17:04:05]

break here ?!

[bsalomon, 2016/06/28 17:13:02]

Done.

         case Type::kPath:
             fPathData.fGenID = that.fPathData.fGenID;
             break;
     }
     fInheritedKey.reset(that.fInheritedKey.count());
     sk_careful_memcpy(fInheritedKey.get(), that.fInheritedKey.get(),
                       sizeof(uint32_t) * fInheritedKey.count());
 }
 
 GrShape::GrShape(const GrShape& parent, GrStyle::Apply apply, SkScalar scale) {
@@ skipping 82 matching lines @@
     this->attemptToSimplifyPath();
     this->setInheritedKey(*parentForKey, apply, scale);
 }
 
 void GrShape::attemptToSimplifyPath() {
     SkRect rect;
     SkRRect rrect;
     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;
         fRRectData.fDir = rrectDir;
         fRRectData.fStart = rrectStart;
         fRRectData.fInverted = inverted;
         // Currently SkPath does not acknowledge that empty, rect, or oval subtypes as rrects.
         SkASSERT(!fRRectData.fRRect.isEmpty());
         SkASSERT(fRRectData.fRRect.getType() != SkRRect::kRect_Type);
         SkASSERT(fRRectData.fRRect.getType() != SkRRect::kOval_Type);
@@ skipping 25 matching lines @@
                 fRRectData.fStart = kDefaultRRectStart;
                 // There isn't dashing so we will have to preserver inverseness.
                 fRRectData.fInverted = inverted;
             }
         }
     }
     if (Type::kPath != fType) {
         fInheritedKey.reset(0);
         if (Type::kRRect == fType) {
             this->attemptToSimplifyRRect();
+        } else if (Type::kLine == fType) {
+            this->attemptToSimplifyLine();
         }
     } else {
         if (fInheritedKey.count() || this->path().isVolatile()) {
             fPathData.fGenID = 0;
         } else {
             fPathData.fGenID = this->path().getGenerationID();
         }
         if (this->style().isSimpleFill()) {
-            // 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);
-            }
+            this->path().close();
+            this->path().setIsVolatile(true);
         }
         if (!this->style().hasNonDashPathEffect()) {
             if (this->style().strokeRec().getStyle() == SkStrokeRec::kStroke_Style ||
                 this->style().strokeRec().getStyle() == SkStrokeRec::kHairline_Style) {
                 // Stroke styles don't differentiate between winding and even/odd.
                 // Moreover, dashing ignores inverseness (skbug.com/5421)
                 bool inverse = !this->style().isDashed() && this->path().isInverseFillType();
                 if (inverse) {
                     this->path().setFillType(kDefaultPathInverseFillType);
                 } else {
@@ skipping 20 matching lines @@
         return;
     }
     if (!this->style().hasPathEffect()) {
         fRRectData.fDir = kDefaultRRectDir;
         fRRectData.fStart = kDefaultRRectStart;
     } else if (fStyle.isDashed()) {
         // Dashing ignores the inverseness (currently). skbug.com/5421
         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()) {

[robertphillips, 2016/06/28 17:04:05]

// dashing ignores inverseness
?

[bsalomon, 2016/06/28 17:13:02]

Done.

+            fLineData.fInverted = false;
+        }
+    }
+}