Fix incorrect matrix outputs in CPDF_PageContentGenerator When CPDF_PageContentGenerator generates content stream data for a page object, it currently does not take the object's container's current transformation matrix into account. As a result, the generated output effectively has the CTM applied twice for page objects whose CTM is not the identity matrix, as shown in recently added PDFEditImgTest cases. Change CPDF_PageContentGenerator to take the CTM into account for images and forms. Then update the related PDFEditImgTest cases so they pass with the expected results. Add TODOs for CPDF_PageContentGenerator methods that process text and paths, as there are no associated tests for those. Along the way, use more local matrix variables to avoid repeated calls to getters. Bug: pdfium:2132 Change-Id: I8d4c783ddee5c44dfeca32ec5575d228572e2417 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116933 Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Thomas Sepez <tsepez@google.com> Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp index 617891c..c427478 100644 --- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp +++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
@@ -174,8 +174,10 @@ // Set the default graphic state values buf << "q\n"; - if (!m_pObjHolder->GetLastCTM().IsIdentity()) - WriteMatrix(buf, m_pObjHolder->GetLastCTM().GetInverse()) << " cm\n"; + const CFX_Matrix& ctm = m_pObjHolder->GetLastCTM(); + if (!ctm.IsIdentity()) { + WriteMatrix(buf, ctm.GetInverse()) << " cm\n"; + } ProcessDefaultGraphics(&buf); streams[dirty_stream] = std::move(buf); @@ -397,8 +399,8 @@ void CPDF_PageContentGenerator::ProcessImage(fxcrt::ostringstream* buf, CPDF_ImageObject* pImageObj) { - if ((pImageObj->matrix().a == 0 && pImageObj->matrix().b == 0) || - (pImageObj->matrix().c == 0 && pImageObj->matrix().d == 0)) { + CFX_Matrix matrix = pImageObj->matrix(); + if ((matrix.a == 0 && matrix.b == 0) || (matrix.c == 0 && matrix.d == 0)) { return; } @@ -411,7 +413,12 @@ return; *buf << "q "; - WriteMatrix(*buf, pImageObj->matrix()) << " cm "; + + const CFX_Matrix& ctm = m_pObjHolder->GetLastCTM(); + if (!ctm.IsIdentity()) { + matrix.Concat(ctm.GetInverse()); + } + WriteMatrix(*buf, matrix) << " cm "; bool bWasInline = pStream->IsInline(); if (bWasInline) @@ -430,8 +437,8 @@ void CPDF_PageContentGenerator::ProcessForm(fxcrt::ostringstream* buf, CPDF_FormObject* pFormObj) { - if ((pFormObj->form_matrix().a == 0 && pFormObj->form_matrix().b == 0) || - (pFormObj->form_matrix().c == 0 && pFormObj->form_matrix().d == 0)) { + CFX_Matrix matrix = pFormObj->form_matrix(); + if ((matrix.a == 0 && matrix.b == 0) || (matrix.c == 0 && matrix.d == 0)) { return; } @@ -443,7 +450,13 @@ pFormObj->SetResourceName(name); *buf << "q\n"; - WriteMatrix(*buf, pFormObj->form_matrix()) << " cm "; + + const CFX_Matrix& ctm = m_pObjHolder->GetLastCTM(); + if (!ctm.IsIdentity()) { + matrix.Concat(ctm.GetInverse()); + } + WriteMatrix(*buf, matrix) << " cm "; + *buf << "/" << PDF_NameEncode(name) << " Do Q\n"; } @@ -502,6 +515,8 @@ CPDF_PathObject* pPathObj) { ProcessGraphics(buf, pPathObj); + // TODO(crbug.com/pdfium/2132): Does this need to take the current + // transformation matrix in `m_pObjHolder` into account? WriteMatrix(*buf, pPathObj->matrix()) << " cm "; ProcessPathPoints(buf, &pPathObj->path()); @@ -654,6 +669,8 @@ CPDF_TextObject* pTextObj) { ProcessGraphics(buf, pTextObj); *buf << "BT "; + // TODO(crbug.com/pdfium/2132): Does this need to take the current + // transformation matrix in `m_pObjHolder` into account? WriteMatrix(*buf, pTextObj->GetTextMatrix()) << " Tm "; RetainPtr<CPDF_Font> pFont(pTextObj->GetFont()); if (!pFont)
diff --git a/fpdfsdk/fpdf_editimg_embeddertest.cpp b/fpdfsdk/fpdf_editimg_embeddertest.cpp index c06d495..d16cef3 100644 --- a/fpdfsdk/fpdf_editimg_embeddertest.cpp +++ b/fpdfsdk/fpdf_editimg_embeddertest.cpp
@@ -219,9 +219,7 @@ UnloadPage(page); - // TODO(crbug.com/pdfium/2132): Should be `kExpectedChecksum`. - constexpr char kWrongChecksum[] = "d01c62c9ee094f5be545f006148808b5"; - VerifySavedDocument(kExpectedWidth, kExpectedHeight, kWrongChecksum); + VerifySavedDocument(kExpectedWidth, kExpectedHeight, kExpectedChecksum); } TEST_F(PDFEditImgTest, GetAndSetMatrixForFormWithImage) { @@ -289,7 +287,5 @@ UnloadPage(page); - // TODO(crbug.com/pdfium/2132): Should be `kExpectedChecksum`. - constexpr char kWrongChecksum[] = "308e2263f01744b2749d68a9ca711fc6"; - VerifySavedDocument(kExpectedWidth, kExpectedHeight, kWrongChecksum); + VerifySavedDocument(kExpectedWidth, kExpectedHeight, kExpectedChecksum); }