Fix SDF generation for pixel-aligned paths (take two)

BUG=668550

Change-Id: Ic771818bd5a4a46b83fdb82b69b98cb6b93a23a2
Reviewed-on: https://skia-review.googlesource.com/5697
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

3 files changed, 103 insertions, 70 deletions

diff --git a/gm/pathfill.cpp b/gm/pathfill.cpp
index 3496cfd04d..da0efea243 100644
--- a/gm/pathfill.cpp
+++ b/gm/pathfill.cpp
@@ -50,15 +50,15 @@ static SkScalar make_oval(SkPath* path) {
     return SkIntToScalar(30);
 }
 
-static SkScalar make_sawtooth(SkPath* path) {
+static SkScalar make_sawtooth(SkPath* path, int teeth) {
     SkScalar x = SkIntToScalar(20);
     SkScalar y = SkIntToScalar(20);
     const SkScalar x0 = x;
-    const SkScalar dx = SK_Scalar1 * 5;
-    const SkScalar dy = SK_Scalar1 * 10;
+    const SkScalar dx = SkIntToScalar(5);
+    const SkScalar dy = SkIntToScalar(10);
 
     path->moveTo(x, y);
-    for (int i = 0; i < 32; i++) {
+    for (int i = 0; i < teeth; i++) {
         x += dx;
         path->lineTo(x, y - dy);
         x += dx;
@@ -70,6 +70,44 @@ static SkScalar make_sawtooth(SkPath* path) {
     return SkIntToScalar(30);
 }
 
+static SkScalar make_sawtooth_3(SkPath* path) { return make_sawtooth(path, 3); }
+static SkScalar make_sawtooth_32(SkPath* path) { return make_sawtooth(path, 32); }
+
+static SkScalar make_house(SkPath* path) {
+    path->moveTo(21, 23);
+    path->lineTo(21, 11.534f);
+    path->lineTo(22.327f, 12.741f);
+    path->lineTo(23.673f, 11.261f);
+    path->lineTo(12, 0.648f);
+    path->lineTo(8, 4.285f);
+    path->lineTo(8, 2);
+    path->lineTo(4, 2);
+    path->lineTo(4, 7.921f);
+    path->lineTo(0.327f, 11.26f);
+    path->lineTo(1.673f, 12.74f);
+    path->lineTo(3, 11.534f);
+    path->lineTo(3, 23);
+    path->lineTo(11, 23);
+    path->lineTo(11, 18);
+    path->lineTo(13, 18);
+    path->lineTo(13, 23);
+    path->lineTo(21, 23);
+    path->close();
+    path->lineTo(9, 16);
+    path->lineTo(9, 21);
+    path->lineTo(5, 21);
+    path->lineTo(5, 9.715f);
+    path->lineTo(12, 3.351f);
+    path->lineTo(19, 9.715f);
+    path->lineTo(19, 21);
+    path->lineTo(15, 21);
+    path->lineTo(15, 16);
+    path->lineTo(9, 16);
+    path->close();
+    path->offset(20, 0);
+    return SkIntToScalar(30);
+}
+
 static SkScalar make_star(SkPath* path, int n) {
     const SkScalar c = SkIntToScalar(45);
     const SkScalar r = SkIntToScalar(20);
@@ -107,10 +145,12 @@ constexpr MakePathProc gProcs[] = {
     make_triangle,
     make_rect,
     make_oval,
-    make_sawtooth,
+    make_sawtooth_32,
     make_star_5,
     make_star_13,
     make_line,
+    make_house,
+    make_sawtooth_3,
 };
 
 #define N   SK_ARRAY_COUNT(gProcs)
diff --git a/src/gpu/batches/GrAADistanceFieldPathRenderer.cpp b/src/gpu/batches/GrAADistanceFieldPathRenderer.cpp
index 738d1c8f63..87dcbb34fe 100644
--- a/src/gpu/batches/GrAADistanceFieldPathRenderer.cpp
+++ b/src/gpu/batches/GrAADistanceFieldPathRenderer.cpp
@@ -144,13 +144,7 @@ public:
 
         // Compute bounds
         this->setTransformedBounds(shape.bounds(), viewMatrix, HasAABloat::kYes, IsZeroArea::kNo);
-        // There is currently an issue where we may produce 2 pixels worth of AA around the path.
-        // A workaround is to outset the bounds by 1 in device space. (skbug.com/5989)
-        SkRect bounds = this->bounds();
-        bounds.outset(1.f, 1.f);
-        this->setBounds(bounds, HasAABloat::kYes, IsZeroArea::kNo);
     }
-
     const char* name() const override { return "AADistanceFieldPathBatch"; }
 
     SkString dumpInfo() const override {
@@ -247,9 +241,16 @@ private:
             const SkRect& bounds = args.fShape.bounds();
             SkScalar maxDim = SkMaxScalar(bounds.width(), bounds.height());
             SkScalar size = maxScale * maxDim;
-            uint32_t desiredDimension;
-            if (size <= kSmallMIP) {
+            SkScalar desiredDimension;
+            // For minimizing (or the common case of identity) transforms, we try to
+            // create the DF at the appropriately sized native src-space path resolution.
+            // In the majority of cases this will yield a crisper rendering.
+            if (size <= maxDim && maxDim < kSmallMIP) {
+                desiredDimension = maxDim;
+            } else if (size <= kSmallMIP) {
                 desiredDimension = kSmallMIP;
+            } else if (size <= maxDim) {
+                desiredDimension = maxDim;
             } else if (size <= kMediumMIP) {
                 desiredDimension = kMediumMIP;
             } else {
@@ -257,7 +258,7 @@ private:
             }
 
             // check to see if path is cached
-            ShapeData::Key key(args.fShape, desiredDimension);
+            ShapeData::Key key(args.fShape, SkScalarCeilToInt(desiredDimension));
             ShapeData* shapeData = fShapeCache->find(key);
             if (nullptr == shapeData || !atlas->hasID(shapeData->fID)) {
                 // Remove the stale cache entry
@@ -267,13 +268,14 @@ private:
                     delete shapeData;
                 }
                 SkScalar scale = desiredDimension/maxDim;
+
                 shapeData = new ShapeData;
                 if (!this->addPathToAtlas(target,
                                           &flushInfo,
                                           atlas,
                                           shapeData,
                                           args.fShape,
-                                          desiredDimension,
+                                          SkScalarCeilToInt(desiredDimension),
                                           scale)) {
                     delete shapeData;
                     SkDebugf("Can't rasterize path\n");
@@ -288,7 +290,7 @@ private:
                                     offset,
                                     args.fColor,
                                     vertexStride,
-                                    this->viewMatrix(),
+                                    maxScale,
                                     shapeData);
             offset += kVerticesPerQuad * vertexStride;
             flushInfo.fInstancesToFlush++;
@@ -309,29 +311,25 @@ private:
         scaledBounds.fTop *= scale;
         scaledBounds.fRight *= scale;
         scaledBounds.fBottom *= scale;
-        // move the origin to an integer boundary (gives better results)
-        SkScalar dx = SkScalarFraction(scaledBounds.fLeft);
-        SkScalar dy = SkScalarFraction(scaledBounds.fTop);
+        // subtract out integer portion of origin
+        // (SDF created will be placed with fractional offset burnt in)
+        SkScalar dx = SkScalarFloorToInt(scaledBounds.fLeft);
+        SkScalar dy = SkScalarFloorToInt(scaledBounds.fTop);
         scaledBounds.offset(-dx, -dy);
         // get integer boundary
         SkIRect devPathBounds;
         scaledBounds.roundOut(&devPathBounds);
         // pad to allow room for antialiasing
         const int intPad = SkScalarCeilToInt(kAntiAliasPad);
-        // pre-move origin (after outset, will be 0,0)
-        int width = devPathBounds.width();
-        int height = devPathBounds.height();
-        devPathBounds.fLeft = intPad;
-        devPathBounds.fTop = intPad;
-        devPathBounds.fRight = intPad + width;
-        devPathBounds.fBottom = intPad + height;
-        devPathBounds.outset(intPad, intPad);
+        // place devBounds at origin
+        int width = devPathBounds.width() + 2*intPad;
+        int height = devPathBounds.height() + 2*intPad;
+        devPathBounds = SkIRect::MakeWH(width, height);
 
         // draw path to bitmap
         SkMatrix drawMatrix;
-        drawMatrix.setTranslate(-bounds.left(), -bounds.top());
-        drawMatrix.postScale(scale, scale);
-        drawMatrix.postTranslate(kAntiAliasPad, kAntiAliasPad);
+        drawMatrix.setScale(scale, scale);
+        drawMatrix.postTranslate(intPad - dx, intPad - dy);
 
         // setup bitmap backing
         SkASSERT(devPathBounds.fLeft == 0);
@@ -376,7 +374,7 @@ private:
         // add to atlas
         SkIPoint16 atlasLocation;
         GrBatchAtlas::AtlasID id;
-       if (!atlas->addToAtlas(&id, target, width, height, dfStorage.get(), &atlasLocation)) {
+        if (!atlas->addToAtlas(&id, target, width, height, dfStorage.get(), &atlasLocation)) {
             this->flush(target, flushInfo);
             if (!atlas->addToAtlas(&id, target, width, height, dfStorage.get(), &atlasLocation)) {
                 return false;
@@ -385,22 +383,34 @@ private:
 
         // add to cache
         shapeData->fKey.set(shape, dimension);
-        shapeData->fScale = scale;
         shapeData->fID = id;
-        // change the scaled rect to match the size of the inset distance field
-        scaledBounds.fRight = scaledBounds.fLeft +
-            SkIntToScalar(devPathBounds.width() - 2*SK_DistanceFieldInset);
-        scaledBounds.fBottom = scaledBounds.fTop +
-            SkIntToScalar(devPathBounds.height() - 2*SK_DistanceFieldInset);
-        // shift the origin to the correct place relative to the distance field
-        // need to also restore the fractional translation
-        scaledBounds.offset(-SkIntToScalar(SK_DistanceFieldInset) - kAntiAliasPad + dx,
-                            -SkIntToScalar(SK_DistanceFieldInset) - kAntiAliasPad + dy);
-        shapeData->fBounds = scaledBounds;
-        // origin we render from is inset from distance field edge
-        atlasLocation.fX += SK_DistanceFieldInset;
-        atlasLocation.fY += SK_DistanceFieldInset;
-        shapeData->fAtlasLocation = atlasLocation;
+
+        // set the bounds rect to the original bounds
+        shapeData->fBounds = bounds;
+
+        // set up texture coordinates
+        SkScalar texLeft = bounds.fLeft;
+        SkScalar texTop = bounds.fTop;
+        SkScalar texRight = bounds.fRight;
+        SkScalar texBottom = bounds.fBottom;
+
+        // transform original path's bounds to texture space
+        texLeft *= scale;
+        texTop *= scale;
+        texRight *= scale;
+        texBottom *= scale;
+        dx -= SK_DistanceFieldPad + kAntiAliasPad;
+        dy -= SK_DistanceFieldPad + kAntiAliasPad;
+        texLeft += atlasLocation.fX - dx;
+        texTop += atlasLocation.fY - dy;
+        texRight += atlasLocation.fX - dx;
+        texBottom += atlasLocation.fY - dy;
+
+        GrTexture* texture = atlas->getTexture();
+        shapeData->fTexCoords.setLTRB(texLeft / texture->width(),
+                                      texTop / texture->height(),
+                                      texRight / texture->width(),
+                                      texBottom / texture->height());
 
         fShapeCache->add(shapeData);
         fShapeList->addToTail(shapeData);
@@ -415,27 +425,15 @@ private:
                            intptr_t offset,
                            GrColor color,
                            size_t vertexStride,
-                           const SkMatrix& viewMatrix,
+                           SkScalar maxScale,
                            const ShapeData* shapeData) const {
-        GrTexture* texture = atlas->getTexture();
-
-        SkScalar dx = shapeData->fBounds.fLeft;
-        SkScalar dy = shapeData->fBounds.fTop;
-        SkScalar width = shapeData->fBounds.width();
-        SkScalar height = shapeData->fBounds.height();
-
-        SkScalar invScale = 1.0f / shapeData->fScale;
-        dx *= invScale;
-        dy *= invScale;
-        width *= invScale;
-        height *= invScale;
 
         SkPoint* positions = reinterpret_cast<SkPoint*>(offset);
 
         // vertex positions
         // TODO make the vertex attributes a struct
-        SkRect r = SkRect::MakeXYWH(dx, dy, width, height);
-        positions->setRectFan(r.left(), r.top(), r.right(), r.bottom(), vertexStride);
+        positions->setRectFan(shapeData->fBounds.left(), shapeData->fBounds.top(),
+                              shapeData->fBounds.right(), shapeData->fBounds.bottom(), vertexStride);
 
         // colors
         for (int i = 0; i < kVerticesPerQuad; i++) {
@@ -443,15 +441,11 @@ private:
             *colorPtr = color;
         }
 
-        const SkScalar tx = SkIntToScalar(shapeData->fAtlasLocation.fX);
-        const SkScalar ty = SkIntToScalar(shapeData->fAtlasLocation.fY);
-
         // vertex texture coords
+        // TODO make these int16_t
         SkPoint* textureCoords = (SkPoint*)(offset + sizeof(SkPoint) + sizeof(GrColor));
-        textureCoords->setRectFan(tx / texture->width(),
-                                  ty / texture->height(),
-                                  (tx + shapeData->fBounds.width()) / texture->width(),
-                                  (ty + shapeData->fBounds.height())  / texture->height(),
+        textureCoords->setRectFan(shapeData->fTexCoords.left(), shapeData->fTexCoords.top(),
+                                  shapeData->fTexCoords.right(), shapeData->fTexCoords.bottom(),
                                   vertexStride);
     }
 
diff --git a/src/gpu/batches/GrAADistanceFieldPathRenderer.h b/src/gpu/batches/GrAADistanceFieldPathRenderer.h
index 171108af91..b4c3e6c44f 100644
--- a/src/gpu/batches/GrAADistanceFieldPathRenderer.h
+++ b/src/gpu/batches/GrAADistanceFieldPathRenderer.h
@@ -70,10 +70,9 @@ private:
             SkAutoSTArray<24, uint32_t> fKey;
         };
         Key                   fKey;
-        SkScalar              fScale;
         GrBatchAtlas::AtlasID fID;
         SkRect                fBounds;
-        SkIPoint16            fAtlasLocation;
+        SkRect                fTexCoords;
         SK_DECLARE_INTERNAL_LLIST_INTERFACE(ShapeData);
 
         static inline const Key& GetKey(const ShapeData& data) {