Fix rounding of colour values

This CL fixes rounding issues with the colour values when written then
read from path objects.

Bug: pdfium:919
Change-Id: I8ab33706f1c7d81c3ec755706b1a613cf2a557b3
Reviewed-on: https://pdfium-review.googlesource.com/16270
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

9 files changed, 162 insertions, 49 deletions

diff --git a/BUILD.gn b/BUILD.gn
index 3de2d32566..5d0ad6dcd8 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -2008,6 +2008,7 @@ test("pdfium_embeddertests") {
     "fpdfsdk/fpdfattachment_embeddertest.cpp",
     "fpdfsdk/fpdfdoc_embeddertest.cpp",
     "fpdfsdk/fpdfedit_embeddertest.cpp",
+    "fpdfsdk/fpdfeditpath_embeddertest.cpp",
     "fpdfsdk/fpdfext_embeddertest.cpp",
     "fpdfsdk/fpdfformfill_embeddertest.cpp",
     "fpdfsdk/fpdfppo_embeddertest.cpp",
diff --git a/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp b/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
index 91346dae2a..3770e98d86 100644
--- a/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
@@ -73,14 +73,20 @@ TEST_F(CPDFSecurityHandlerEmbeddertest, PasswordAfterGenerateSave) {
   file_access.m_GetBlock = GetBlockFromString;
   file_access.m_Param = &new_file;
   EXPECT_FALSE(FPDF_LoadCustomDocument(&file_access, nullptr));
+
   struct {
     const char* password;
     const unsigned long permissions;
   } tests[] = {{"1234", 0xFFFFF2C0}, {"5678", 0xFFFFFFFC}};
+
   for (const auto& test : tests) {
-    TestSaved(612, 792, md5, test.password);
+    OpenSavedDocument(test.password);
+    LoadSavedPage();
+    VerifySavedRendering(612, 792, md5);
     EXPECT_EQ(test.permissions, FPDF_GetDocPermissions(m_SavedDocument));
-    CloseSaved();
+
+    CloseSavedPage();
+    CloseSavedDocument();
   }
 }
 
diff --git a/fpdfsdk/fpdfannot_embeddertest.cpp b/fpdfsdk/fpdfannot_embeddertest.cpp
index b2cace9b16..fbcd9f2972 100644
--- a/fpdfsdk/fpdfannot_embeddertest.cpp
+++ b/fpdfsdk/fpdfannot_embeddertest.cpp
@@ -284,7 +284,10 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndSaveUnderlineAnnotation) {
 
   // Open the saved document.
   const char md5[] = "184b67b322edaee27994b3232544b8b3";
-  TestSaved(612, 792, md5);
+
+  OpenSavedDocument();
+  LoadSavedPage();
+  VerifySavedRendering(612, 792, md5);
 
   // Check that the saved document has 2 annotations on the first page
   EXPECT_EQ(2, FPDFPage_GetAnnotCount(m_SavedPage));
@@ -302,7 +305,9 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndSaveUnderlineAnnotation) {
   EXPECT_NEAR(quadpoints.y4, new_quadpoints.y4, 0.001f);
 
   FPDFPage_CloseAnnot(new_annot);
-  CloseSaved();
+
+  CloseSavedPage();
+  CloseSavedDocument();
 }
 
 TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
