Serialize floats in page content to avoid exponential representation
CPDF_PageContentGenerator class uses std::ostringstream to serialize
floats to string while generating the content stream. Float values
close to 0 (e.g., 0.00000001) and large numbers (e.g., 10000000) get
converted to their exponential representation (e.g., 1e-7 and 1e+7
respectively). PDF specification does not recognize exponential format
representation for floats.
CPDF_PageContentGenerator is invoked when FPDFPage_GenerateContent API
is called. Existing code addressed a specific case in path generation
code by using SkFloatToDecimal to serialize floats. The fix in this
CL is to re-factor that code into a utility function and use it in
other places except for the color operators. Code that serializes the
color operators (rg and RG) and their operands, converts an integer in
CPDF_Color::GetRGB into float dividing by 256. Thus the floating point
serialization anomaly wasn't observed for these operators.
A new unit-test CPDF_PageContentGeneratorTest::BUG_937 was added to
test the product code changes. Existing unit-tests and embedder-tests
needed to be updated to reflect the serialization format.
R=thestig@chromium.org
Bug: pdfium:937
Change-Id: I38f066a0ad074cd0d68d6b7620fb581d04ce2c16
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/52070
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
index c5f9568..d2c8bf2 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
@@ -39,9 +39,20 @@
namespace {
+std::ostream& WriteFloat(std::ostream& stream, float value) {
+ char buffer[pdfium::skia::kMaximumSkFloatToDecimalLength];
+ unsigned size = pdfium::skia::SkFloatToDecimal(value, buffer);
+ stream.write(buffer, size);
+ return stream;
+}
+
std::ostream& operator<<(std::ostream& ar, const CFX_Matrix& matrix) {
- ar << matrix.a << " " << matrix.b << " " << matrix.c << " " << matrix.d << " "
- << matrix.e << " " << matrix.f;
+ WriteFloat(ar, matrix.a) << " ";
+ WriteFloat(ar, matrix.b) << " ";
+ WriteFloat(ar, matrix.c) << " ";
+ WriteFloat(ar, matrix.d) << " ";
+ WriteFloat(ar, matrix.e) << " ";
+ WriteFloat(ar, matrix.f);
return ar;
}
@@ -358,19 +369,17 @@
const auto& pPoints = pPathObj->path().GetPoints();
if (pPathObj->path().IsRect()) {
CFX_PointF diff = pPoints[2].m_Point - pPoints[0].m_Point;
- *buf << pPoints[0].m_Point.x << " " << pPoints[0].m_Point.y << " " << diff.x
- << " " << diff.y << " re";
+ WriteFloat(*buf, pPoints[0].m_Point.x) << " ";
+ WriteFloat(*buf, pPoints[0].m_Point.y) << " ";
+ WriteFloat(*buf, diff.x) << " ";
+ WriteFloat(*buf, diff.y) << " re";
} else {
for (size_t i = 0; i < pPoints.size(); i++) {
if (i > 0)
*buf << " ";
- char buffer[pdfium::skia::kMaximumSkFloatToDecimalLength];
- unsigned size =
- pdfium::skia::SkFloatToDecimal(pPoints[i].m_Point.x, buffer);
- buf->write(buffer, size) << " ";
- size = pdfium::skia::SkFloatToDecimal(pPoints[i].m_Point.y, buffer);
- buf->write(buffer, size);
+ WriteFloat(*buf, pPoints[i].m_Point.x) << " ";
+ WriteFloat(*buf, pPoints[i].m_Point.y);
FXPT_TYPE pointType = pPoints[i].m_Type;
if (pointType == FXPT_TYPE::MoveTo) {
@@ -386,9 +395,11 @@
*buf << " h";
break;
}
- *buf << " " << pPoints[i + 1].m_Point.x << " "
- << pPoints[i + 1].m_Point.y << " " << pPoints[i + 2].m_Point.x
- << " " << pPoints[i + 2].m_Point.y << " c";
+ *buf << " ";
+ WriteFloat(*buf, pPoints[i + 1].m_Point.x) << " ";
+ WriteFloat(*buf, pPoints[i + 1].m_Point.y) << " ";
+ WriteFloat(*buf, pPoints[i + 2].m_Point.x) << " ";
+ WriteFloat(*buf, pPoints[i + 2].m_Point.y) << " c";
i += 2;
}
if (pPoints[i].m_CloseFigure)
@@ -427,7 +438,7 @@
}
float lineWidth = pPageObj->m_GraphState.GetLineWidth();
if (lineWidth != 1.0f)
- *buf << lineWidth << " w ";
+ WriteFloat(*buf, lineWidth) << " w ";
CFX_GraphStateData::LineCap lineCap = pPageObj->m_GraphState.GetLineCap();
if (lineCap != CFX_GraphStateData::LineCapButt)
*buf << static_cast<int>(lineCap) << " J ";
@@ -545,8 +556,8 @@
dictName = RealizeResource(pIndirectFont, "Font");
m_pObjHolder->m_FontsMap[data] = dictName;
}
- *buf << "/" << PDF_NameEncode(dictName) << " " << pTextObj->GetFontSize()
- << " Tf ";
+ *buf << "/" << PDF_NameEncode(dictName) << " ";
+ WriteFloat(*buf, pTextObj->GetFontSize()) << " Tf ";
ByteString text;
for (uint32_t charcode : pTextObj->GetCharCodes()) {
if (charcode != CPDF_Font::kInvalidCharCode)
diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
index 8e72e90..ff61c01 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
@@ -70,9 +70,76 @@
false);
pPathObj->path().AppendPoint(CFX_PointF(0, 3.78f), FXPT_TYPE::LineTo, true);
buf.str("");
-
TestProcessPath(&generator, &buf, pPathObj.get());
- EXPECT_EQ("q 1 0 0 1 0 0 cm 0 0 5.2 3.78 re n Q\n", ByteString(buf));
+ EXPECT_EQ("q 1 0 0 1 0 0 cm 0 0 5.1999998 3.78 re n Q\n", ByteString(buf));
+}
+
+TEST_F(CPDF_PageContentGeneratorTest, BUG_937) {
+ static const std::vector<float> rgb = {0.000000000000000000001f, 0.7f, 0.35f};
+ CPDF_ColorSpace* pCS = CPDF_ColorSpace::GetStockCS(PDFCS_DEVICERGB);
+
+ {
+ auto pPathObj = pdfium::MakeUnique<CPDF_PathObject>();
+ pPathObj->set_filltype(FXFILL_WINDING);
+
+ // Test code in ProcessPath that generates re operator
+ pPathObj->path().AppendRect(0.000000000000000000001,
+ 0.000000000000000000001, 100, 100);
+
+ pPathObj->m_ColorState.SetFillColor(pCS, rgb);
+ pPathObj->m_ColorState.SetStrokeColor(pCS, rgb);
+ pPathObj->m_GraphState.SetLineWidth(200000000000000000001.0);
+ pPathObj->Transform(CFX_Matrix(1, 0, 0, 1, 0.000000000000000000001,
+ 200000000000000.000002));
+
+ CPDF_Dictionary dummy_page_dict;
+ auto pTestPage =
+ pdfium::MakeRetain<CPDF_Page>(nullptr, &dummy_page_dict, false);
+ CPDF_PageContentGenerator generator(pTestPage.Get());
+ std::ostringstream buf;
+ TestProcessPath(&generator, &buf, pPathObj.get());
+ EXPECT_EQ(
+ "q 0 0.701961 0.34902 rg 0 0.701961 0.34902 RG 200000000000000000000 w"
+ " 1 0 0 1 .00000000000000000000099999997 200000000000000 cm .000000000"
+ "00000000000099999997 .00000000000000000000099999997 100 100 re f Q\n",
+ ByteString(buf));
+ }
+
+ {
+ // Test code in ProcessPath that handles bezier operator
+ auto pPathObj = pdfium::MakeUnique<CPDF_PathObject>();
+ pPathObj->m_ColorState.SetFillColor(pCS, rgb);
+ pPathObj->m_ColorState.SetStrokeColor(pCS, rgb);
+ pPathObj->m_GraphState.SetLineWidth(2.000000000000000000001);
+ pPathObj->Transform(CFX_Matrix(1, 0, 0, 1, 432, 500000000000000.000002));
+
+ pPathObj->set_filltype(FXFILL_WINDING);
+ pPathObj->path().AppendPoint(CFX_PointF(0.000000000000000000001f, 4.67f),
+ FXPT_TYPE::MoveTo, false);
+ pPathObj->path().AppendPoint(
+ CFX_PointF(0.000000000000000000001, 100000000000000.000002),
+ FXPT_TYPE::LineTo, false);
+ pPathObj->path().AppendPoint(CFX_PointF(0.0000000000001f, 3.15f),
+ FXPT_TYPE::BezierTo, false);
+ pPathObj->path().AppendPoint(CFX_PointF(3.57f, 2.98f), FXPT_TYPE::BezierTo,
+ false);
+ pPathObj->path().AppendPoint(
+ CFX_PointF(53.4f, 5000000000000000000.00000000000000004),
+ FXPT_TYPE::BezierTo, true);
+ CPDF_Dictionary dummy_page_dict;
+ auto pTestPage =
+ pdfium::MakeRetain<CPDF_Page>(nullptr, &dummy_page_dict, false);
+ CPDF_PageContentGenerator generator(pTestPage.Get());
+ std::ostringstream buf;
+
+ TestProcessPath(&generator, &buf, pPathObj.get());
+ EXPECT_EQ(
+ "q 0 0.701961 0.34902 rg 0 0.701961 0.34902 RG 2 w 1 0 0 1 432 4999999"
+ "90000000 cm .00000000000000000000099999997 4.6700001 m .0000000000000"
+ "0000000099999997 100000000000000 l .000000000000099999998 3.1500001 3"
+ ".5699999 2.98 53.400002 5000000000000000000 c h f Q\n",
+ ByteString(buf));
+ }
}
TEST_F(CPDF_PageContentGeneratorTest, ProcessPath) {
@@ -106,8 +173,9 @@
TestProcessPath(&generator, &buf, pPathObj.get());
EXPECT_EQ(
"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",
+ "3.1500001 4.6500001 2.98 3.4560001 .23999999 c 10.6000004 11.149999"
+ "6 l 11 12.5 l 11.46 12.6700001 11.8400002 12.96 12 13.6400003 c h f"
+ " Q\n",
ByteString(buf));
}
@@ -333,5 +401,8 @@
CPDF_PageContentGenerator generator(pTestForm.get());
std::ostringstream process_buf;
generator.ProcessPageObjects(&process_buf);
- EXPECT_STREQ(content, ByteString(process_buf).c_str());
+ EXPECT_STREQ(
+ "q 1 0 0 1 0 0 cm 3.102 4.6700001 m 5.4500012 .28999999 l 4.2399998 3.14"
+ "99999 4.6500001 2.98 3.4560001 .24000001 c 3.102 4.6700001 l h f Q\n",
+ ByteString(process_buf).c_str());
}
diff --git a/fpdfsdk/fpdf_annot_embeddertest.cpp b/fpdfsdk/fpdf_annot_embeddertest.cpp
index 59dcd99..b865619 100644
--- a/fpdfsdk/fpdf_annot_embeddertest.cpp
+++ b/fpdfsdk/fpdf_annot_embeddertest.cpp
@@ -689,19 +689,19 @@
TEST_F(FPDFAnnotEmbedderTest, AddAndModifyPath) {
#if _FX_PLATFORM_ == _FX_PLATFORM_APPLE_
const char md5_original[] = "c35408717759562d1f8bf33d317483d2";
- const char md5_modified_path[] = "873b92ea83ccf006e58415d866ce145b";
- const char md5_two_paths[] = "6f1f1c91f50240e9cc9d7c87c48b93a7";
- const char md5_new_annot[] = "078bf58f939645ac305854f31ee9a828";
+ const char md5_modified_path[] = "9059723a045e17478753d2f0eb33bc03";
+ const char md5_two_paths[] = "7eed0cfba780f1d4dd8068f717d3a6bf";
+ const char md5_new_annot[] = "1de8212d43b7066a6df042095c2aca61";
#elif _FX_PLATFORM_ == _FX_PLATFORM_WINDOWS_
const char md5_original[] = "6f3cc2dd37479ce7cc072bfb0c63c275";
- const char md5_modified_path[] = "c66293426cbf1f568502d1f7c0728fc7";
- const char md5_two_paths[] = "122ac4625393b1dfe4be8707b73cbb13";
- const char md5_new_annot[] = "253c7fd739646ba2568e1e8bfc782336";
+ const char md5_modified_path[] = "c0c2eb2aba73ad15b3240e342fbe0d72";
+ const char md5_two_paths[] = "2306bf04915fe001b5f4726843d184c8";
+ const char md5_new_annot[] = "64a319f145768cb09944d2109efe394e";
#else
const char md5_original[] = "964f89bbe8911e540a465cf1a64b7f7e";
- const char md5_modified_path[] = "5a4a6091cff648a4ece3ce7e245e3e38";
- const char md5_two_paths[] = "d6e4072a4415cfc6ec17201fb6be0ee0";
- const char md5_new_annot[] = "fc338b97bf66a656916c6198697a8a28";
+ const char md5_modified_path[] = "9a38048fb3ac1b2c9a4b34139caa993c";
+ const char md5_two_paths[] = "ece3d4df54b3395d6a2bf7a29b17239c";
+ const char md5_new_annot[] = "4299493de84c249f42f220f30f2bbb67";
#endif
// Open a file with two annotations and load its first page.
diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp
index 6e03644..948d356 100644
--- a/fpdfsdk/fpdf_edit_embeddertest.cpp
+++ b/fpdfsdk/fpdf_edit_embeddertest.cpp
@@ -689,10 +689,13 @@
#if _FX_PLATFORM_ == _FX_PLATFORM_APPLE_
const char kNonPrimesMD5[] = "57e76dc7375d896704f0fd6d6d1b9e65";
+ const char kNonPrimesAfterSaveMD5[] = "6304512d0150bbd5578e8e22d3121103";
#elif _FX_PLATFORM_ == _FX_PLATFORM_WINDOWS_
const char kNonPrimesMD5[] = "4d906b57fba36c70c600cf50d60f508c";
+ const char kNonPrimesAfterSaveMD5[] = "4d906b57fba36c70c600cf50d60f508c";
#else
const char kNonPrimesMD5[] = "33d9c45bec41ead92a295e252f6b7922";
+ const char kNonPrimesAfterSaveMD5[] = "33d9c45bec41ead92a295e252f6b7922";
#endif
{
ScopedFPDFBitmap page_bitmap = RenderPage(page);
@@ -711,7 +714,7 @@
{
ScopedFPDFBitmap page_bitmap = RenderPage(saved_page);
- CompareBitmap(page_bitmap.get(), 200, 200, kNonPrimesMD5);
+ CompareBitmap(page_bitmap.get(), 200, 200, kNonPrimesAfterSaveMD5);
}
CloseSavedPage(saved_page);
@@ -2668,7 +2671,7 @@
#if _FX_PLATFORM_ == _FX_PLATFORM_APPLE_
const char md5[] = "17d2b6cd574cf66170b09c8927529a94";
#elif _FX_PLATFORM_ == _FX_PLATFORM_WINDOWS_
- const char md5[] = "d60ba39f9698e32360d99e727dd93165";
+ const char md5[] = "d60ba39f9698e32360d99e727dd93165";
#else
const char md5[] = "70592859010ffbf532a2237b8118bcc4";
#endif
