Cleanups related to SkFixed.

This CL relands https://codereview.chromium.org/1683743005/ with two fixes:
  - Removing the test for vertical text. Vertical text doesn't work on Windows and breakText isn't supported for non-trivial text.
  - Omit PaintBreakTextTest in Google3 because we don't have the correct font setup yet.

Remove SK_FixedNaN. It does not seem to be supported or used anywhere in Skia, Chromium, Android, or Google3, (except accidentally by TwoPtRadial::kDontDrawT). I think supporting it would erase any benefit of SkFixed over float.

Remove SkBitmapFilter::lookup. It does not appear to be used anywhere in Skia, Chromium, Android, or Google3.

Fix a bug in SkPaint::breakText that limited it to ~5kB of text. Now it can handle more than 1GB.

BUG=skia:4632
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1683743005

Committed: https://skia.googlesource.com/skia/+/7ea5cb18389db11a94175de95c9db2b44972630c

patch from issue 1683743005 at patchset 120001 (http://crrev.com/1683743005#ps120001)

Review URL: https://codereview.chromium.org/1739453002

diff --git a/include/core/SkFixed.h b/include/core/SkFixed.h
index f6dd3d60020207382e3f9bdf238bde5a0ba8955c..1541e32105ff55cb995cfcbce4a3d2cf1ba5127c 100644 (file)
--- a/include/core/SkFixed.h
+++ b/include/core/SkFixed.h
@@ -22,7 +22,6 @@ typedef int32_t             SkFixed;
 #define SK_FixedHalf        (1 << 15)
 #define SK_FixedMax         (0x7FFFFFFF)
 #define SK_FixedMin         (-SK_FixedMax)
-#define SK_FixedNaN         ((int) 0x80000000)
 #define SK_FixedPI          (0x3243F)
 #define SK_FixedSqrt2       (92682)
 #define SK_FixedTanPIOver8  (0x6A0A)

diff --git a/include/core/SkRect.h b/include/core/SkRect.h
index 4f649b7c7325efcb82872f329eb83ac25d614198..3ebe099ae6385a2aa7a0fd124f1ccb87cd6ded5c 100644 (file)
--- a/include/core/SkRect.h
+++ b/include/core/SkRect.h
@@ -463,8 +463,7 @@ struct SK_API SkRect {
 
     /**
      *  Returns true iff all values in the rect are finite. If any are
-     *  infinite or NaN (or SK_FixedNaN when SkScalar is fixed) then this
-     *  returns false.
+     *  infinite or NaN then this returns false.
      */
     bool isFinite() const {
         float accum = 0;

diff --git a/public.bzl b/public.bzl
index 0fb0ceaf7dd1eadb0a7a4dde1f655beaaea4feaa..986bfd93421065cdae70d180eb3d6274c9fff8e5 100644 (file)
--- a/public.bzl
+++ b/public.bzl
@@ -464,7 +464,7 @@ def DM_ARGS(base_dir):
         "--nogpu",
         "--verbose",
         # TODO(mtklein): maybe investigate why these fail?
-        "--match ~FontMgr ~Scalar ~Canvas ~Codec_stripes ~Codec_Dimensions ~Codec ~Stream ~skps ~RecordDraw_TextBounds",
+        "--match ~FontMgr ~Scalar ~Canvas ~Codec_stripes ~Codec_Dimensions ~Codec ~Stream ~skps ~RecordDraw_TextBounds ~PaintBreakText",
         "--resourcePath %s/resources" % base_dir,
         "--images %s/resources" % base_dir,
     ]

diff --git a/samplecode/SampleText.cpp b/samplecode/SampleText.cpp
index f5f8dbefa0b6ef7e35ab167ab3941a1bd4137313..87ca91406b7060438cba25e85022f8f60f98ee6e 100644 (file)
--- a/samplecode/SampleText.cpp
+++ b/samplecode/SampleText.cpp
@@ -27,41 +27,6 @@
 #include "SkStream.h"
 #include "SkXMLParser.h"
 
-static void test_breakText() {
-    SkPaint paint;
-    const char* text = "sdfkljAKLDFJKEWkldfjlk#$%&sdfs.dsj";
-    size_t length = strlen(text);
-    SkScalar width = paint.measureText(text, length);
-
-    SkScalar mm = 0;
-    SkScalar nn = 0;
-    for (SkScalar w = 0; w <= width; w += SK_Scalar1) {
-        SkScalar m;
-        size_t n = paint.breakText(text, length, w, &m);
-
-        SkASSERT(n <= length);
-        SkASSERT(m <= width);
-
-        if (n == 0) {
-            SkASSERT(m == 0);
-        } else {
-            // now assert that we're monotonic
-            if (n == nn) {
-                SkASSERT(m == mm);
-            } else {
-                SkASSERT(n > nn);
-                SkASSERT(m > mm);
-            }
-        }
-        nn = SkIntToScalar((unsigned int)n);
-        mm = m;
-    }
-
-    SkDEBUGCODE(size_t length2 =) paint.breakText(text, length, width, &mm);
-    SkASSERT(length2 == length);
-    SkASSERT(mm == width);
-}
-
 static const struct {
     const char* fName;
     uint32_t    fFlags;
@@ -109,8 +74,6 @@ public:
     TextSpeedView() {
         fHints = 0;
         fClickX = 0;
-
-        test_breakText();
     }
 
 protected:

diff --git a/src/core/SkBitmapFilter.h b/src/core/SkBitmapFilter.h
index 6fa8edde3460ab3992cc14f0e90c216f83665436..ca3e0930f27570ebff359e95370a81f59d189f5c 100644 (file)
--- a/src/core/SkBitmapFilter.h
+++ b/src/core/SkBitmapFilter.h
@@ -28,15 +28,6 @@ public:
     }
     virtual ~SkBitmapFilter() {}
 
-    SkFixed lookup(float x) const {
-        if (!fPrecomputed) {
-            precomputeTable();
-        }
-        int filter_idx = int(sk_float_abs(x * fLookupMultiplier));
-        SkASSERT(filter_idx < SKBITMAP_FILTER_TABLE_SIZE);
-        return fFilterTable[filter_idx];
-    }
-
     SkScalar lookupScalar(float x) const {
         if (!fPrecomputed) {
             precomputeTable();
@@ -67,19 +58,16 @@ protected:
     float fLookupMultiplier;
 
     mutable bool fPrecomputed;
-    mutable SkFixed fFilterTable[SKBITMAP_FILTER_TABLE_SIZE];
     mutable SkScalar fFilterTableScalar[SKBITMAP_FILTER_TABLE_SIZE];
 
 private:
     void precomputeTable() const {
         fPrecomputed = true;
-        SkFixed *ftp = fFilterTable;
         SkScalar *ftpScalar = fFilterTableScalar;
         for (int x = 0; x < SKBITMAP_FILTER_TABLE_SIZE; ++x) {
             float fx = ((float)x + .5f) * this->width() / SKBITMAP_FILTER_TABLE_SIZE;
             float filter_value = evaluate(fx);
             *ftpScalar++ = filter_value;
-            *ftp++ = SkFloatToFixed(filter_value);
         }
     }
 };

diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp
index d3384a628d05dc421758816d893bbbb7800fe403..638e251b363908a57d5875690472b7ccf35e7077 100644 (file)
--- a/src/core/SkPaint.cpp
+++ b/src/core/SkPaint.cpp
@@ -925,7 +925,7 @@ size_t SkPaint::breakText(const void* textD, size_t length, SkScalar maxWidth,
     GlyphCacheProc   glyphCacheProc = paint.getGlyphCacheProc(false);
     const int        xyIndex = paint.isVerticalText() ? 1 : 0;
     // use 64bits for our accumulator, to avoid overflowing 16.16
-    Sk48Dot16        max = SkScalarToFixed(maxWidth);
+    Sk48Dot16        max = SkScalarTo48Dot16(maxWidth);
     Sk48Dot16        width = 0;
 
     SkAutoKern  autokern;

diff --git a/src/effects/gradients/SkTwoPointConicalGradient.h b/src/effects/gradients/SkTwoPointConicalGradient.h
index f468de800785c8ee5528cdad87fddce8b9a282e3..954b09698eec907016be913e14a46e6531a3cb99 100644 (file)
--- a/src/effects/gradients/SkTwoPointConicalGradient.h
+++ b/src/effects/gradients/SkTwoPointConicalGradient.h
@@ -15,6 +15,7 @@
 // Should only be initialized once via init(). Immutable afterwards.
 struct TwoPtRadial {
     enum {
+        // This value is outside the range SK_FixedMin to SK_FixedMax.
         kDontDrawT  = 0x80000000
     };
 

diff --git a/tests/PaintBreakTextTest.cpp b/tests/PaintBreakTextTest.cpp
new file mode 100644 (file)
index 0000000..474bbf6
--- /dev/null
+++ b/tests/PaintBreakTextTest.cpp
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2011-2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkPaint.h"
+#include "Test.h"
+
+static void test_monotonic(skiatest::Reporter* reporter,
+                           const SkPaint& paint,
+                           const char* msg) {
+    const char* text = "sdfkljAKLDFJKEWkldfjlk#$%&sdfs.dsj";
+    const size_t length = strlen(text);
+    const SkScalar width = paint.measureText(text, length);
+
+    SkScalar mm = 0;
+    size_t nn = 0;
+    const SkScalar step = SkMaxScalar(width / 10, SK_Scalar1);
+    for (SkScalar w = 0; w <= width; w += step) {
+        SkScalar m;
+        const size_t n = paint.breakText(text, length, w, &m);
+
+        REPORTER_ASSERT_MESSAGE(reporter, n <= length, msg);
+        REPORTER_ASSERT_MESSAGE(reporter, m <= width, msg);
+
+        if (n == 0) {
+            REPORTER_ASSERT_MESSAGE(reporter, m == 0, msg);
+        } else {
+            // now assert that we're monotonic
+            if (n == nn) {
+                REPORTER_ASSERT_MESSAGE(reporter, m == mm, msg);
+            } else {
+                REPORTER_ASSERT_MESSAGE(reporter, n > nn, msg);
+                REPORTER_ASSERT_MESSAGE(reporter, m > mm, msg);
+            }
+        }
+        nn = n;
+        mm = m;
+    }
+}
+
+static void test_eq_measure_text(skiatest::Reporter* reporter,
+                                 const SkPaint& paint,
+                                 const char* msg) {
+    const char* text = "The ultimate measure of a man is not where he stands in moments of comfort "
+        "and convenience, but where he stands at times of challenge and controversy.";
+    const size_t length = strlen(text);
+    const SkScalar width = paint.measureText(text, length);
+
+    SkScalar mm;
+    const size_t length2 = paint.breakText(text, length, width, &mm);
+    REPORTER_ASSERT_MESSAGE(reporter, length2 == length, msg);
+    REPORTER_ASSERT_MESSAGE(reporter, mm == width, msg);
+}
+
+static void test_long_text(skiatest::Reporter* reporter,
+                           const SkPaint& paint,
+                           const char* msg) {
+    static const int kSize = 16 * 1024;
+    SkAutoMalloc block(kSize);
+    memset(block.get(), 'a', kSize - 1);
+    char* text = static_cast<char*>(block.get());
+    text[kSize - 1] = '\0';
+    const SkScalar width = paint.measureText(text, kSize);
+
+    SkScalar mm;
+    const size_t length = paint.breakText(text, kSize, width, &mm);
+    REPORTER_ASSERT_MESSAGE(reporter, length == kSize, msg);
+    REPORTER_ASSERT_MESSAGE(reporter, mm == width, msg);
+}
+
+DEF_TEST(PaintBreakText, reporter) {
+    SkPaint paint;
+    test_monotonic(reporter, paint, "default");
+    test_eq_measure_text(reporter, paint, "default");
+    test_long_text(reporter, paint, "default");
+    paint.setTextSize(SkIntToScalar(1 << 17));
+    test_monotonic(reporter, paint, "huge text size");
+    test_eq_measure_text(reporter, paint, "huge text size");
+    paint.setTextSize(0);
+    test_monotonic(reporter, paint, "zero text size");
+}