From 844d79e853074c99b7e5e64e051f1e1236c1723e Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Fri, 16 Feb 2018 03:46:26 +0000 Subject: 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 Reviewed-by: Lei Zhang Reviewed-by: Hal Canary --- BUILD.gn | 1 + core/fpdfapi/edit/DEPS | 3 + core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp | 9 +- .../edit/cpdf_pagecontentgenerator_unittest.cpp | 11 +- fpdfsdk/fpdfannot_embeddertest.cpp | 12 +- third_party/BUILD.gn | 13 ++ third_party/skia_shared/SkFloatToDecimal.cpp | 174 +++++++++++++++++++++ third_party/skia_shared/SkFloatToDecimal.h | 34 ++++ 8 files changed, 245 insertions(+), 12 deletions(-) create mode 100644 core/fpdfapi/edit/DEPS create mode 100644 third_party/skia_shared/SkFloatToDecimal.cpp create mode 100644 third_party/skia_shared/SkFloatToDecimal.h 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(); 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 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 +#include +#include + +//#include "SkTypes.h" +#include +#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(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(std::floor(kLog2 * binaryExponent)); + int decimalShift = decimalExponent - 8; + double power = pow10(-decimalShift); + SkASSERT(value * power <= (double)INT_MAX); + int d = static_cast(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(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(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 -- cgit v1.2.3