SkPDF: fix scalar serialization

src/pdf/SkPDFUtils.cpp

Index: src/pdf/SkPDFUtils.cpp
diff --git a/src/pdf/SkPDFUtils.cpp b/src/pdf/SkPDFUtils.cpp
index 108f2c10abd56e6d0ae94b263c89c5c9a5d34dd4..30d6ee7d68899b7f3415352abb668d1966c432fb 100644
--- a/src/pdf/SkPDFUtils.cpp
+++ b/src/pdf/SkPDFUtils.cpp
@@ -16,6 +16,8 @@
 #include "SkString.h"
 #include "SkPDFTypes.h"
 
+#include <cmath>
+
 //static
 SkPDFArray* SkPDFUtils::RectToArray(const SkRect& rect) {
     SkPDFArray* result = new SkPDFArray();
@@ -254,51 +256,120 @@ void SkPDFUtils::ApplyPattern(int objectIndex, SkWStream* content) {
 }
 
 void SkPDFUtils::AppendScalar(SkScalar value, SkWStream* stream) {
-    // The range of reals in PDF/A is the same as SkFixed: +/- 32,767 and
-    // +/- 1/65,536 (though integers can range from 2^31 - 1 to -2^31).
-    // When using floats that are outside the whole value range, we can use
-    // integers instead.
-
-#if !defined(SK_ALLOW_LARGE_PDF_SCALARS)
-    if (value > 32767 || value < -32767) {
-        stream->writeDecAsText(SkScalarRoundToInt(value));
-        return;
-    }
+    char result[kMaximumFloatDecimalLength];
+    size_t len = SkPDFUtils::FloatToDecimal(SkScalarToFloat(value), result);
+    SkASSERT(len < kMaximumFloatDecimalLength);
+    stream->write(result, len);
+}
+
+/** Write a string into result, includeing a terminating '\0' (for

[tomhudson, 2016/02/24 20:10:56]

nit: including

+    unit testing).  Return strlen(result) (for SkWStream::write) The
+    resulting string will be in the form /[-]?([0-9]*.)?[0-9]+/ and
+    sscanf(result, "%f", &x) will return the original value iff the
+    value is finite. This function accepts all possible input values.
+
+    Motivation: "PDF does not support [numbers] in exponential format
+    (such as 6.02e23)."  Otherwise, this function would rely on a
+    sprintf-type function from the standard library. */
+size_t SkPDFUtils::FloatToDecimal(float value,
+                                  char result[kMaximumFloatDecimalLength]) {
+    /* The longest result is -FLT_MIN.
+       We serialize it as "-.0000000000000000000000000000000000000117549435"
+       which has 48 characters plus a terminating '\0'. */
+
+    /* section C.1 of the PDF1.4 spec (http://goo.gl/0SCswJ) says that
+       most PDF rasterizers will use fixed-point scalars that lack the
+       dynamic range of floats.  Even if this is the case, I want to
+       serialize these (uncommon) very small and very large scalar
+       values with enough precision to allow a floating-point
+       rasterizer to read them in with perfect accuracy.
+       Experimentally, rasterizers such as pdfium do seem to benefit
+       from this.  Rasterizers that rely on fixed-point scalars should
+       gracefully ignore these values that they can not parse. */
+    char* output = &result[0];
+    const char* const end = &result[kMaximumFloatDecimalLength - 1];
+    // subtract one to leave space for '\0'.
 
-    char buffer[SkStrAppendScalar_MaxSize];
-    char* end = SkStrAppendFixed(buffer, SkScalarToFixed(value));
-    stream->write(buffer, end - buffer);
-    return;
-#endif  // !SK_ALLOW_LARGE_PDF_SCALARS
-
-#if defined(SK_ALLOW_LARGE_PDF_SCALARS)
-    // Floats have 24bits of significance, so anything outside that range is
-    // no more precise than an int. (Plus PDF doesn't support scientific
-    // notation, so this clamps to SK_Max/MinS32).
-    if (value > (1 << 24) || value < -(1 << 24)) {
-        stream->writeDecAsText(value);
-        return;
+    /* This function is written to accept any possible input value,
+       including non-finite values such as INF and NAN.  In that case,
+       we ignore value-correctness and and output a syntacticly-valid
+       number. */
+    if (value == SK_FloatInfinity) {
+        value = FLT_MAX;  // nearest finite float.
     }
-    // Continue to enforce the PDF limits for small floats.
-    if (value < 1.0f/65536 && value > -1.0f/65536) {
-        stream->writeDecAsText(0);
-        return;
+    if (value == SK_FloatNegativeInfinity) {
+        value = -FLT_MAX;  // nearest finite float.
     }
-    // SkStrAppendFloat might still use scientific notation, so use snprintf
-    // directly..
-    static const int kFloat_MaxSize = 19;
-    char buffer[kFloat_MaxSize];
-    int len = SNPRINTF(buffer, kFloat_MaxSize, "%#.8f", value);
-    // %f always prints trailing 0s, so strip them.
-    for (; buffer[len - 1] == '0' && len > 0; len--) {
-        buffer[len - 1] = '\0';
+    if (!std::isfinite(value) || value == 0.0f) {
+        // NAN is unsupported in PDF.  Always output a valid number.
+        // Also catch zero here, as a special case.
+        *output++ = '0';
+        *output = '\0';
+        return output - result;
     }
-    if (buffer[len - 1] == '.') {
-        buffer[len - 1] = '\0';
+    // Inspired by:
+    // http://www.exploringbinary.com/quick-and-dirty-floating-point-to-decimal-conversion/
+
+    if (value < 0.0) {
+        *output++ = '-';
+        value = -value;
+    }
+    SkASSERT(value >= 0.0f);
+
+    // Must use double math to keep precision right.
+    double intPart;
+    double fracPart = std::modf(static_cast<double>(value), &intPart);
+    SkASSERT(intPart + fracPart == static_cast<double>(value));
+    size_t significantDigits = 0;
+    const size_t maxSignificantDigits = 9;
+    // Any fewer significant digits loses precision.  The unit test
+    // checks round-trip correctness.
+    SkASSERT(intPart >= 0.0 && fracPart >= 0.0);  // negative handled already.
+    SkASSERT(intPart > 0.0 || fracPart > 0.0);  // zero already caught.
+    if (intPart > 0.0) {
+        // put the intPart digits onto a stack for later reversal.
+        char reversed[1 + FLT_MAX_10_EXP];  // 39 == 1 + FLT_MAX_10_EXP
+        // the largest integer part is FLT_MAX; it has 39 decimal digits.
+        size_t reversedIndex = 0;
+        do {
+            SkASSERT(reversedIndex < sizeof(reversed));
+            int digit = static_cast<int>(std::fmod(intPart, 10.0));
+            SkASSERT(digit >= 0 && digit <= 9);
+            reversed[reversedIndex++] = '0' + digit;
+            intPart = std::floor(intPart / 10.0);
+        } while (intPart > 0.0);
+        significantDigits = reversedIndex;
+        SkASSERT(reversedIndex <= sizeof(reversed));
+        SkASSERT(output + reversedIndex <= end);
+        while (reversedIndex-- > 0) {  // pop from stack, append to result
+            *output++ = reversed[reversedIndex];
+        }
+    }
+    if (fracPart > 0 && significantDigits < maxSignificantDigits) {
+        *output++ = '.';
+        SkASSERT(output <= end);
+        do {
+            fracPart = std::modf(fracPart * 10.0, &intPart);
+            int digit = static_cast<int>(intPart);
+            SkASSERT(digit >= 0 && digit <= 9);
+            *output++ = '0' + digit;
+            SkASSERT(output <= end);
+            if (digit > 0 || significantDigits > 0) {
+                // start counting significantDigits after first non-zero digit.
+                ++significantDigits;
+            }
+        } while (fracPart > 0.0
+                 && significantDigits < maxSignificantDigits
+                 && output < end);
+        // When fracPart == 0, additional digits will be zero.
+        // When significantDigits == maxSignificantDigits, we can stop.
+        // when output == end, we have filed the string.
+        // Note: denormalized numbers will not have the same number of
+        // significantDigits, but do not need them to round-trip.
     }
-    stream->writeText(buffer);
-    return;
-#endif  // SK_ALLOW_LARGE_PDF_SCALARS
+    SkASSERT(output <= end);
+    *output = '\0';
+    return output - result;
 }
 
 SkString SkPDFUtils::FormatString(const char* cin, size_t len) {