drawText Cleanup, part 3

Change-Id: I17bdc317b2f22bb3d4c33785aa2163e8beb3b280
Reviewed-on: https://skia-review.googlesource.com/c/182201
Auto-Submit: Hal Canary <halcanary@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>

tests/DrawTextTest.cpp

@@ -29,10 +29,6 @@ static void create(SkBitmap* bm, SkIRect bound) {
     bm->allocN32Pixels(bound.width(), bound.height());
 }
 
-static void drawBG(SkCanvas* canvas) {
-    canvas->drawColor(bgColor);
-}
-
 /** Assumes that the ref draw was completely inside ref canvas --
     implies that everything outside is "bgColor".
     Checks that all overlap is the same and that all non-overlap on the
@@ -82,17 +78,17 @@ DEF_TEST(DrawText_dashout, reporter) {
     SkCanvas emptyCanvas(emptyBitmap);
 
     SkPoint point = SkPoint::Make(25.0f, 25.0f);
+    SkFont font(nullptr, 20);
+    font.setEdging(SkFont::Edging::kSubpixelAntiAlias);
+    font.setSubpixel(true);
+
     SkPaint paint;
     paint.setColor(SK_ColorGRAY);
-    paint.setTextSize(SkIntToScalar(20));
-    paint.setAntiAlias(true);
-    paint.setSubpixelText(true);
-    paint.setLCDRenderText(true);
     paint.setStyle(SkPaint::kStroke_Style);
 
     // Draw a stroked "A" without a dash which will draw something.
-    drawBG(&drawTextCanvas);
+    drawTextCanvas.drawColor(SK_ColorWHITE);
-    drawTextCanvas.drawText("A", 1, point.fX, point.fY, paint);
+    drawTextCanvas.drawString("A", point.fX, point.fY, font, paint);
 
     // Draw an "A" but with a dash which will never draw anything.
     paint.setStrokeWidth(2);
@@ -100,11 +96,11 @@ DEF_TEST(DrawText_dashout, reporter) {
     static constexpr SkScalar intervals[] = { 1, bigInterval };
     paint.setPathEffect(SkDashPathEffect::Make(intervals, SK_ARRAY_COUNT(intervals), 2));
 
-    drawBG(&drawDashedTextCanvas);
+    drawDashedTextCanvas.drawColor(SK_ColorWHITE);
-    drawDashedTextCanvas.drawText("A", 1, point.fX, point.fY, paint);
+    drawDashedTextCanvas.drawString("A", point.fX, point.fY, font, paint);
 
     // Draw nothing.
-    drawBG(&emptyCanvas);
+    emptyCanvas.drawColor(SK_ColorWHITE);
 
     REPORTER_ASSERT(reporter, !compare(drawTextBitmap, size, emptyBitmap, size));
     REPORTER_ASSERT(reporter, compare(drawDashedTextBitmap, size, emptyBitmap, size));

tests/FontHostStreamTest.cpp

@@ -20,41 +20,14 @@
 #include "SkTypes.h"
 #include "Test.h"
 
-static const SkColor bgColor = SK_ColorWHITE;
+/** Checks that all is the same.  */
-
+static bool compare(const SkBitmap& ref, const SkBitmap& test) {
-static void create(SkBitmap* bm, SkIRect bound) {
+    if (ref.dimensions() != test.dimensions()) {
-    bm->allocN32Pixels(bound.width(), bound.height());
+        return false;
-}
+    }
-
-static void drawBG(SkCanvas* canvas) {
-    canvas->drawColor(bgColor);
-}
-
-/** Assumes that the ref draw was completely inside ref canvas --
-    implies that everything outside is "bgColor".
-    Checks that all overlap is the same and that all non-overlap on the
-    ref is "bgColor".
- */
-static bool compare(const SkBitmap& ref, const SkIRect& iref,
-                    const SkBitmap& test, const SkIRect& itest)
-{
-    const int xOff = itest.fLeft - iref.fLeft;
-    const int yOff = itest.fTop - iref.fTop;
-
     for (int y = 0; y < test.height(); ++y) {
         for (int x = 0; x < test.width(); ++x) {
-            SkColor testColor = test.getColor(x, y);
+            if (test.getColor(x, y) != ref.getColor(x, y)) {
-            int refX = x + xOff;
-            int refY = y + yOff;
-            SkColor refColor;
-            if (refX >= 0 && refX < ref.width() &&
-                refY >= 0 && refY < ref.height())
-            {
-                refColor = ref.getColor(refX, refY);
-            } else {
-                refColor = bgColor;
-            }
-            if (refColor != testColor) {
                 return false;
             }
         }
@@ -62,52 +35,47 @@ static bool compare(const SkBitmap& ref, const SkIRect& iref,
     return true;
 }
 
+static void draw(SkBitmap* bitmap, const SkFont& font) {
+    SkPaint paint;
+    paint.setColor(SK_ColorGRAY);
+    SkASSERT(bitmap);
+    bitmap->allocN32Pixels(64, 64);
+    SkCanvas canvas(*bitmap);
+    canvas.drawColor(SK_ColorWHITE);
+    canvas.drawString("A", 24, 32, font, paint);
+}
+
 DEF_TEST(FontHostStream, reporter) {
-    {
+    sk_sp<SkTypeface> typeface = SkTypeface::MakeFromName("Georgia", SkFontStyle());
-        SkPaint paint;
+    if (!typeface) {
-        paint.setColor(SK_ColorGRAY);
+        typeface = SkTypeface::MakeDefault();
-
-        SkFont font(SkTypeface::MakeFromName("Georgia", SkFontStyle()), 30);
-
-        SkIRect origRect = SkIRect::MakeWH(64, 64);
-        SkBitmap origBitmap;
-        create(&origBitmap, origRect);
-        SkCanvas origCanvas(origBitmap);
-
-        SkIRect streamRect = SkIRect::MakeWH(64, 64);
-        SkBitmap streamBitmap;
-        create(&streamBitmap, streamRect);
-        SkCanvas streamCanvas(streamBitmap);
-
-        SkPoint point = SkPoint::Make(24, 32);
-
-        // Test: origTypeface and streamTypeface from orig data draw the same
-        drawBG(&origCanvas);
-        origCanvas.drawSimpleText("A", 1, kUTF8_SkTextEncoding, point.fX, point.fY, font, paint);
-
-        sk_sp<SkTypeface> typeface = SkPaintPriv::RefTypefaceOrDefault(paint);
-        int ttcIndex;
-        std::unique_ptr<SkStreamAsset> fontData(typeface->openStream(&ttcIndex));
-        if (!fontData) {
-            // We're using a SkTypeface that can't give us a stream.
-            // This happens with portable or system fonts.  End the test now.
-            return;
-        }
-
-        sk_sp<SkTypeface> streamTypeface(SkTypeface::MakeFromStream(std::move(fontData)));
-
-        SkFontDescriptor desc;
-        bool isLocalStream = false;
-        streamTypeface->getFontDescriptor(&desc, &isLocalStream);
-        REPORTER_ASSERT(reporter, isLocalStream);
-
-        paint.setTypeface(streamTypeface);
-        drawBG(&streamCanvas);
-        streamCanvas.drawSimpleText("A", 1, kUTF8_SkTextEncoding, point.fX, point.fY, font, paint);
-
-        REPORTER_ASSERT(reporter,
-                        compare(origBitmap, origRect, streamBitmap, streamRect));
     }
+    SkFont font(typeface, 30);
+    SkBitmap origBitmap, streamBitmap;
+
+    // Test: origTypeface and streamTypeface from orig data draw the same
+    draw(&origBitmap, font);
+
+    int ttcIndex;
+    std::unique_ptr<SkStreamAsset> fontData(typeface->openStream(&ttcIndex));
+    if (!fontData) {
+        // We're using a SkTypeface that can't give us a stream.
+        // This happens with portable or system fonts.  End the test now.
+        return;
+    }
+
+    sk_sp<SkTypeface> streamTypeface(SkTypeface::MakeFromStream(std::move(fontData)));
+
+    SkFontDescriptor desc;
+    bool isLocalStream = false;
+    streamTypeface->getFontDescriptor(&desc, &isLocalStream);
+    REPORTER_ASSERT(reporter, isLocalStream);
+
+    SkFont streamFont(streamTypeface);
+    draw(&streamBitmap, streamFont);
+
+    REPORTER_ASSERT(reporter, compare(origBitmap, streamBitmap));
+
     //Make sure the typeface is deleted and removed.
     SkGraphics::PurgeFontCache();
 }

tests/FontMgrAndroidParserTest.cpp

@@ -5,15 +5,17 @@
  * found in the LICENSE file.
  */
 
+#include "Test.h"
+
 #include "Resources.h"
 #include "SkCanvas.h"
 #include "SkCommandLineFlags.h"
 #include "SkFixed.h"
+#include "SkFont.h"
 #include "SkFontMgr_android.h"
 #include "SkFontMgr_android_parser.h"
 #include "SkOSFile.h"
 #include "SkTypeface.h"
-#include "Test.h"
 
 #include <cmath>
 #include <cstdio>
@@ -301,17 +303,10 @@ DEF_TEST(FontMgrAndroidSystemVariableTypeface, reporter) {
     SkCanvas canvasClone(bitmapClone);
     canvasStream.drawColor(SK_ColorWHITE);
 
-    SkPaint paintStream;
+    SkPaint paint;
-    paintStream.setColor(SK_ColorGRAY);
+    paint.setColor(SK_ColorGRAY);
-    paintStream.setTextSize(SkIntToScalar(20));
+    paint.setAntiAlias(true);
-    paintStream.setAntiAlias(true);
+    constexpr float kTextSize = 20;
-    paintStream.setLCDRenderText(true);
-
-    SkPaint paintClone;
-    paintClone.setColor(SK_ColorGRAY);
-    paintClone.setTextSize(SkIntToScalar(20));
-    paintClone.setAntiAlias(true);
-    paintClone.setLCDRenderText(true);
 
     std::unique_ptr<SkStreamAsset> distortableStream(
         GetResourceAsStream("fonts/Distortable.ttf"));
@@ -319,8 +314,6 @@ DEF_TEST(FontMgrAndroidSystemVariableTypeface, reporter) {
         return;
     }
 
-    const char* text = "abc";
-    const size_t textLen = strlen(text);
     SkPoint point = SkPoint::Make(20.0f, 20.0f);
     SkFourByteTag tag = SkSetFourByteTag('w', 'g', 'h', 't');
 
@@ -332,18 +325,24 @@ DEF_TEST(FontMgrAndroidSystemVariableTypeface, reporter) {
         SkFontArguments::VariationPosition
             position = {coordinates, SK_ARRAY_COUNT(coordinates)};
 
-        paintStream.setTypeface(sk_sp<SkTypeface>(
+        SkFont fontStream(
             fontMgr->makeFromStream(distortableStream->duplicate(),
-                                    SkFontArguments().setVariationDesignPosition(position))));
+                                    SkFontArguments().setVariationDesignPosition(position)),
+            kTextSize);
+        fontStream.setEdging(SkFont::Edging::kSubpixelAntiAlias);
 
-        paintClone.setTypeface(sk_sp<SkTypeface>(
+
-            typeface->makeClone(SkFontArguments().setVariationDesignPosition(position))));
+        SkFont fontClone(
+            typeface->makeClone(SkFontArguments().setVariationDesignPosition(position)), kTextSize);
+        fontClone.setEdging(SkFont::Edging::kSubpixelAntiAlias);
+
+        constexpr char text[] = "abc";
 
         canvasStream.drawColor(SK_ColorWHITE);
-        canvasStream.drawText(text, textLen, point.fX, point.fY, paintStream);
+        canvasStream.drawString(text, point.fX, point.fY, fontStream, paint);
 
         canvasClone.drawColor(SK_ColorWHITE);
-        canvasClone.drawText(text, textLen, point.fX, point.fY, paintClone);
+        canvasClone.drawString(text, point.fX, point.fY, fontClone, paint);
 
         bool success = bitmap_compare(bitmapStream, bitmapClone);
         REPORTER_ASSERT(reporter, success);

tests/FontMgrFontConfigTest.cpp

@@ -7,6 +7,7 @@
 
 #include "Resources.h"
 #include "SkCanvas.h"
+#include "SkFont.h"
 #include "SkFontMgr.h"
 #include "SkFontMgr_fontconfig.h"
 #include "SkTypeface.h"
@@ -47,17 +48,10 @@ DEF_TEST(FontMgrFontConfig, reporter) {
     SkCanvas canvasClone(bitmapClone);
     canvasStream.drawColor(SK_ColorWHITE);
 
-    SkPaint paintStream;
+    SkPaint paint;
-    paintStream.setColor(SK_ColorGRAY);
+    paint.setColor(SK_ColorGRAY);
-    paintStream.setTextSize(SkIntToScalar(20));
-    paintStream.setAntiAlias(true);
-    paintStream.setLCDRenderText(true);
 
-    SkPaint paintClone;
+    constexpr float kTextSize = 20;
-    paintClone.setColor(SK_ColorGRAY);
-    paintClone.setTextSize(SkIntToScalar(20));
-    paintClone.setAntiAlias(true);
-    paintClone.setLCDRenderText(true);
 
     std::unique_ptr<SkStreamAsset> distortableStream(
         GetResourceAsStream("fonts/Distortable.ttf"));
@@ -65,8 +59,6 @@ DEF_TEST(FontMgrFontConfig, reporter) {
         return;
     }
 
-    const char* text = "abc";
-    const size_t textLen = strlen(text);
     SkPoint point = SkPoint::Make(20.0f, 20.0f);
     SkFourByteTag tag = SkSetFourByteTag('w', 'g', 'h', 't');
 
@@ -78,17 +70,23 @@ DEF_TEST(FontMgrFontConfig, reporter) {
         SkFontArguments::VariationPosition
             position = {coordinates, SK_ARRAY_COUNT(coordinates)};
 
-        paintStream.setTypeface(sk_sp<SkTypeface>(
+        SkFont fontStream(
             fontMgr->makeFromStream(distortableStream->duplicate(),
-                                    SkFontArguments().setVariationDesignPosition(position))));
+                                    SkFontArguments().setVariationDesignPosition(position)),
-        paintClone.setTypeface(sk_sp<SkTypeface>(
+            kTextSize);
-            typeface->makeClone(SkFontArguments().setVariationDesignPosition(position))));
+        fontStream.setEdging(SkFont::Edging::kSubpixelAntiAlias);
+
+        SkFont fontClone(
+            typeface->makeClone(SkFontArguments().setVariationDesignPosition(position)), kTextSize);
+        fontClone.setEdging(SkFont::Edging::kSubpixelAntiAlias);
+
+        constexpr char text[] = "abc";
 
         canvasStream.drawColor(SK_ColorWHITE);
-        canvasStream.drawText(text, textLen, point.fX, point.fY, paintStream);
+        canvasStream.drawString(text, point.fX, point.fY, fontStream, paint);
 
         canvasClone.drawColor(SK_ColorWHITE);
-        canvasClone.drawText(text, textLen, point.fX, point.fY, paintClone);
+        canvasClone.drawString(text, point.fX, point.fY, fontClone, paint);
 
         bool success = bitmap_compare(bitmapStream, bitmapClone);
         REPORTER_ASSERT(reporter, success);

tests/PDFTaggedTest.cpp

@@ -9,6 +9,7 @@
 #include "SkCanvas.h"
 #include "SkPDFDocument.h"
 #include "SkStream.h"
+#include "SkFont.h"
 
 using PDFTag = SkPDF::StructureElementNode;
 
@@ -106,42 +107,42 @@ DEF_TEST(SkPDF_tagged, r) {
             document->beginPage(pageSize.width(),
                                 pageSize.height());
     SkPDF::SetNodeId(canvas, 2);
-    paint.setTextSize(36);
+    SkFont font(nullptr, 36);
     const char* message = "This is the title";
     canvas->translate(72, 72);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     SkPDF::SetNodeId(canvas, 3);
-    paint.setTextSize(14);
+    font.setSize(14);
     message = "This is a simple paragraph.";
     canvas->translate(0, 72);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     SkPDF::SetNodeId(canvas, 6);
-    paint.setTextSize(14);
+    font.setSize(14);
     message = "*";
     canvas->translate(0, 72);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     SkPDF::SetNodeId(canvas, 7);
     message = "List item 1";
     canvas->translate(36, 0);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     SkPDF::SetNodeId(canvas, 8);
     message = "*";
     canvas->translate(-36, 36);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     SkPDF::SetNodeId(canvas, 9);
     message = "List item 2";
     canvas->translate(36, 0);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     SkPDF::SetNodeId(canvas, 10);
     message = "This is a paragraph that starts on one page";
     canvas->translate(-36, 6 * 72);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     document->endPage();
 
@@ -151,14 +152,14 @@ DEF_TEST(SkPDF_tagged, r) {
     SkPDF::SetNodeId(canvas, 10);
     message = "and finishes on the second page.";
     canvas->translate(72, 72);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     // This has a node ID but never shows up in the tag tree so it
     // won't be tagged.
     SkPDF::SetNodeId(canvas, 999);
     message = "Page 2";
     canvas->translate(468, -36);
-    canvas->drawText(message, strlen(message), 0, 0, paint);
+    canvas->drawString(message, 0, 0, font, paint);
 
     document->endPage();
 

tests/SerialProcsTest.cpp

@@ -182,10 +182,11 @@ static sk_sp<SkPicture> make_picture(sk_sp<SkTypeface> tf0, sk_sp<SkTypeface> tf
     SkPictureRecorder rec;
     SkCanvas* canvas = rec.beginRecording(100, 100);
     SkPaint paint;
-    paint.setTypeface(tf0); canvas->drawText("hello", 5, 0, 0, paint);
+    SkFont font;
-    paint.setTypeface(tf1); canvas->drawText("hello", 5, 0, 0, paint);
+    font.setTypeface(tf0); canvas->drawString("hello", 0, 0, font, paint);
-    paint.setTypeface(tf0); canvas->drawText("hello", 5, 0, 0, paint);
+    font.setTypeface(tf1); canvas->drawString("hello", 0, 0, font, paint);
-    paint.setTypeface(tf1); canvas->drawText("hello", 5, 0, 0, paint);
+    font.setTypeface(tf0); canvas->drawString("hello", 0, 0, font, paint);
+    font.setTypeface(tf1); canvas->drawString("hello", 0, 0, font, paint);
     return rec.finishRecordingAsPicture();
 }
 

tests/SerializationTest.cpp

@@ -341,11 +341,10 @@ static void compare_bitmaps(skiatest::Reporter* reporter,
 static void serialize_and_compare_typeface(sk_sp<SkTypeface> typeface, const char* text,
                                            skiatest::Reporter* reporter)
 {
-    // Create a paint with the typeface.
+    // Create a font with the typeface.
     SkPaint paint;
     paint.setColor(SK_ColorGRAY);
-    paint.setTextSize(SkIntToScalar(30));
+    SkFont font(std::move(typeface), 30);
-    paint.setTypeface(std::move(typeface));
 
     // Paint some text.
     SkPictureRecorder recorder;
@@ -354,7 +353,7 @@ static void serialize_and_compare_typeface(sk_sp<SkTypeface> typeface, const cha
                                                SkIntToScalar(canvasRect.height()),
                                                nullptr, 0);
     canvas->drawColor(SK_ColorWHITE);
-    canvas->drawText(text, 2, 24, 32, paint);
+    canvas->drawString(text, 24, 32, font, paint);
     sk_sp<SkPicture> picture(recorder.finishRecordingAsPicture());
 
     // Serlialize picture and create its clone from stream.
@@ -392,7 +391,7 @@ static void TestPictureTypefaceSerialization(skiatest::Reporter* reporter) {
             if (!typeface) {
                 INFOF(reporter, "Could not run fontstream test because Distortable.ttf not created.");
             } else {
-                serialize_and_compare_typeface(std::move(typeface), "abc", reporter);
+                serialize_and_compare_typeface(std::move(typeface), "ab", reporter);
             }
         }
     }