@@ -449,7 +454,7 @@ TEST_F(FPDFAnnotEmbeddertest, RemoveAnnotation) {
   EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
   FPDF_ClosePage(page);
 
-  // TODO(npm): TestSaved changes annot rect dimensions by 1??
+  // TODO(npm): VerifySavedRendering changes annot rect dimensions by 1??
   // Open the saved document.
   std::string new_file = GetString();
   FPDF_FILEACCESS file_access;
@@ -590,7 +595,9 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndModifyPath) {
   FPDF_ClosePage(page);
 
   // Open the saved document.
-  TestSaved(595, 842, md5_new_annot);
+  OpenSavedDocument();
+  LoadSavedPage();
+  VerifySavedRendering(595, 842, md5_new_annot);
 
   // Check that the document has a correct count of annotations and objects.
   EXPECT_EQ(3, FPDFPage_GetAnnotCount(m_SavedPage));
@@ -606,7 +613,8 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndModifyPath) {
   EXPECT_EQ(rect.top, new_rect.top);
 
   FPDFPage_CloseAnnot(annot);
-  CloseSaved();
+  CloseSavedPage();
+  CloseSavedDocument();
 }
 
 TEST_F(FPDFAnnotEmbeddertest, ModifyAnnotationFlags) {
@@ -730,12 +738,10 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndModifyImage) {
   // Save the document, closing the page and document.
   EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
   FPDF_ClosePage(page);
+  FPDFBitmap_Destroy(image_bitmap);
 
   // Test that the saved document renders the modified image object correctly.
-  TestSaved(595, 842, md5_modified_image);
-
-  FPDFBitmap_Destroy(image_bitmap);
-  CloseSaved();
+  VerifySavedDocument(595, 842, md5_modified_image);
 }
 
 TEST_F(FPDFAnnotEmbeddertest, AddAndModifyText) {
@@ -878,7 +884,9 @@ TEST_F(FPDFAnnotEmbeddertest, GetSetStringValue) {
 #else
   const char md5[] = "0e3710ea6476f5bcba2cd39eb42d54e2";
 #endif
-  TestSaved(595, 842, md5);
+  OpenSavedDocument();
+  LoadSavedPage();
+  VerifySavedRendering(595, 842, md5);
   FPDF_ANNOTATION new_annot = FPDFPage_GetAnnot(m_SavedPage, 0);
 
   // Check that the string value of the modified date is the newly-set value.
@@ -893,7 +901,8 @@ TEST_F(FPDFAnnotEmbeddertest, GetSetStringValue) {
                    .c_str());
 
   FPDFPage_CloseAnnot(new_annot);
-  CloseSaved();
+  CloseSavedPage();
+  CloseSavedDocument();
 }
 
 TEST_F(FPDFAnnotEmbeddertest, ExtractLinkedAnnotations) {
diff --git a/fpdfsdk/fpdfedit_embeddertest.cpp b/fpdfsdk/fpdfedit_embeddertest.cpp
index 74e85f6795..60db27ad0c 100644
--- a/fpdfsdk/fpdfedit_embeddertest.cpp
+++ b/fpdfsdk/fpdfedit_embeddertest.cpp
@@ -223,7 +223,7 @@ TEST_F(FPDFEditEmbeddertest, RasterizePDF) {
   // Get the generated content. Make sure it is at least as big as the original
   // PDF.
   EXPECT_GT(GetString().size(), 923U);
-  TestAndCloseSaved(612, 792, kAllBlackMd5sum);
+  VerifySavedDocument(612, 792, kAllBlackMd5sum);
 }
 
 TEST_F(FPDFEditEmbeddertest, AddPaths) {
@@ -366,7 +366,7 @@ TEST_F(FPDFEditEmbeddertest, AddPaths) {
   FPDF_ClosePage(page);
 
   // Render the saved result
-  TestAndCloseSaved(612, 792, last_md5);
+  VerifySavedDocument(612, 792, last_md5);
 }
 
 TEST_F(FPDFEditEmbeddertest, PathsPoints) {
@@ -456,8 +456,9 @@ TEST_F(FPDFEditEmbeddertest, EditOverExistingContent) {
   EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
   UnloadPage(page);
 
-  // Render the saved result without closing the page and document
-  TestSaved(612, 792, "ad04e5bd0f471a9a564fb034bd0fb073");
+  OpenSavedDocument();
+  LoadSavedPage();
+  VerifySavedRendering(612, 792, "ad04e5bd0f471a9a564fb034bd0fb073");
 
   ClearString();
   // Add another opaque rectangle on top of the existing content
@@ -479,10 +480,12 @@ TEST_F(FPDFEditEmbeddertest, EditOverExistingContent) {
 
   // Now save the result, closing the page and document
   EXPECT_TRUE(FPDF_SaveAsCopy(m_SavedDocument, this, 0));
-  CloseSaved();
+
+  CloseSavedPage();
+  CloseSavedDocument();
 
   // Render the saved result
-  TestAndCloseSaved(612, 792, last_md5);
+  VerifySavedDocument(612, 792, last_md5);
 }
 
 TEST_F(FPDFEditEmbeddertest, AddStrokedPaths) {
@@ -915,7 +918,8 @@ TEST_F(FPDFEditEmbeddertest, AddTrueTypeFontText) {
   EXPECT_TRUE(FPDFPage_GenerateContent(page));
   EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
   FPDF_ClosePage(page);
-  TestAndCloseSaved(612, 792, md5_2);
+
+  VerifySavedDocument(612, 792, md5_2);
 }
 
 TEST_F(FPDFEditEmbeddertest, TransformAnnot) {
@@ -989,7 +993,8 @@ TEST_F(FPDFEditEmbeddertest, AddCIDFontText) {
   EXPECT_TRUE(FPDFPage_GenerateContent(page));
   EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
   FPDF_ClosePage(page);
-  TestAndCloseSaved(612, 792, md5);
+
+  VerifySavedDocument(612, 792, md5);
 }
 #endif  // _FX_PLATFORM_ == _FX_PLATFORM_LINUX_
 
@@ -1021,7 +1026,8 @@ TEST_F(FPDFEditEmbeddertest, SaveAndRender) {
     EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
     UnloadPage(page);
   }
-  TestAndCloseSaved(612, 792, md5);
+
+  VerifySavedDocument(612, 792, md5);
 }
 
 TEST_F(FPDFEditEmbeddertest, ExtractImageBitmap) {
diff --git a/fpdfsdk/fpdfeditpath.cpp b/fpdfsdk/fpdfeditpath.cpp
index 30c6cb0698..a291987dce 100644
--- a/fpdfsdk/fpdfeditpath.cpp
+++ b/fpdfsdk/fpdfeditpath.cpp
@@ -85,8 +85,8 @@ FPDFPath_GetStrokeColor(FPDF_PAGEOBJECT path,
   *R = FXSYS_GetRValue(strokeRGB);
   *G = FXSYS_GetGValue(strokeRGB);
   *B = FXSYS_GetBValue(strokeRGB);
-  *A = static_cast<unsigned int>(pPathObj->m_GeneralState.GetStrokeAlpha() *
-                                 255.f);
+  *A = static_cast<unsigned int>(
+      (pPathObj->m_GeneralState.GetStrokeAlpha() * 255.f) + 0.5f);
   return true;
 }
 
@@ -122,8 +122,8 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFPath_GetFillColor(FPDF_PAGEOBJECT path,
   *R = FXSYS_GetRValue(fillRGB);
   *G = FXSYS_GetGValue(fillRGB);
   *B = FXSYS_GetBValue(fillRGB);
-  *A = static_cast<unsigned int>(pPathObj->m_GeneralState.GetFillAlpha() *
-                                 255.f);
+  *A = static_cast<unsigned int>(
+      (pPathObj->m_GeneralState.GetFillAlpha() * 255.f) + 0.5f);
   return true;
 }
 
diff --git a/fpdfsdk/fpdfeditpath_embeddertest.cpp b/fpdfsdk/fpdfeditpath_embeddertest.cpp
new file mode 100644
index 0000000000..71af2fad55
--- /dev/null
+++ b/fpdfsdk/fpdfeditpath_embeddertest.cpp
@@ -0,0 +1,63 @@
+// Copyright 2017 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fxcrt/fx_system.h"
+#include "public/fpdf_edit.h"
+#include "testing/embedder_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/test_support.h"
+
+class FPDFEditPathEmbedderTest : public EmbedderTest {};
+
+TEST_F(FPDFEditPathEmbedderTest, VerifyCorrectColoursReturned) {
+  CreateEmptyDocument();
+  FPDF_PAGE page = FPDFPage_New(document(), 0, 612, 792);
+
+  for (size_t i = 0; i < 256; ++i) {
+    FPDF_PAGEOBJECT path = FPDFPageObj_CreateNewPath(400, 100);
+    EXPECT_TRUE(FPDFPath_SetFillColor(path, i, i, i, i));
+    EXPECT_TRUE(FPDFPath_SetStrokeColor(path, i, i, i, i));
+    EXPECT_TRUE(FPDFPath_SetDrawMode(path, FPDF_FILLMODE_ALTERNATE, 0));
+    EXPECT_TRUE(FPDFPath_LineTo(path, 400, 200));
+    EXPECT_TRUE(FPDFPath_LineTo(path, 300, 100));
+    EXPECT_TRUE(FPDFPath_Close(path));
+
+    FPDFPage_InsertObject(page, path);
+  }
+
+  EXPECT_TRUE(FPDFPage_GenerateContent(page));
+  EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+  FPDF_ClosePage(page);
+  page = nullptr;
+
+  OpenSavedDocument();
+  page = LoadSavedPage();
+  ASSERT(page);
+
+  for (size_t i = 0; i < 256; ++i) {
+    FPDF_PAGEOBJECT path = FPDFPage_GetObject(page, i);
+    ASSERT(path);
+
+    EXPECT_EQ(FPDF_PAGEOBJ_PATH, FPDFPageObj_GetType(path));
+
+    unsigned int r;
+    unsigned int g;
+    unsigned int b;
+    unsigned int a;
+    FPDFPath_GetFillColor(path, &r, &g, &b, &a);
+    EXPECT_EQ(i, r);
+    EXPECT_EQ(i, g);
+    EXPECT_EQ(i, b);
+    EXPECT_EQ(i, a);
+
+    FPDFPath_GetStrokeColor(path, &r, &g, &b, &a);
+    EXPECT_EQ(i, r);
+    EXPECT_EQ(i, g);
+    EXPECT_EQ(i, b);
+    EXPECT_EQ(i, a);
+  }
+
+  CloseSavedPage();
+  CloseSavedDocument();
+}
diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp
index 7a9ebcfdd2..4a3fc03d24 100644
--- a/fpdfsdk/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/fpdfformfill_embeddertest.cpp
@@ -562,7 +562,7 @@ TEST_F(FPDFFormFillEmbeddertest, FormText) {
     UnloadPage(page);
   }
   // Check saved document
-  TestAndCloseSaved(300, 300, md5_3);
+  VerifySavedDocument(300, 300, md5_3);
 }
 
 TEST_F(FPDFFormFillEmbeddertest, HasFormInfoNone) {
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index 2694977384..4c29c06da5 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -310,10 +310,7 @@ void EmbedderTest::UnloadPage(FPDF_PAGE page) {
   page_reverse_map_.erase(it);
 }
 
-void EmbedderTest::TestSaved(int width,
-                             int height,
-                             const char* md5,
-                             const char* password) {
+FPDF_DOCUMENT EmbedderTest::OpenSavedDocument(const char* password) {
   memset(&saved_file_access_, 0, sizeof(saved_file_access_));
   saved_file_access_.m_FileLen = m_String.size();
   saved_file_access_.m_GetBlock = GetBlockFromString;
@@ -322,28 +319,59 @@ void EmbedderTest::TestSaved(int width,
   saved_fake_file_access_ =
       pdfium::MakeUnique<FakeFileAccess>(&saved_file_access_);
 
-  ASSERT_TRUE(OpenDocumentHelper(password, false, saved_fake_file_access_.get(),
+  EXPECT_TRUE(OpenDocumentHelper(password, false, saved_fake_file_access_.get(),
                                  &m_SavedDocument, &m_SavedAvail,
                                  &m_SavedForm));
+  return m_SavedDocument;
+}
+
+void EmbedderTest::CloseSavedDocument() {
+  ASSERT(!m_SavedPage);
+  ASSERT(m_SavedDocument);
+
+  FPDFDOC_ExitFormFillEnvironment(m_SavedForm);
+  FPDF_CloseDocument(m_SavedDocument);
+  FPDFAvail_Destroy(m_SavedAvail);
+
+  m_SavedForm = nullptr;
+  m_SavedDocument = nullptr;
+  m_SavedAvail = nullptr;
+}
+
+FPDF_PAGE EmbedderTest::LoadSavedPage() {
+  ASSERT(m_SavedDocument);
+
   EXPECT_EQ(1, FPDF_GetPageCount(m_SavedDocument));
   m_SavedPage = FPDF_LoadPage(m_SavedDocument, 0);
-  ASSERT_TRUE(m_SavedPage);
+
+  ASSERT(m_SavedPage);
+  return m_SavedPage;
+}
+
+void EmbedderTest::CloseSavedPage() {
+  ASSERT(m_SavedPage);
+  FPDF_ClosePage(m_SavedPage);
+  m_SavedPage = nullptr;
+}
+
+void EmbedderTest::VerifySavedRendering(int width,
+                                        int height,
+                                        const char* md5) {
+  ASSERT(m_SavedDocument);
+  ASSERT(m_SavedPage);
+
   FPDF_BITMAP new_bitmap =
       RenderPageWithFlags(m_SavedPage, m_SavedForm, FPDF_ANNOT);
   CompareBitmap(new_bitmap, width, height, md5);
   FPDFBitmap_Destroy(new_bitmap);
 }
 
-void EmbedderTest::CloseSaved() {
-  FPDF_ClosePage(m_SavedPage);
-  FPDFDOC_ExitFormFillEnvironment(m_SavedForm);
-  FPDF_CloseDocument(m_SavedDocument);
-  FPDFAvail_Destroy(m_SavedAvail);
-}
-
-void EmbedderTest::TestAndCloseSaved(int width, int height, const char* md5) {
-  TestSaved(width, height, md5);
-  CloseSaved();
+void EmbedderTest::VerifySavedDocument(int width, int height, const char* md5) {
+  OpenSavedDocument();
+  LoadSavedPage();
+  VerifySavedRendering(width, height, md5);
+  CloseSavedPage();
+  CloseSavedDocument();
 }
 
 void EmbedderTest::SetWholeFileAvailable() {
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index be89c2a6cd..606472bc1d 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -143,12 +143,12 @@ class EmbedderTest : public ::testing::Test,
                                 unsigned char* buf,
                                 unsigned long size);
 
-  void TestSaved(int width,
-                 int height,
-                 const char* md5,
-                 const char* password = nullptr);
-  void CloseSaved();
-  void TestAndCloseSaved(int width, int height, const char* md5);
+  FPDF_DOCUMENT OpenSavedDocument(const char* password = nullptr);
+  void CloseSavedDocument();
+  FPDF_PAGE LoadSavedPage();
+  void CloseSavedPage();
+  void VerifySavedRendering(int width, int height, const char* md5);
+  void VerifySavedDocument(int width, int height, const char* md5);
 
   void SetWholeFileAvailable();