Add test for QuickFDot6Div

This test will catch our (1 << 10) bug (which should be 1 << 9)

BUG=skia:

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4986

Change-Id: I25b607d1535a647284cee3b304a6f567f389e7f6
Reviewed-on: https://skia-review.googlesource.com/4986
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>

diff --git a/gn/core.gni b/gn/core.gni
index d7f1156..fcc8de1 100644 (file)
--- a/gn/core.gni
+++ b/gn/core.gni
@@ -121,7 +121,7 @@ skia_core_sources = [
   "$_src/core/SkEmptyShader.h",
   "$_src/core/SkEndian.h",
   "$_src/core/SkAnalyticEdge.cpp",
-  "$_src/core/SkAAAConstants.h",
+  "$_src/core/SkFDot6Constants.h",
   "$_src/core/SkEdge.cpp",
   "$_src/core/SkEdge.h",
   "$_src/core/SkFDot6.h",

diff --git a/src/core/SkAnalyticEdge.cpp b/src/core/SkAnalyticEdge.cpp
index 17e3793..a9cbc50 100644 (file)
--- a/src/core/SkAnalyticEdge.cpp
+++ b/src/core/SkAnalyticEdge.cpp
@@ -9,37 +9,6 @@
 #include "SkAnalyticEdge.h"
 #include "SkFDot6.h"
 #include "SkMathPriv.h"
-#include "SkAAAConstants.h"
-
-class QuickFDot6Inverse {
-private:
-    static constexpr const SkFDot6* table = gFDot6INVERSE + kInverseTableSize;
-public:
-    inline static SkFixed Lookup(SkFDot6 x) {
-        SkASSERT(SkAbs32(x) < kInverseTableSize);
-        return table[x];
-    }
-};
-
-static inline SkFixed quickSkFDot6Div(SkFDot6 a, SkFDot6 b) {
-    // Max inverse of b is 2^6 which is 2^22 in SkFixed format.
-    // Hence the safe value of abs(a) should be less than 2^9.
-    if (SkAbs32(b) < kInverseTableSize && SkAbs32(a) < (1 << 9)) {
-        SkASSERT((int64_t)a * QuickFDot6Inverse::Lookup(b) <= SK_MaxS32
-                && (int64_t)a * QuickFDot6Inverse::Lookup(b) >= SK_MinS32);
-        SkFixed ourAnswer = (a * QuickFDot6Inverse::Lookup(b)) >> 6;
-        #ifdef SK_DEBUG
-        SkFixed directAnswer = SkFDot6Div(a, b);
-        SkASSERT(
-            (directAnswer == 0 && ourAnswer == 0) ||
-            SkFixedDiv(SkAbs32(directAnswer - ourAnswer), SkAbs32(directAnswer)) <= 1 << 10
-        );
-        #endif
-        return ourAnswer;
-    } else {
-        return SkFDot6Div(a, b);
-    }
-}
 
 // This will become a bottleneck for small ovals rendering if we call SkFixedDiv twice here.
 // Therefore, we'll let the outter function compute the slope once and send in the value.
@@ -74,7 +43,7 @@ bool SkAnalyticEdge::updateLine(SkFixed x0, SkFixed y0, SkFixed x1, SkFixed y1,
                   ? SK_MaxS32
                   : absSlope < kInverseTableSize
                     ? QuickFDot6Inverse::Lookup(absSlope)
-                    : SkAbs32(quickSkFDot6Div(dy, dx));
+                    : SkAbs32(QuickSkFDot6Div(dy, dx));
 
     return true;
 }
@@ -136,7 +105,7 @@ bool SkAnalyticQuadraticEdge::updateQuadratic() {
         {
             newx    = oldx + (dx >> shift);
             newy    = oldy + (dy >> shift);
-            slope = dy >> 10 > 0 ? quickSkFDot6Div(dx >> 10, dy >> 10) : SK_MaxS32;
+            slope = dy >> 10 > 0 ? QuickSkFDot6Div(dx >> 10, dy >> 10) : SK_MaxS32;
             if (SkAbs32(dy) >= SK_Fixed1 * 2) { // only snap when dy is large enough
                 newSnappedY = SkTMin<SkFixed>(fQEdge.fQLastY, SkFixedRoundToFixed(newy));
                 newSnappedX = newx + SkFixedMul(slope, newSnappedY - newy);
@@ -154,7 +123,7 @@ bool SkAnalyticQuadraticEdge::updateQuadratic() {
             newSnappedY = newy;
             newSnappedX = newx;
             slope = (newSnappedY - fSnappedY) >> 10
-                    ? quickSkFDot6Div((newx - fSnappedX) >> 10, (newy - fSnappedY) >> 10)
+                    ? QuickSkFDot6Div((newx - fSnappedX) >> 10, (newy - fSnappedY) >> 10)
                     : SK_MaxS32;
         }
         if (slope < SK_MaxS32) {

diff --git a/src/core/SkFDot6.h b/src/core/SkFDot6.h
index 726aa2e..b9a5c2a 100644 (file)
--- a/src/core/SkFDot6.h
+++ b/src/core/SkFDot6.h
@@ -75,4 +75,41 @@ inline SkFixed SkFDot6Div(SkFDot6 a, SkFDot6 b) {
     }
 }
 
+#include "SkFDot6Constants.h"
+
+class QuickFDot6Inverse {
+private:
+    static constexpr const SkFDot6* table = gFDot6INVERSE + kInverseTableSize;
+public:
+    inline static SkFixed Lookup(SkFDot6 x) {
+        SkASSERT(SkAbs32(x) < kInverseTableSize);
+        return table[x];
+    }
+};
+
+static inline SkFixed QuickSkFDot6Div(SkFDot6 a, SkFDot6 b) {
+    const int kMinBits = 3;  // abs(b) should be at least (1 << kMinBits) for quick division
+    const int kMaxBits = 31; // Number of bits available in signed int
+    // Given abs(b) <= (1 << kMinBits), the inverse of abs(b) is at most 1 << (22 - kMinBits) in
+    // SkFixed format. Hence abs(a) should be less than kMaxAbsA
+    const int kMaxAbsA = 1 << (kMaxBits - (22 - kMinBits));
+    SkFDot6 abs_a = SkAbs32(a);
+    SkFDot6 abs_b = SkAbs32(b);
+    if (abs_b >= (1 << kMinBits) && abs_b < kInverseTableSize && abs_a < kMaxAbsA) {
+        SkASSERT((int64_t)a * QuickFDot6Inverse::Lookup(b) <= SK_MaxS32
+                && (int64_t)a * QuickFDot6Inverse::Lookup(b) >= SK_MinS32);
+        SkFixed ourAnswer = (a * QuickFDot6Inverse::Lookup(b)) >> 6;
+        #ifdef SK_DEBUG
+        SkFixed directAnswer = SkFDot6Div(a, b);
+        SkASSERT(
+            (directAnswer == 0 && ourAnswer == 0) ||
+            SkFixedDiv(SkAbs32(directAnswer - ourAnswer), SkAbs32(directAnswer)) <= 1 << 10
+        );
+        #endif
+        return ourAnswer;
+    } else {
+        return SkFDot6Div(a, b);
+    }
+}
+
 #endif

diff --git a/src/core/SkAAAConstants.h b/src/core/SkFDot6Constants.h
similarity index 96%
rename from src/core/SkAAAConstants.h
rename to src/core/SkFDot6Constants.h
index e008e4b..945bee5 100644 (file)
--- a/src/core/SkAAAConstants.h
+++ b/src/core/SkFDot6Constants.h
@@ -10,26 +10,6 @@
 
 static const int kInverseTableSize = 1024; // SK_FDot6One * 16
 
-/*
-The following table is generated by:
-
-struct FDot6InverseTable {
-    SkFixed storage[kInverseTableSize * 2];
-    SkFixed* table = storage + kInverseTableSize;
-
-    FDot6InverseTable() {
-        SkDebugf("static const int gFDot6INVERSE[] = {");
-        for (SkFDot6 i=-kInverseTableSize; i<kInverseTableSize; i++) {
-            if (i != 0) {
-                table[i] = SkFDot6Div(SK_FDot6One, i);
-            }
-            SkDebugf("%d, ", table[i]);
-        }
-        SkDebugf("}\n");
-    }
-};
-*/
-
 static constexpr const SkFDot6 gFDot6INVERSE[kInverseTableSize * 2] = {
     -4096, -4100, -4104, -4108, -4112, -4116, -4120, -4124, -4128, -4132, -4136,
     -4140, -4144, -4148, -4152, -4156, -4161, -4165, -4169, -4173, -4177, -4181,
@@ -216,5 +196,4 @@ static constexpr const SkFDot6 gFDot6INVERSE[kInverseTableSize * 2] = {
     4124, 4120, 4116, 4112, 4108, 4104, 4100
 };
 
-
 #endif

diff --git a/tests/MathTest.cpp b/tests/MathTest.cpp
index 35b4136..0bd3cf5 100644 (file)
--- a/tests/MathTest.cpp
+++ b/tests/MathTest.cpp
@@ -9,6 +9,7 @@
 
 #include "SkColorPriv.h"
 #include "SkEndian.h"
+#include "SkFDot6.h"
 #include "SkFixed.h"
 #include "SkFloatBits.h"
 #include "SkFloatingPoint.h"
@@ -36,6 +37,38 @@ static void test_clz(skiatest::Reporter* reporter) {
     }
 }
 
+static void test_quick_div(skiatest::Reporter* reporter) {
+    /*
+    The inverse table is generated by turning on SkDebugf in the following test code
+    */
+    SkFixed storage[kInverseTableSize * 2];
+    SkFixed* table = storage + kInverseTableSize;
+
+    // SkDebugf("static const int gFDot6INVERSE[] = {");
+    for (SkFDot6 i=-kInverseTableSize; i<kInverseTableSize; i++) {
+        if (i != 0) {
+            table[i] = SkFDot6Div(SK_FDot6One, i);
+            REPORTER_ASSERT(reporter, table[i] == gFDot6INVERSE[i + kInverseTableSize]);
+        }
+        // SkDebugf("%d, ", table[i]);
+    }
+    // SkDebugf("}\n");
+
+
+    for (SkFDot6 a = -1024; a <= 1024; a++) {
+        for (SkFDot6 b = -1024; b <= 1024; b++) {
+            if (b != 0) {
+                SkFixed ourAnswer = QuickSkFDot6Div(a, b);
+                SkFixed directAnswer = SkFDot6Div(a, b);
+                REPORTER_ASSERT(reporter,
+                    (directAnswer == 0 && ourAnswer == 0) ||
+                    SkFixedDiv(SkAbs32(directAnswer - ourAnswer), SkAbs32(directAnswer)) <= 1 << 10
+                );
+            }
+        }
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 static float sk_fsel(float pred, float result_ge, float result_lt) {
@@ -576,6 +609,7 @@ DEF_TEST(Math, reporter) {
 
     test_muldivround(reporter);
     test_clz(reporter);
+    test_quick_div(reporter);
 }
 
 template <typename T> struct PairRec {