commit 875e13ca0990e32da9db639743a913efe77f7e89
author mtklein <mtklein@chromium.org> Sun Jun 19 05:28:33 2016 -0700
committer Commit bot <commit-bot@chromium.org> Sun Jun 19 05:28:33 2016 -0700
tree 4e700e765058e895930acc383d8b6e65cb309524
parent cc3a22b369e1a60fa2acf2987f2934baf7c4b198

Simplify mask/clip intersection, making sure to explicitly check for an empty mask.

Previously we were only asserting the mask wasn't empty, which isn't necessarily true when we're given pathological float coordinates like +Inf or NaN.

A local run of nanobench --match text_ was not able to show this is faster or slower.

This patch fixed this first Chrome bug on my desktop, and the second is probably a dupe.
BUG=chromium:619378,chromium:613912

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

Review-Url: https://codereview.chromium.org/2073873002

src/core/SkDraw.cpp

diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp
index 24fc906..9a0509c 100644
--- a/src/core/SkDraw.cpp
+++ b/src/core/SkDraw.cpp
@@ -1457,10 +1457,13 @@
     void operator()(const SkGlyph& glyph, SkPoint position, SkPoint rounding) {
         position += rounding;
         // Prevent glyphs from being drawn outside of or straddling the edge of device space.
-        if (position.fX > INT_MAX - (INT16_MAX + UINT16_MAX) ||
-            position.fX < INT_MIN - (INT16_MIN + 0 /*UINT16_MIN*/) ||
-            position.fY > INT_MAX - (INT16_MAX + UINT16_MAX) ||
-            position.fY < INT_MIN - (INT16_MIN + 0 /*UINT16_MIN*/)) {
+        // Comparisons written a little weirdly so that NaN coordinates are treated safely.
+        auto gt = [](float a, int b) { return !(a <= (float)b); };
+        auto lt = [](float a, int b) { return !(a >= (float)b); };
+        if (gt(position.fX, INT_MAX - (INT16_MAX + UINT16_MAX)) ||
+            lt(position.fX, INT_MIN - (INT16_MIN + 0 /*UINT16_MIN*/)) ||
+            gt(position.fY, INT_MAX - (INT16_MAX + UINT16_MAX)) ||
+            lt(position.fY, INT_MIN - (INT16_MIN + 0 /*UINT16_MIN*/))) {
             return;
         }
 
@@ -1476,6 +1479,7 @@
 
         SkMask mask;
         mask.fBounds.set(left, top, right, bottom);
+        SkASSERT(!mask.fBounds.isEmpty());
 
         if (fUseRegionToDraw) {
             SkRegion::Cliperator clipper(*fClip, mask.fBounds);

tests/DrawTextTest.cpp

diff --git a/tests/DrawTextTest.cpp b/tests/DrawTextTest.cpp
index f2da450..bf9bd39 100644
--- a/tests/DrawTextTest.cpp
+++ b/tests/DrawTextTest.cpp
@@ -11,8 +11,10 @@
 #include "SkPaint.h"
 #include "SkPoint.h"
 #include "SkRect.h"
+#include "SkSurface.h"
 #include "SkTypes.h"
 #include "Test.h"
+#include <math.h>
 
 static const SkColor bgColor = SK_ColorWHITE;
 
@@ -111,3 +113,21 @@
         }
     }
 }
+
+// Test drawing text at some unusual coordinates.
+// We measure success by not crashing or asserting.
+DEF_TEST(DrawText_weirdCoordinates, r) {
+    auto surface = SkSurface::MakeRasterN32Premul(10,10);
+    auto canvas = surface->getCanvas();
+
+    SkScalar oddballs[] = { 0.0f, (float)INFINITY, (float)NAN, 34359738368.0f };
+
+    for (auto x : oddballs) {
+        canvas->drawText("a", 1, +x, 0.0f, SkPaint());
+        canvas->drawText("a", 1, -x, 0.0f, SkPaint());
+    }
+    for (auto y : oddballs) {
+        canvas->drawText("a", 1, 0.0f, +y, SkPaint());
+        canvas->drawText("a", 1, 0.0f, -y, SkPaint());
+    }
+}