Improve performance of writing path floats.

This CL copies the SkPDF code to convert floats into strings when
writing back to PDF files.

Change-Id: I8f8af3924a07aa67f93b9d951af1eef5d2c705db
Reviewed-on: https://pdfium-review.googlesource.com/21990
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Hal Canary <halcanary@chromium.org>

8 files changed, 245 insertions, 12 deletions

diff --git a/BUILD.gn b/BUILD.gn
index bdadebd400..f97a195413 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -200,6 +200,7 @@ jumbo_static_library("pdfium") {
     ":pwl",
     "third_party:bigint",
     "third_party:pdfium_base",
+    "third_party:skia_shared",
   ]
 
   public_deps = [
diff --git a/core/fpdfapi/edit/DEPS b/core/fpdfapi/edit/DEPS
new file mode 100644
index 0000000000..8a4c38c44b
--- /dev/null
+++ b/core/fpdfapi/edit/DEPS
@@ -0,0 +1,3 @@
+include_rules = [
+  '+third_party/skia_shared',
+]
diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
index 8a08a849cc..a2d4d4f125 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
@@ -25,6 +25,7 @@
 #include "core/fpdfapi/parser/cpdf_reference.h"
 #include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fpdfapi/parser/fpdf_parser_decode.h"
+#include "third_party/skia_shared/SkFloatToDecimal.h"
 
 namespace {
 
@@ -221,7 +222,13 @@ void CPDF_PageContentGenerator::ProcessPath(std::ostringstream* buf,
     for (size_t i = 0; i < pPoints.size(); i++) {
       if (i > 0)
         *buf << " ";
-      *buf << pPoints[i].m_Point.x << " " << pPoints[i].m_Point.y;
+
+      char buffer[kMaximumSkFloatToDecimalLength];
+      unsigned size = SkFloatToDecimal(pPoints[i].m_Point.x, buffer);
+      buf->write(buffer, size) << " ";
+      size = SkFloatToDecimal(pPoints[i].m_Point.y, buffer);
+      buf->write(buffer, size);
+
       FXPT_TYPE pointType = pPoints[i].m_Type;
       if (pointType == FXPT_TYPE::MoveTo) {
         *buf << " m";
diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
index a0db869410..403bd6ab03 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
@@ -103,8 +103,9 @@ TEST_F(CPDF_PageContentGeneratorTest, ProcessPath) {
   std::ostringstream buf;
   TestProcessPath(&generator, &buf, pPathObj.get());
   EXPECT_EQ(
-      "q 1 0 0 1 0 0 cm 3.102 4.67 m 5.45 0.29 l 4.24 3.15 4.65 2.98 3.456 0.24"
-      " c 10.6 11.15 l 11 12.5 l 11.46 12.67 11.84 12.96 12 13.64 c h f Q\n",
+      "q 1 0 0 1 0 0 cm 3.102 4.6700001 m 5.4499998 .28999999 l 4.2399998 "
+      "3.1500001 4.65 2.98 3.456 0.24 c 10.6000004 11.1499996 l 11 12.5 "
+      "l 11.46 12.6700001 11.84 12.96 12 13.64 c h f Q\n",
       ByteString(buf));
 }
 
@@ -312,8 +313,8 @@ TEST_F(CPDF_PageContentGeneratorTest, ProcessFormWithPath) {
   pDoc->CreateNewDoc();
   auto pDict = pdfium::MakeUnique<CPDF_Dictionary>();
   const char content[] =
-      "q 1 0 0 1 0 0 cm 3.102 4.67 m 5.45 0.29 l 4.24 3.15 4.65 2.98 3.456 "
-      "0.24 c 3.102 4.67 l h f Q\n";
+      "q 1 0 0 1 0 0 cm 3.102 4.6700001 m 5.4500012 .28999999 "
+      "l 4.2399998 3.1499999 4.65 2.98 3.456 0.24 c 3.102 4.6700001 l h f Q\n";
   size_t buf_len = FX_ArraySize(content);
   std::unique_ptr<uint8_t, FxFreeDeleter> buf(FX_Alloc(uint8_t, buf_len));
   memcpy(buf.get(), content, buf_len);
@@ -329,5 +330,5 @@ TEST_F(CPDF_PageContentGeneratorTest, ProcessFormWithPath) {
   CPDF_PageContentGenerator generator(pTestForm.get());
   std::ostringstream process_buf;
   generator.ProcessPageObjects(&process_buf);
-  EXPECT_EQ(content, ByteString(process_buf));
+  EXPECT_STREQ(content, ByteString(process_buf).c_str());
 }
diff --git a/fpdfsdk/fpdfannot_embeddertest.cpp b/fpdfsdk/fpdfannot_embeddertest.cpp
index 310d042885..70e184dd41 100644
--- a/fpdfsdk/fpdfannot_embeddertest.cpp
+++ b/fpdfsdk/fpdfannot_embeddertest.cpp
@@ -544,14 +544,14 @@ TEST_F(FPDFAnnotEmbeddertest, RemoveAnnotation) {
 TEST_F(FPDFAnnotEmbeddertest, AddAndModifyPath) {
 #if _FX_PLATFORM_ == _FX_PLATFORM_APPLE_
   const char md5_original[] = "c35408717759562d1f8bf33d317483d2";
-  const char md5_modified_path[] = "cf3cea74bd46497520ff6c4d1ea228c8";
-  const char md5_two_paths[] = "e8994452fc4385337bae5522354e10ff";
-  const char md5_new_annot[] = "ee5372b31fede117fc83b9384598aa25";
+  const char md5_modified_path[] = "873b92ea83ccf006e58415d866ce145b";
+  const char md5_two_paths[] = "6f1f1c91f50240e9cc9d7c87c48b93a7";
+  const char md5_new_annot[] = "078bf58f939645ac305854f31ee9a828";
 #else
   const char md5_original[] = "964f89bbe8911e540a465cf1a64b7f7e";
-  const char md5_modified_path[] = "3f77b88ce6048e08e636c9a03921b2e5";
-  const char md5_two_paths[] = "bffbf5ecd15862b9fe553c795400ff8e";
-  const char md5_new_annot[] = "e020534c7eeea76be537c70d6e359a40";
+  const char md5_modified_path[] = "5a4a6091cff648a4ece3ce7e245e3e38";
+  const char md5_two_paths[] = "d6e4072a4415cfc6ec17201fb6be0ee0";
+  const char md5_new_annot[] = "fc338b97bf66a656916c6198697a8a28";
 #endif
 
   // Open a file with two annotations and load its first page.
diff --git a/third_party/BUILD.gn b/third_party/BUILD.gn
index 1cfee77c60..fc6a7e50c8 100644
--- a/third_party/BUILD.gn
+++ b/third_party/BUILD.gn
@@ -12,6 +12,7 @@ group("third_party") {
   deps = [
     ":bigint",
     ":pdfium_base",
+    ":skia_shared",
   ]
   if (pdf_bundle_freetype) {
     deps += [ ":fx_freetype" ]
@@ -568,3 +569,15 @@ jumbo_source_set("pdfium_base") {
     "build/build_config.h",
   ]
 }
+
+jumbo_source_set("skia_shared") {
+  configs -= [ "//build/config/compiler:chromium_code" ]
+  configs += [
+    "//build/config/compiler:no_chromium_code",
+    ":pdfium_third_party_config",
+  ]
+  sources = [
+    "skia_shared/SkFloatToDecimal.cpp",
+    "skia_shared/SkFloatToDecimal.h",
+  ]
+}
diff --git a/third_party/skia_shared/SkFloatToDecimal.cpp b/third_party/skia_shared/SkFloatToDecimal.cpp
new file mode 100644
index 0000000000..ac24a11e91
--- /dev/null
+++ b/third_party/skia_shared/SkFloatToDecimal.cpp
@@ -0,0 +1,174 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkFloatToDecimal.h"
+
+#include <cfloat>
+#include <climits>
+#include <cmath>
+
+//#include "SkTypes.h"
+#include <cassert>
+#define SkASSERT assert
+
+// Return pow(10.0, e), optimized for common cases.
+static double pow10(int e) {
+    switch (e) {
+        case 0:  return 1.0;  // common cases
+        case 1:  return 10.0;
+        case 2:  return 100.0;
+        case 3:  return 1e+03;
+        case 4:  return 1e+04;
+        case 5:  return 1e+05;
+        case 6:  return 1e+06;
+        case 7:  return 1e+07;
+        case 8:  return 1e+08;
+        case 9:  return 1e+09;
+        case 10: return 1e+10;
+        case 11: return 1e+11;
+        case 12: return 1e+12;
+        case 13: return 1e+13;
+        case 14: return 1e+14;
+        case 15: return 1e+15;
+        default:
+            if (e > 15) {
+                double value = 1e+15;
+                while (e-- > 15) { value *= 10.0; }
+                return value;
+            } else {
+                SkASSERT(e < 0);
+                double value = 1.0;
+                while (e++ < 0) { value /= 10.0; }
+                return value;
+            }
+    }
+}
+
+/** Write a string into result, includeing a terminating '\0' (for
+    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. */
+unsigned SkFloatToDecimal(float value, char result[kMaximumSkFloatToDecimalLength]) {
+    /* The longest result is -FLT_MIN.
+       We serialize it as "-.0000000000000000000000000000000000000117549435"
+       which has 48 characters plus a terminating '\0'. */
+
+    static_assert(kMaximumSkFloatToDecimalLength == 49, "");
+    // 3 = '-', '.', and '\0' characters.
+    // 9 = number of significant digits
+    // abs(FLT_MIN_10_EXP) = number of zeros in FLT_MIN
+    static_assert(kMaximumSkFloatToDecimalLength == 3 + 9 - FLT_MIN_10_EXP, "");
+
+    /* 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[kMaximumSkFloatToDecimalLength - 1];
+    // subtract one to leave space for '\0'.
+
+    /* 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 == INFINITY) {
+        value = FLT_MAX;  // nearest finite float.
+    }
+    if (value == -INFINITY) {
+        value = -FLT_MAX;  // nearest finite float.
+    }
+    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 static_cast<unsigned>(output - result);
+    }
+    if (value < 0.0) {
+        *output++ = '-';
+        value = -value;
+    }
+    SkASSERT(value >= 0.0f);
+
+    int binaryExponent;
+    (void)std::frexp(value, &binaryExponent);
+    static const double kLog2 = 0.3010299956639812;  // log10(2.0);
+    int decimalExponent = static_cast<int>(std::floor(kLog2 * binaryExponent));
+    int decimalShift = decimalExponent - 8;
+    double power = pow10(-decimalShift);
+    SkASSERT(value * power <= (double)INT_MAX);
+    int d = static_cast<int>(value * power + 0.5);
+    // SkASSERT(value == (float)(d * pow(10.0, decimalShift)));
+    SkASSERT(d <= 999999999);
+    if (d > 167772159) {  // floor(pow(10,1+log10(1<<24)))
+       // need one fewer decimal digits for 24-bit precision.
+       decimalShift = decimalExponent - 7;
+       // SkASSERT(power * 0.1 = pow10(-decimalShift));
+       // recalculate to get rounding right.
+       d = static_cast<int>(value * (power * 0.1) + 0.5);
+       SkASSERT(d <= 99999999);
+    }
+    while (d % 10 == 0) {
+        d /= 10;
+        ++decimalShift;
+    }
+    SkASSERT(d > 0);
+    // SkASSERT(value == (float)(d * pow(10.0, decimalShift)));
+    unsigned char buffer[9]; // decimal value buffer.
+    int bufferIndex = 0;
+    do {
+        buffer[bufferIndex++] = d % 10;
+        d /= 10;
+    } while (d != 0);
+    SkASSERT(bufferIndex <= (int)sizeof(buffer) && bufferIndex > 0);
+    if (decimalShift >= 0) {
+        do {
+            --bufferIndex;
+            *output++ = '0' + buffer[bufferIndex];
+        } while (bufferIndex);
+        for (int i = 0; i < decimalShift; ++i) {
+            *output++ = '0';
+        }
+    } else {
+        int placesBeforeDecimal = bufferIndex + decimalShift;
+        if (placesBeforeDecimal > 0) {
+            while (placesBeforeDecimal-- > 0) {
+                --bufferIndex;
+                *output++ = '0' + buffer[bufferIndex];
+            }
+            *output++ = '.';
+        } else {
+            *output++ = '.';
+            int placesAfterDecimal = -placesBeforeDecimal;
+            while (placesAfterDecimal-- > 0) {
+                *output++ = '0';
+            }
+        }
+        while (bufferIndex > 0) {
+            --bufferIndex;
+            *output++ = '0' + buffer[bufferIndex];
+            if (output == end) {
+                break;  // denormalized: don't need extra precision.
+                // Note: denormalized numbers will not have the same number of
+                // significantDigits, but do not need them to round-trip.
+            }
+        }
+    }
+    SkASSERT(output <= end);
+    *output = '\0';
+    return static_cast<unsigned>(output - result);
+}
diff --git a/third_party/skia_shared/SkFloatToDecimal.h b/third_party/skia_shared/SkFloatToDecimal.h
new file mode 100644
index 0000000000..ac1042dbfb
--- /dev/null
+++ b/third_party/skia_shared/SkFloatToDecimal.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkFloatToDecimal_DEFINED
+#define SkFloatToDecimal_DEFINED
+
+constexpr unsigned kMaximumSkFloatToDecimalLength = 49;
+
+/** \fn SkFloatToDecimal
+    Convert a float into a decimal string.
+
+    The resulting string will be in the form `[-]?([0-9]*\.)?[0-9]+` (It does
+    not use scientific notation.) and `sscanf(output, "%f", &x)` will return
+    the original value if the value is finite. This function accepts all
+    possible input values.
+
+    INFINITY and -INFINITY are rounded to FLT_MAX and -FLT_MAX.
+
+    NAN values are converted to 0.
+
+    This function will always add a terminating '\0' to the output.
+
+    @param value  Any floating-point number
+    @param output The buffer to write the string into.  Must be non-null.
+
+    @return strlen(output)
+*/
+unsigned SkFloatToDecimal(float value, char output[kMaximumSkFloatToDecimalLength]);
+
+#endif  // SkFloatToDecimal_DEFINED