Made existing annotation APIs to work with AP
1. Modified some existing annotation APIs to take into account the
effect of annotation's appearance streams.
* Added an embedder test testing annotations with APs.
This CL is refactored out of
https://pdfium-review.googlesource.com/c/6676/.
Bug=pdfium:737
Change-Id: I27d5e66dfdb90038d147cab1a26e0bf86b324982
Reviewed-on: https://pdfium-review.googlesource.com/7030
Commit-Queue: Jane Liu <janeliulwq@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
diff --git a/core/fpdfdoc/cpdf_annot.cpp b/core/fpdfdoc/cpdf_annot.cpp
index e0f073f..e43a735 100644
--- a/core/fpdfdoc/cpdf_annot.cpp
+++ b/core/fpdfdoc/cpdf_annot.cpp
@@ -158,7 +158,7 @@
return m_pAnnotDict->GetIntegerFor("F");
}
-CPDF_Stream* FPDFDOC_GetAnnotAP(CPDF_Dictionary* pAnnotDict,
+CPDF_Stream* FPDFDOC_GetAnnotAP(const CPDF_Dictionary* pAnnotDict,
CPDF_Annot::AppearanceMode mode) {
CPDF_Dictionary* pAP = pAnnotDict->GetDictFor("AP");
if (!pAP) {
diff --git a/core/fpdfdoc/cpdf_annot.h b/core/fpdfdoc/cpdf_annot.h
index 85a2053..081cef7 100644
--- a/core/fpdfdoc/cpdf_annot.h
+++ b/core/fpdfdoc/cpdf_annot.h
@@ -124,7 +124,7 @@
CPDF_Annot* m_pPopupAnnot = nullptr;
};
-CPDF_Stream* FPDFDOC_GetAnnotAP(CPDF_Dictionary* pAnnotDict,
+CPDF_Stream* FPDFDOC_GetAnnotAP(const CPDF_Dictionary* pAnnotDict,
CPDF_Annot::AppearanceMode mode);
#endif // CORE_FPDFDOC_CPDF_ANNOT_H_
diff --git a/fpdfsdk/fpdfannot.cpp b/fpdfsdk/fpdfannot.cpp
index 169c31c..4a288b7 100644
--- a/fpdfsdk/fpdfannot.cpp
+++ b/fpdfsdk/fpdfannot.cpp
@@ -136,6 +136,10 @@
return static_cast<CPDF_AnnotContext*>(annot);
}
+bool HasAPStream(const CPDF_Dictionary* pAnnotDict) {
+ return !!FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+}
+
} // namespace
DLLEXPORT FPDF_BOOL STDCALL
@@ -225,6 +229,12 @@
if (!pAnnotDict)
return false;
+ // For annotations with their appearance streams already defined, the path
+ // stream's own color definitions take priority over the annotation color
+ // definitions set by this method, hence this method will simply fail.
+ if (HasAPStream(pAnnotDict))
+ return false;
+
// Set the opacity of the annotation.
pAnnotDict->SetNewFor<CPDF_Number>("CA", A / 255.f);
@@ -257,6 +267,12 @@
if (!pAnnotDict)
return false;
+ // For annotations with their appearance streams already defined, the path
+ // stream's own color definitions take priority over the annotation color
+ // definitions retrieved by this method, hence this method will simply fail.
+ if (HasAPStream(pAnnotDict))
+ return false;
+
CPDF_Array* pColor = pAnnotDict->GetArrayFor(
type == FPDFANNOT_COLORTYPE_InteriorColor ? "IC" : "C");
*A =
@@ -315,8 +331,8 @@
DLLEXPORT FPDF_BOOL STDCALL
FPDFAnnot_SetAttachmentPoints(FPDF_ANNOTATION annot,
- FS_QUADPOINTSF quadPoints) {
- if (!annot || !FPDFAnnot_HasAttachmentPoints(annot))
+ const FS_QUADPOINTSF* quadPoints) {
+ if (!annot || !quadPoints || !FPDFAnnot_HasAttachmentPoints(annot))
return false;
CPDF_Dictionary* pAnnotDict =
@@ -324,20 +340,33 @@
if (!pAnnotDict)
return false;
+ // Update the "QuadPoints" entry in the annotation dictionary.
CPDF_Array* pQuadPoints = pAnnotDict->GetArrayFor("QuadPoints");
if (pQuadPoints)
pQuadPoints->Clear();
else
pQuadPoints = pAnnotDict->SetNewFor<CPDF_Array>("QuadPoints");
- pQuadPoints->AddNew<CPDF_Number>(quadPoints.x1);
- pQuadPoints->AddNew<CPDF_Number>(quadPoints.y1);
- pQuadPoints->AddNew<CPDF_Number>(quadPoints.x2);
- pQuadPoints->AddNew<CPDF_Number>(quadPoints.y2);
- pQuadPoints->AddNew<CPDF_Number>(quadPoints.x3);
- pQuadPoints->AddNew<CPDF_Number>(quadPoints.y3);
- pQuadPoints->AddNew<CPDF_Number>(quadPoints.x4);
- pQuadPoints->AddNew<CPDF_Number>(quadPoints.y4);
+ pQuadPoints->AddNew<CPDF_Number>(quadPoints->x1);
+ pQuadPoints->AddNew<CPDF_Number>(quadPoints->y1);
+ pQuadPoints->AddNew<CPDF_Number>(quadPoints->x2);
+ pQuadPoints->AddNew<CPDF_Number>(quadPoints->y2);
+ pQuadPoints->AddNew<CPDF_Number>(quadPoints->x3);
+ pQuadPoints->AddNew<CPDF_Number>(quadPoints->y3);
+ pQuadPoints->AddNew<CPDF_Number>(quadPoints->x4);
+ pQuadPoints->AddNew<CPDF_Number>(quadPoints->y4);
+
+ // If the annotation's appearance stream is defined, and the new quadpoints
+ // defines a bigger bounding box than the appearance stream currently
+ // specifies, then update the "BBox" entry in the AP dictionary too, since it
+ // comes from annotation dictionary's "QuadPoints" entry.
+ CPDF_Stream* pStream =
+ FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+ if (pStream) {
+ CFX_FloatRect newRect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
+ if (newRect.Contains(pStream->GetDict()->GetRectFor("BBox")))
+ pStream->GetDict()->SetRectFor("BBox", newRect);
+ }
return true;
}
@@ -351,25 +380,52 @@
if (!pAnnotDict)
return FS_QUADPOINTSF();
- CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
- if (!pArray)
- return FS_QUADPOINTSF();
-
+ // If the annotation's appearance stream is defined, then retrieve the
+ // quadpoints defined by the "BBox" entry in the AP dictionary, since its
+ // "BBox" entry comes from annotation dictionary's "QuadPoints" entry, but
+ // takes priority over "QuadPoints" when rendering. Otherwise, retrieve
+ // the "Quadpoints" entry from the annotation dictionary.
+ CPDF_Array* pArray;
FS_QUADPOINTSF quadPoints;
- quadPoints.x1 = pArray->GetNumberAt(0);
- quadPoints.y1 = pArray->GetNumberAt(1);
- quadPoints.x2 = pArray->GetNumberAt(2);
- quadPoints.y2 = pArray->GetNumberAt(3);
- quadPoints.x3 = pArray->GetNumberAt(4);
- quadPoints.y3 = pArray->GetNumberAt(5);
- quadPoints.x4 = pArray->GetNumberAt(6);
- quadPoints.y4 = pArray->GetNumberAt(7);
+ CPDF_Stream* pStream =
+ FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+ if (pStream) {
+ pArray = pStream->GetDict()->GetArrayFor("BBox");
+ if (!pArray)
+ return FS_QUADPOINTSF();
+
+ // Convert the BBox array into quadpoint coordinates. BBox array follows the
+ // order of a rectangle array: (left, bottom, right, up); and quadpoints
+ // follows the following order: (top-left vertex, top-right vertex, bottom-
+ // left vertex, bottom-right vertex).
+ quadPoints.x1 = pArray->GetNumberAt(0);
+ quadPoints.y1 = pArray->GetNumberAt(3);
+ quadPoints.x2 = pArray->GetNumberAt(2);
+ quadPoints.y2 = pArray->GetNumberAt(3);
+ quadPoints.x3 = pArray->GetNumberAt(0);
+ quadPoints.y3 = pArray->GetNumberAt(1);
+ quadPoints.x4 = pArray->GetNumberAt(2);
+ quadPoints.y4 = pArray->GetNumberAt(1);
+ } else {
+ pArray = pAnnotDict->GetArrayFor("QuadPoints");
+ if (!pArray)
+ return FS_QUADPOINTSF();
+
+ quadPoints.x1 = pArray->GetNumberAt(0);
+ quadPoints.y1 = pArray->GetNumberAt(1);
+ quadPoints.x2 = pArray->GetNumberAt(2);
+ quadPoints.y2 = pArray->GetNumberAt(3);
+ quadPoints.x3 = pArray->GetNumberAt(4);
+ quadPoints.y3 = pArray->GetNumberAt(5);
+ quadPoints.x4 = pArray->GetNumberAt(6);
+ quadPoints.y4 = pArray->GetNumberAt(7);
+ }
return quadPoints;
}
DLLEXPORT FPDF_BOOL STDCALL FPDFAnnot_SetRect(FPDF_ANNOTATION annot,
- FS_RECTF rect) {
- if (!annot)
+ const FS_RECTF* rect) {
+ if (!annot || !rect)
return false;
CPDF_Dictionary* pAnnotDict =
@@ -377,16 +433,23 @@
if (!pAnnotDict)
return false;
- CPDF_Array* pRect = pAnnotDict->GetArrayFor("Rect");
- if (pRect)
- pRect->Clear();
- else
- pRect = pAnnotDict->SetNewFor<CPDF_Array>("Rect");
+ CFX_FloatRect newRect(rect->left, rect->bottom, rect->right, rect->top);
- pRect->AddNew<CPDF_Number>(rect.left);
- pRect->AddNew<CPDF_Number>(rect.bottom);
- pRect->AddNew<CPDF_Number>(rect.right);
- pRect->AddNew<CPDF_Number>(rect.top);
+ // Update the "Rect" entry in the annotation dictionary.
+ pAnnotDict->SetRectFor("Rect", newRect);
+
+ // If the annotation's appearance stream is defined, the annotation is of a
+ // type that does not have quadpoints, and the new rectangle is bigger than
+ // the current bounding box, then update the "BBox" entry in the AP
+ // dictionary too, since its "BBox" entry comes from annotation dictionary's
+ // "Rect" entry.
+ if (FPDFAnnot_HasAttachmentPoints(annot))
+ return true;
+
+ CPDF_Stream* pStream =
+ FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+ if (pStream && newRect.Contains(pStream->GetDict()->GetRectFor("BBox")))
+ pStream->GetDict()->SetRectFor("BBox", newRect);
return true;
}
@@ -399,9 +462,19 @@
if (!pAnnotDict)
return FS_RECTF();
- CFX_FloatRect rt = pAnnotDict->GetRectFor("Rect");
- if (rt.IsEmpty())
- return FS_RECTF();
+ // If the annotation's appearance stream is defined and the annotation is of
+ // a type that does not have quadpoints, then retrieve the rectangle defined
+ // by the "BBox" entry in the AP dictionary, since its "BBox" entry comes
+ // from annotation dictionary's "Rect" entry, but takes priority over "Rect"
+ // when rendering. Otherwise, retrieve the "Rect" entry from the annotation
+ // dictionary.
+ CFX_FloatRect rt;
+ CPDF_Stream* pStream =
+ FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+ if (!pStream || FPDFAnnot_HasAttachmentPoints(annot))
+ rt = pAnnotDict->GetRectFor("Rect");
+ else
+ rt = pStream->GetDict()->GetRectFor("BBox");
FS_RECTF rect;
rect.left = rt.left;
diff --git a/fpdfsdk/fpdfannot_embeddertest.cpp b/fpdfsdk/fpdfannot_embeddertest.cpp
index 4d6ae1c..9d9e2d3 100644
--- a/fpdfsdk/fpdfannot_embeddertest.cpp
+++ b/fpdfsdk/fpdfannot_embeddertest.cpp
@@ -218,7 +218,7 @@
rect.bottom = 150;
rect.right = 53;
rect.top = 165;
- ASSERT_TRUE(FPDFAnnot_SetRect(annot, rect));
+ ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
// Check that the annotation rectangle has been set correctly.
rect = FPDFAnnot_GetRect(annot);
EXPECT_EQ(35.f, rect.left);
@@ -269,7 +269,7 @@
ASSERT_TRUE(annot);
quadpoints.x1 = 140.802643f;
quadpoints.x3 = 140.802643f;
- ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, quadpoints));
+ ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, &quadpoints));
FPDFPage_CloseAnnot(annot);
// Save the document, closing the page and document.
@@ -306,3 +306,89 @@
FPDF_ClosePage(new_page);
FPDF_CloseDocument(new_doc);
}
+
+TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
+ // Open a file with four annotations and load its first page.
+ ASSERT_TRUE(OpenDocument("annotation_highlight_square_with_ap.pdf"));
+ FPDF_PAGE page = FPDF_LoadPage(document(), 0);
+ ASSERT_TRUE(page);
+ EXPECT_EQ(4, FPDFPage_GetAnnotCount(page));
+
+ // Retrieve the highlight annotation which has its AP stream already defined.
+ FPDF_ANNOTATION annot = FPDFPage_GetAnnot(page, 0);
+ ASSERT_TRUE(annot);
+ EXPECT_EQ(FPDF_ANNOT_HIGHLIGHT, FPDFAnnot_GetSubtype(annot));
+
+ // Check that color cannot be set when an AP stream is defined already.
+ EXPECT_FALSE(
+ FPDFAnnot_SetColor(annot, FPDFANNOT_COLORTYPE_Color, 51, 102, 153, 204));
+
+ // Check that when getting the attachment points, bounding box points are
+ // returned since this is a markup annotation with AP defined.
+ FS_QUADPOINTSF quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+ EXPECT_NEAR(0.f, quadpoints.x1, 0.001f);
+ EXPECT_NEAR(16.9955f, quadpoints.y1, 0.001f);
+ EXPECT_NEAR(68.5953f, quadpoints.x4, 0.001f);
+ EXPECT_NEAR(0.f, quadpoints.y4, 0.001f);
+
+ // Check that when new attachment points define a smaller bounding box, the
+ // bounding box does not get updated.
+ quadpoints.x1 = 1.0f;
+ quadpoints.x3 = 1.0f;
+ ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, &quadpoints));
+ FS_QUADPOINTSF new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+ EXPECT_NE(quadpoints.x1, new_quadpoints.x1);
+
+ // Check that the bounding box gets updated successfully when valid attachment
+ // points are set.
+ quadpoints.x1 = 0.f;
+ quadpoints.y1 = 721.792f;
+ quadpoints.x2 = 133.055f;
+ quadpoints.y2 = 721.792f;
+ quadpoints.x3 = 0.f;
+ quadpoints.x4 = 133.055f;
+ ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, &quadpoints));
+ new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+ EXPECT_EQ(quadpoints.x1, new_quadpoints.x1);
+ EXPECT_EQ(quadpoints.y1, new_quadpoints.y1);
+ EXPECT_EQ(quadpoints.x4, new_quadpoints.x4);
+ EXPECT_EQ(quadpoints.y4, new_quadpoints.y4);
+
+ // Check that when getting the annotation rectangle, rectangle points are
+ // returned, but not bounding box points.
+ FS_RECTF rect = FPDFAnnot_GetRect(annot);
+ EXPECT_NEAR(67.7299f, rect.left, 0.001f);
+ EXPECT_NEAR(704.296f, rect.bottom, 0.001f);
+ EXPECT_NEAR(136.325f, rect.right, 0.001f);
+ EXPECT_NEAR(721.292f, rect.top, 0.001f);
+
+ // Check that the rectangle gets updated successfully when a valid rectangle
+ // is set, and that the bounding box is not modified.
+ rect.left = 0.f;
+ rect.bottom = 0.f;
+ rect.right = 134.055f;
+ rect.top = 722.792f;
+ ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
+ FS_RECTF new_rect = FPDFAnnot_GetRect(annot);
+ EXPECT_EQ(rect.right, new_rect.right);
+ new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+ EXPECT_NE(rect.right, new_quadpoints.x2);
+
+ FPDFPage_CloseAnnot(annot);
+
+ // Retrieve the square annotation which has its AP stream already defined.
+ annot = FPDFPage_GetAnnot(page, 2);
+ ASSERT_TRUE(annot);
+ EXPECT_EQ(FPDF_ANNOT_SQUARE, FPDFAnnot_GetSubtype(annot));
+
+ // Check that the rectangle and the bounding box get updated successfully when
+ // a valid rectangle is set, since this is not a markup annotation.
+ rect = FPDFAnnot_GetRect(annot);
+ rect.right += 1.f;
+ ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
+ new_rect = FPDFAnnot_GetRect(annot);
+ EXPECT_EQ(rect.right, new_rect.right);
+
+ FPDFPage_CloseAnnot(annot);
+ UnloadPage(page);
+}
diff --git a/fpdfsdk/fsdk_define.h b/fpdfsdk/fsdk_define.h
index 1aec6cb..12b9b47 100644
--- a/fpdfsdk/fsdk_define.h
+++ b/fpdfsdk/fsdk_define.h
@@ -62,9 +62,6 @@
CPDF_Page* CPDFPageFromFPDFPage(FPDF_PAGE page);
-FPDF_ANNOTATION FPDFAnnotationFromCPDFDictionary(CPDF_Dictionary* pDict);
-CPDF_Dictionary* CPDFDictionaryFromFPDFAnnotation(FPDF_ANNOTATION annot);
-
CPDF_PathObject* CPDFPathObjectFromFPDFPageObject(FPDF_PAGEOBJECT page_object);
CFX_DIBitmap* CFXBitmapFromFPDFBitmap(FPDF_BITMAP bitmap);
diff --git a/public/fpdf_annot.h b/public/fpdf_annot.h
index 0a1646f..d997927 100644
--- a/public/fpdf_annot.h
+++ b/public/fpdf_annot.h
@@ -106,7 +106,9 @@
DLLEXPORT FPDF_ANNOTATION_SUBTYPE STDCALL
FPDFAnnot_GetSubtype(FPDF_ANNOTATION annot);
-// Set the color of an annotation.
+// Set the color of an annotation. Fails when called on annotations with
+// appearance streams already defined; instead use
+// FPDFPath_Set{Stroke|Fill}Color().
//
// annot - handle to an annotation.
// type - type of the color to be set.
@@ -122,7 +124,9 @@
unsigned int A);
// Get the color of an annotation. If no color is specified, default to yellow
-// for highlight annotation, black for all else.
+// for highlight annotation, black for all else. Fails when called on
+// annotations with appearance streams already defined; instead use
+// FPDFPath_Get{Stroke|Fill}Color().
//
// annot - handle to an annotation.
// type - type of the color requested.
@@ -151,16 +155,21 @@
DLLEXPORT FPDF_BOOL STDCALL
FPDFAnnot_HasAttachmentPoints(FPDF_ANNOTATION annot);
-// Set the attachment points (i.e. quadpoints) of an annotation.
+// Set the attachment points (i.e. quadpoints) of an annotation. If the
+// annotation's appearance stream is defined and this annotation is of a type
+// with quadpoints, then update the bounding box too.
//
// annot - handle to an annotation.
// quadPoints - the quadpoints to be set.
//
// Returns true if successful, false otherwise.
DLLEXPORT FPDF_BOOL STDCALL
-FPDFAnnot_SetAttachmentPoints(FPDF_ANNOTATION annot, FS_QUADPOINTSF quadPoints);
+FPDFAnnot_SetAttachmentPoints(FPDF_ANNOTATION annot,
+ const FS_QUADPOINTSF* quadPoints);
-// Get the attachment points (i.e. quadpoints) of an annotation.
+// Get the attachment points (i.e. quadpoints) of an annotation. If the
+// annotation's appearance stream is defined and this annotation is of a type
+// with quadpoints, then return the bounding box it specifies instead.
//
// annot - handle to an annotation.
//
@@ -168,16 +177,20 @@
DLLEXPORT FS_QUADPOINTSF STDCALL
FPDFAnnot_GetAttachmentPoints(FPDF_ANNOTATION annot);
-// Set the annotation rectangle defining the location of the annotation.
+// Set the annotation rectangle defining the location of the annotation. If the
+// annotation's appearance stream is defined and this annotation is of a type
+// without quadpoints, then update the bounding box too.
//
// annot - handle to an annotation.
// rect - the annotation rectangle to be set.
//
// Returns true if successful, false otherwise.
DLLEXPORT FPDF_BOOL STDCALL FPDFAnnot_SetRect(FPDF_ANNOTATION annot,
- FS_RECTF rect);
+ const FS_RECTF* rect);
-// Get the annotation rectangle defining the location of the annotation.
+// Get the annotation rectangle defining the location of the annotation. If the
+// annotation's appearance stream is defined and this annotation is of a type
+// without quadpoints, then return the bounding box it specifies instead.
//
// annot - handle to an annotation.
//
diff --git a/testing/resources/annotation_highlight_square_with_ap.pdf b/testing/resources/annotation_highlight_square_with_ap.pdf
new file mode 100644
index 0000000..64d5c40
--- /dev/null
+++ b/testing/resources/annotation_highlight_square_with_ap.pdf
Binary files differ
