Use either /RECT or /QuadPoints for annotation coordinates, depending on /AP
On Acrobat, if "/AP" is present on a text markup definition,
the coordinates used to draw the annotation come from "/Rect
values, whereas if "/AP" is not defined, the array defined
in /QuadPoints is used to grab the annotation coordinates from.
PDFium, on the other hand, uses "/Rect" regardless of
presence or absence of "/AP".
CL fixes PDFium to work similarly to Acrobat, in this case.
TEST=testing/resources/pixel/bug_585_*.in
BUG=pdfium:585
Review-Url: https://codereview.chromium.org/2289293005
diff --git a/DEPS b/DEPS
index e3ef8e8..e57e5af 100644
--- a/DEPS
+++ b/DEPS
@@ -14,7 +14,7 @@
'gmock_revision': '29763965ab52f24565299976b936d1265cb6a271',
'gtest_revision': '8245545b6dc9c4703e6496d1efd19e975ad2b038',
'icu_revision': '2341038bf72869a5683a893a2b319a48ffec7f62',
- 'pdfium_tests_revision': '8485b3093524ddb7319e0381ab10c576e59d5091',
+ 'pdfium_tests_revision': 'e86a1bac556194ee572b0cd37d04ea646c7b5320',
'skia_revision': '39f7a10a04a914384944d8bf62621144ac4eeaa3',
'tools_memory_revision': '427f10475e1a8d72424c29d00bf689122b738e5d',
'trace_event_revision': '54b8455be9505c2cb0cf5c26bb86739c236471aa',
diff --git a/core/fpdfdoc/cpdf_annot.cpp b/core/fpdfdoc/cpdf_annot.cpp
index bee96a6..2cc767a 100644
--- a/core/fpdfdoc/cpdf_annot.cpp
+++ b/core/fpdfdoc/cpdf_annot.cpp
@@ -20,6 +20,15 @@
namespace {
+char kPDFiumKey_HasGeneratedAP[] = "PDFIUM_HasGeneratedAP";
+
+bool IsTextMarkupAnnotation(CPDF_Annot::Subtype type) {
+ return type == CPDF_Annot::Subtype::HIGHLIGHT ||
+ type == CPDF_Annot::Subtype::SQUIGGLY ||
+ type == CPDF_Annot::Subtype::STRIKEOUT ||
+ type == CPDF_Annot::Subtype::UNDERLINE;
+}
+
bool ShouldGenerateAPForAnnotation(CPDF_Dictionary* pAnnotDict) {
// If AP dictionary exists, we use the appearance defined in the
// existing AP dictionary.
@@ -40,6 +49,8 @@
m_bOpenState(false),
m_pPopupAnnot(nullptr) {
m_nSubtype = StringToAnnotSubtype(m_pAnnotDict->GetStringFor("Subtype"));
+ m_bIsTextMarkupAnnotation = IsTextMarkupAnnotation(m_nSubtype);
+ m_bHasGeneratedAP = m_pAnnotDict->GetBooleanFor(kPDFiumKey_HasGeneratedAP);
GenerateAPIfNeeded();
}
@@ -53,24 +64,30 @@
if (!ShouldGenerateAPForAnnotation(m_pAnnotDict))
return;
+ bool result = false;
if (m_nSubtype == CPDF_Annot::Subtype::CIRCLE)
- CPVT_GenerateAP::GenerateCircleAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GenerateCircleAP(m_pDocument, m_pAnnotDict);
else if (m_nSubtype == CPDF_Annot::Subtype::HIGHLIGHT)
- CPVT_GenerateAP::GenerateHighlightAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GenerateHighlightAP(m_pDocument, m_pAnnotDict);
else if (m_nSubtype == CPDF_Annot::Subtype::INK)
- CPVT_GenerateAP::GenerateInkAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GenerateInkAP(m_pDocument, m_pAnnotDict);
else if (m_nSubtype == CPDF_Annot::Subtype::POPUP)
- CPVT_GenerateAP::GeneratePopupAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GeneratePopupAP(m_pDocument, m_pAnnotDict);
else if (m_nSubtype == CPDF_Annot::Subtype::SQUARE)
- CPVT_GenerateAP::GenerateSquareAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GenerateSquareAP(m_pDocument, m_pAnnotDict);
else if (m_nSubtype == CPDF_Annot::Subtype::SQUIGGLY)
- CPVT_GenerateAP::GenerateSquigglyAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GenerateSquigglyAP(m_pDocument, m_pAnnotDict);
else if (m_nSubtype == CPDF_Annot::Subtype::STRIKEOUT)
- CPVT_GenerateAP::GenerateStrikeOutAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GenerateStrikeOutAP(m_pDocument, m_pAnnotDict);
else if (m_nSubtype == CPDF_Annot::Subtype::TEXT)
- CPVT_GenerateAP::GenerateTextAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GenerateTextAP(m_pDocument, m_pAnnotDict);
else if (m_nSubtype == CPDF_Annot::Subtype::UNDERLINE)
- CPVT_GenerateAP::GenerateUnderlineAP(m_pDocument, m_pAnnotDict);
+ result = CPVT_GenerateAP::GenerateUnderlineAP(m_pDocument, m_pAnnotDict);
+
+ if (result) {
+ m_pAnnotDict->SetBooleanFor(kPDFiumKey_HasGeneratedAP, result);
+ m_bHasGeneratedAP = result;
+ }
}
bool CPDF_Annot::ShouldDrawAnnotation() {
@@ -91,11 +108,23 @@
return m_nSubtype;
}
+CFX_FloatRect CPDF_Annot::RectForDrawing() const {
+ if (!m_pAnnotDict)
+ return CFX_FloatRect();
+
+ bool bShouldUseQuadPointsCoords =
+ m_bIsTextMarkupAnnotation && m_bHasGeneratedAP;
+ if (bShouldUseQuadPointsCoords)
+ return RectFromQuadPoints(m_pAnnotDict);
+
+ return m_pAnnotDict->GetRectFor("Rect");
+}
+
CFX_FloatRect CPDF_Annot::GetRect() const {
if (!m_pAnnotDict)
return CFX_FloatRect();
- CFX_FloatRect rect = m_pAnnotDict->GetRectFor("Rect");
+ CFX_FloatRect rect = RectForDrawing();
rect.Normalize();
return rect;
}
@@ -176,6 +205,26 @@
}
// Static.
+CFX_FloatRect CPDF_Annot::RectFromQuadPoints(CPDF_Dictionary* pAnnotDict) {
+ CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
+ if (!pArray)
+ return CFX_FloatRect();
+
+ // QuadPoints are defined with 4 pairs of numbers
+ // ([ pair0, pair1, pair2, pair3 ]), where
+ // pair0 = top_left
+ // pair1 = top_right
+ // pair2 = bottom_left
+ // pair3 = bottom_right
+ //
+ // On the other hand, /Rect is define as 2 pairs [pair0, pair1] where:
+ // pair0 = bottom_left
+ // pair1 = top_right.
+ return CFX_FloatRect(pArray->GetNumberAt(4), pArray->GetNumberAt(5),
+ pArray->GetNumberAt(2), pArray->GetNumberAt(3));
+}
+
+// Static.
bool CPDF_Annot::IsAnnotationHidden(CPDF_Dictionary* pAnnotDict) {
return !!(pAnnotDict->GetIntegerFor("F") & ANNOTFLAG_HIDDEN);
}
diff --git a/core/fpdfdoc/cpvt_generateap.cpp b/core/fpdfdoc/cpvt_generateap.cpp
index decc408..d37aaf3 100644
--- a/core/fpdfdoc/cpvt_generateap.cpp
+++ b/core/fpdfdoc/cpvt_generateap.cpp
@@ -586,7 +586,8 @@
void GenerateAndSetAPDict(CPDF_Document* pDoc,
CPDF_Dictionary* pAnnotDict,
const CFX_ByteTextBuf& sAppStream,
- CPDF_Dictionary* pResourceDict) {
+ CPDF_Dictionary* pResourceDict,
+ bool bIsTextMarkupAnnotation) {
CPDF_Dictionary* pAPDict = new CPDF_Dictionary;
pAnnotDict->SetFor("AP", pAPDict);
@@ -602,7 +603,9 @@
pStreamDict->SetStringFor("Subtype", "Form");
pStreamDict->SetMatrixFor("Matrix", CFX_Matrix());
- CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+ CFX_FloatRect rect = bIsTextMarkupAnnotation
+ ? CPDF_Annot::RectFromQuadPoints(pAnnotDict)
+ : pAnnotDict->GetRectFor("Rect");
pStreamDict->SetRectFor("BBox", rect);
pStreamDict->SetFor("Resources", pResourceDict);
@@ -783,7 +786,8 @@
GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
CPDF_Dictionary* pResourceDict =
GenerateResourceDict(pExtGStateDict, nullptr);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ false /*IsTextMarkupAnnotation*/);
return true;
}
@@ -797,7 +801,7 @@
CPVT_Color(CPVT_Color::kRGB, 1, 1, 0),
PaintOperation::FILL);
- CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+ CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
rect.Normalize();
sAppStream << rect.left << " " << rect.top << " m " << rect.right << " "
@@ -809,7 +813,8 @@
GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Multiply");
CPDF_Dictionary* pResourceDict =
GenerateResourceDict(pExtGStateDict, nullptr);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ true /*IsTextMarkupAnnotation*/);
return true;
}
@@ -863,7 +868,8 @@
GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
CPDF_Dictionary* pResourceDict =
GenerateResourceDict(pExtGStateDict, nullptr);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ false /*IsTextMarkupAnnotation*/);
return true;
}
@@ -885,7 +891,8 @@
GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
CPDF_Dictionary* pResourceDict =
GenerateResourceDict(pExtGStateDict, nullptr);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ false /*IsTextMarkupAnnotation*/);
return true;
}
@@ -899,7 +906,7 @@
CPVT_Color(CPVT_Color::kRGB, 0, 0, 0),
PaintOperation::STROKE);
- CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+ CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
rect.Normalize();
FX_FLOAT fLineWidth = 1.0;
@@ -911,7 +918,8 @@
GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
CPDF_Dictionary* pResourceDict =
GenerateResourceDict(pExtGStateDict, nullptr);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ true /*IsTextMarkupAnnotation*/);
return true;
}
@@ -949,7 +957,8 @@
return false;
sAppStream << GetPopupContentsString(pDoc, *pAnnotDict, pDefFont, sFontName);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ false /*IsTextMarkupAnnotation*/);
return true;
}
@@ -996,7 +1005,8 @@
GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
CPDF_Dictionary* pResourceDict =
GenerateResourceDict(pExtGStateDict, nullptr);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ false /*IsTextMarkupAnnotation*/);
return true;
}
@@ -1010,7 +1020,7 @@
CPVT_Color(CPVT_Color::kRGB, 0, 0, 0),
PaintOperation::STROKE);
- CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+ CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
rect.Normalize();
FX_FLOAT fLineWidth = 1.0;
@@ -1044,7 +1054,8 @@
GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
CPDF_Dictionary* pResourceDict =
GenerateResourceDict(pExtGStateDict, nullptr);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ true /*IsTextMarkupAnnotation*/);
return true;
}
@@ -1058,7 +1069,7 @@
CPVT_Color(CPVT_Color::kRGB, 0, 0, 0),
PaintOperation::STROKE);
- CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+ CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
rect.Normalize();
FX_FLOAT fLineWidth = 1.0;
@@ -1070,7 +1081,8 @@
GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
CPDF_Dictionary* pResourceDict =
GenerateResourceDict(pExtGStateDict, nullptr);
- GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+ GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+ true /*IsTextMarkupAnnotation*/);
return true;
}
diff --git a/core/fpdfdoc/include/cpdf_annot.h b/core/fpdfdoc/include/cpdf_annot.h
index c16decc..43f5893 100644
--- a/core/fpdfdoc/include/cpdf_annot.h
+++ b/core/fpdfdoc/include/cpdf_annot.h
@@ -71,6 +71,7 @@
static CPDF_Annot::Subtype StringToAnnotSubtype(
const CFX_ByteString& sSubtype);
static CFX_ByteString AnnotSubtypeToString(CPDF_Annot::Subtype nSubtype);
+ static CFX_FloatRect RectFromQuadPoints(CPDF_Dictionary* pAnnotDict);
CPDF_Annot(CPDF_Dictionary* pDict, CPDF_Document* pDocument, bool bToOwnDict);
~CPDF_Annot();
@@ -102,6 +103,8 @@
void GenerateAPIfNeeded();
bool ShouldDrawAnnotation();
+ CFX_FloatRect RectForDrawing() const;
+
// For regular annotations, |m_pAnnotDict| is not owned. For
// our artificially created popup annotations, |m_pAnnotDict|
// is owned by this class.
@@ -112,6 +115,8 @@
std::map<CPDF_Stream*, std::unique_ptr<CPDF_Form>> m_APMap;
// |m_bOpenState| is only set for popup annotations.
bool m_bOpenState;
+ bool m_bHasGeneratedAP;
+ bool m_bIsTextMarkupAnnotation;
// Not owned. If there is a valid pointer in |m_pPopupAnnot|,
// then this annot is never a popup.
CPDF_Annot* m_pPopupAnnot;
diff --git a/testing/resources/pixel/bug_585.in b/testing/resources/pixel/bug_585.in
new file mode 100644
index 0000000..b8b56ad
--- /dev/null
+++ b/testing/resources/pixel/bug_585.in
@@ -0,0 +1,175 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 1
+ /Kids [
+ 10 0 R
+ ]
+>>
+endobj
+% Page number 0.
+{{object 10 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /Resources <<
+ /Font <</F1 15 0 R>>
+ >>
+ /MediaBox [0 0 612 792]
+ /Annots [
+ 22 0 R
+ 23 0 R
+ 24 0 R
+ 25 0 R
+ ]
+ /Tabs /R
+>>
+endobj
+
+{{object 22 0}} <<
+ /Type /Annot
+ /Subtype /Highlight
+ /Rect [ -1 -1 -1 -1 ]
+ /NM (Annot-1)
+ /F 4
+ /QuadPoints [ 475 688 512 688 475 679 512 679 ]
+ /C [ 0.0001108646 0.001760244 0.9982184 ]
+ /Contents ()
+>>
+endobj
+
+{{object 23 0}} <<
+ /Border [
+ 0
+ 0
+ 1
+ ]
+ /C [
+ 0.294118
+ 0.6
+ 1
+ ]
+ /CA 1
+ /CreationDate (D:20150312175256+08'00')
+ /F 4
+ /M (D:20150312175256+08'00')
+ /NM (7f264ba2-e270-42a1-a390-eb41278072ff)
+ /QuadPoints [
+ 227.567
+ 688.016
+ 298.115
+ 688.016
+ 227.567
+ 679.292
+ 298.115
+ 679.292
+ ]
+ /RC (<?xml version="1.0"?><body xmlns="http://www.w3.org/1999/xhtml" xmlns:xfa="http://www.xfa.org/schema/xfa-data/1.0/" xfa:APIVersion="Acroform:2.7.0.0" xfa:spec="2.1"><p style="text-align:left" dir="ltr"><span style="line-height:0pt,font-size:0pt;font-style:normal;font-weight:normal;color:#000000;font-family:Helvetica"></span>\r\n</p>\r\n</body>\r\n)
+ /Rect [
+ -1
+ -1
+ -1
+ -1
+ ]
+ /Subj (Squiggly)
+ /Subtype /Squiggly
+ /T (Administrator)
+ /Type /Annot
+>>
+endobj
+
+{{object 24 0}} <<
+ /Border [
+ 0
+ 0
+ 1
+ ]
+ /C [
+ 0.278431
+ 0.603922
+ 0.6
+ ]
+ /CA 1
+ /CreationDate (D:20150312175350+08'00')
+ /F 4
+ /IRT 6 0 R
+ /IT /StrikeOutTextEdit
+ /M (D:20150312175350+08'00')
+ /NM (2912a3cf-3b30-48a7-8531-431ce468e6a8)
+ /P 4 0 R
+ /QuadPoints [
+ 186.875
+ 714.836
+ 293.483
+ 714.836
+ 186.875
+ 706.064
+ 293.483
+ 706.064
+ ]
+ /RT /Group
+ /Rect [
+ -1
+ -1
+ -1
+ -1
+ ]
+ /Subj (Replace)
+ /Subtype /StrikeOut
+ /T (Administrator)
+ /Type /Annot
+>>
+endobj
+
+{{object 25 0}} <<
+ /Border [
+ 0
+ 0
+ 1
+ ]
+ /C [
+ 0.2
+ 0.619608
+ 0
+ ]
+ /CA 1
+ /CreationDate (D:20150312175318+08'00')
+ /F 4
+ /M (D:20150312175318+08'00')
+ /NM (c99bdf16-3040-4bf6-9b8a-a80cc563a6a7)
+ /P 4 0 R
+ /Popup 6 0 R
+ /QuadPoints [
+ 204.395
+ 647.984
+ 403.895
+ 647.984
+ 204.395
+ 636.872
+ 403.895
+ 636.872
+ ]
+ /RC (<?xml version="1.0"?><body xmlns="http://www.w3.org/1999/xhtml" xmlns:xfa="http://www.xfa.org/schema/xfa-data/1.0/" xfa:APIVersion="Acroform:2.7.0.0" xfa:spec="2.1"><p style="text-align:left" dir="ltr"><span style="line-height:0pt,font-size:0pt;font-style:normal;font-weight:normal;color:#000000;font-family:Helvetica"></span>\r\n</p>\r\n</body>\r\n)
+ /Rect [
+ -1
+ -1
+ -1
+ -1
+ ]
+ /Subj (Underline)
+ /Subtype /Underline
+ /T (Administrator)
+ /Type /Annot
+>>
+endobj
+
+{{xref}}
+trailer <<
+ /Root 1 0 R
+>>
+{{startxref}}
+%%EOF
diff --git a/testing/resources/pixel/bug_585_expected.pdf.0.png b/testing/resources/pixel/bug_585_expected.pdf.0.png
new file mode 100644
index 0000000..34bc892
--- /dev/null
+++ b/testing/resources/pixel/bug_585_expected.pdf.0.png
Binary files differ
