From 04e4dc88da34c323e7a16586bddf377a610d63c5 Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Wed, 18 Oct 2017 12:17:14 -0400 Subject: 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 Commit-Queue: dsinclair --- BUILD.gn | 1 + .../parser/cpdf_security_handler_embeddertest.cpp | 10 +++- fpdfsdk/fpdfannot_embeddertest.cpp | 31 +++++++---- fpdfsdk/fpdfedit_embeddertest.cpp | 24 +++++---- fpdfsdk/fpdfeditpath.cpp | 8 +-- fpdfsdk/fpdfeditpath_embeddertest.cpp | 63 ++++++++++++++++++++++ fpdfsdk/fpdfformfill_embeddertest.cpp | 2 +- testing/embedder_test.cpp | 60 +++++++++++++++------ testing/embedder_test.h | 12 ++--- 9 files changed, 162 insertions(+), 49 deletions(-) create mode 100644 fpdfsdk/fpdfeditpath_embeddertest.cpp 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(pPathObj->m_GeneralState.GetStrokeAlpha() * - 255.f); + *A = static_cast( + (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(pPathObj->m_GeneralState.GetFillAlpha() * - 255.f); + *A = static_cast( + (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(&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(); -- cgit v1.2.3