Calculate AP for multi-line markup annotations correctly

Currently, when constructing AP for multi-line markup annotations, we
only take into account the first set of quadpoints, resulting in only
the first line of the annotation being displayed if the annotation spans
multiple lines.

This CL, initially written by Jane Liu <janeliulwq@google.com>
(https://pdfium-review.googlesource.com/12010) takes into account all
the quadpoints, so multi-line annotations can be displayed correctly.

BUG=pdfium:876

Change-Id: I8be10ee38e01eb6525ddef556df1b727189455c7
Reviewed-on: https://pdfium-review.googlesource.com/28590
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

7 files changed, 333 insertions, 58 deletions

diff --git a/core/fpdfdoc/cpdf_annot.cpp b/core/fpdfdoc/cpdf_annot.cpp
index cc70a3087c..91cdb12d9f 100644
--- a/core/fpdfdoc/cpdf_annot.cpp
+++ b/core/fpdfdoc/cpdf_annot.cpp
@@ -6,6 +6,7 @@
 
 #include "core/fpdfdoc/cpdf_annot.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "core/fpdfapi/page/cpdf_form.h"
@@ -158,7 +159,7 @@ CFX_FloatRect CPDF_Annot::RectForDrawing() const {
   bool bShouldUseQuadPointsCoords =
       m_bIsTextMarkupAnnotation && m_bHasGeneratedAP;
   if (bShouldUseQuadPointsCoords)
-    return RectFromQuadPoints(m_pAnnotDict.Get());
+    return BoundingRectFromQuadPoints(m_pAnnotDict.Get());
 
   return m_pAnnotDict->GetRectFor("Rect");
 }
@@ -205,11 +206,8 @@ CPDF_Form* CPDF_Annot::GetAPForm(const CPDF_Page* pPage, AppearanceMode mode) {
 }
 
 // Static.
-CFX_FloatRect CPDF_Annot::RectFromQuadPoints(CPDF_Dictionary* pAnnotDict) {
-  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
-  if (!pArray)
-    return CFX_FloatRect();
-
+CFX_FloatRect CPDF_Annot::RectFromQuadPointsArray(const CPDF_Array* pArray,
+                                                  size_t nIndex) {
   // QuadPoints are defined with 4 pairs of numbers
   // ([ pair0, pair1, pair2, pair3 ]), where
   // pair0 = top_left
@@ -217,11 +215,40 @@ CFX_FloatRect CPDF_Annot::RectFromQuadPoints(CPDF_Dictionary* pAnnotDict) {
   // pair2 = bottom_left
   // pair3 = bottom_right
   //
-  // On the other hand, /Rect is define as 2 pairs [pair0, pair1] where:
+  // On the other hand, /Rect is defined 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));
+
+  return CFX_FloatRect(
+      pArray->GetNumberAt(4 + nIndex * 8), pArray->GetNumberAt(5 + nIndex * 8),
+      pArray->GetNumberAt(2 + nIndex * 8), pArray->GetNumberAt(3 + nIndex * 8));
+}
+
+// Static.
+CFX_FloatRect CPDF_Annot::BoundingRectFromQuadPoints(
+    CPDF_Dictionary* pAnnotDict) {
+  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
+  if (!pArray)
+    return CFX_FloatRect();
+
+  CFX_FloatRect ret = RectFromQuadPointsArray(pArray, 0);
+  size_t nQuadPointCount = QuadPointCount(pArray);
+  for (size_t i = 1; i < nQuadPointCount; ++i) {
+    CFX_FloatRect rect = RectFromQuadPointsArray(pArray, i);
+    ret.Union(rect);
+  }
+  return ret;
+}
+
+// Static.
+CFX_FloatRect CPDF_Annot::RectFromQuadPoints(CPDF_Dictionary* pAnnotDict,
+                                             size_t nIndex) {
+  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
+
+  if (!pArray)
+    return CFX_FloatRect();
+
+  return RectFromQuadPointsArray(pArray, nIndex);
 }
 
 // Static.
@@ -348,6 +375,11 @@ ByteString CPDF_Annot::AnnotSubtypeToString(CPDF_Annot::Subtype nSubtype) {
   return "";
 }
 
+// Static.
+size_t CPDF_Annot::QuadPointCount(const CPDF_Array* pArray) {
+  return pArray->GetCount() / 8;
+}
+
 bool CPDF_Annot::DrawAppearance(CPDF_Page* pPage,
                                 CFX_RenderDevice* pDevice,
                                 const CFX_Matrix& mtUser2Device,
diff --git a/core/fpdfdoc/cpdf_annot.h b/core/fpdfdoc/cpdf_annot.h
index 499c62dcba..3f1164ad18 100644
--- a/core/fpdfdoc/cpdf_annot.h
+++ b/core/fpdfdoc/cpdf_annot.h
@@ -16,6 +16,7 @@
 #include "core/fxcrt/maybe_owned.h"
 
 class CFX_RenderDevice;
+class CPDF_Array;
 class CPDF_Dictionary;
 class CPDF_Document;
 class CPDF_Form;
@@ -66,7 +67,12 @@ class CPDF_Annot {
   static bool IsAnnotationHidden(CPDF_Dictionary* pAnnotDict);
   static CPDF_Annot::Subtype StringToAnnotSubtype(const ByteString& sSubtype);
   static ByteString AnnotSubtypeToString(CPDF_Annot::Subtype nSubtype);
-  static CFX_FloatRect RectFromQuadPoints(CPDF_Dictionary* pAnnotDict);
+  static CFX_FloatRect RectFromQuadPointsArray(const CPDF_Array* pArray,
+                                               size_t nIndex);
+  static CFX_FloatRect BoundingRectFromQuadPoints(CPDF_Dictionary* pAnnotDict);
+  static CFX_FloatRect RectFromQuadPoints(CPDF_Dictionary* pAnnotDict,
+                                          size_t nIndex);
+  static size_t QuadPointCount(const CPDF_Array* pArray);
 
   // The second constructor does not take ownership of the dictionary.
   CPDF_Annot(std::unique_ptr<CPDF_Dictionary> pDict, CPDF_Document* pDocument);
diff --git a/core/fpdfdoc/cpvt_generateap.cpp b/core/fpdfdoc/cpvt_generateap.cpp
index eb3e22d870..e2fbcc9720 100644
--- a/core/fpdfdoc/cpvt_generateap.cpp
+++ b/core/fpdfdoc/cpvt_generateap.cpp
@@ -519,7 +519,7 @@ void GenerateAndSetAPDict(CPDF_Document* pDoc,
   pStreamDict->SetMatrixFor("Matrix", CFX_Matrix());
 
   CFX_FloatRect rect = bIsTextMarkupAnnotation
-                           ? CPDF_Annot::RectFromQuadPoints(pAnnotDict)
+                           ? CPDF_Annot::BoundingRectFromQuadPoints(pAnnotDict)
                            : pAnnotDict->GetRectFor("Rect");
   pStreamDict->SetRectFor("BBox", rect);
   pStreamDict->SetFor("Resources", std::move(pResourceDict));
@@ -606,13 +606,18 @@ bool GenerateHighlightAP(CPDF_Document* pDoc, CPDF_Dictionary* pAnnotDict) {
                                           CFX_Color(CFX_Color::kRGB, 1, 1, 0),
                                           PaintOperation::FILL);
 
-  CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
-  rect.Normalize();
+  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
+  if (pArray) {
+    size_t nQuadPointCount = CPDF_Annot::QuadPointCount(pArray);
+    for (size_t i = 0; i < nQuadPointCount; ++i) {
+      CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict, i);
+      rect.Normalize();
 
-  sAppStream << rect.left << " " << rect.top << " m " << rect.right << " "
-             << rect.top << " l " << rect.right << " " << rect.bottom << " l "
-             << rect.left << " " << rect.bottom << " l "
-             << "h f\n";
+      sAppStream << rect.left << " " << rect.top << " m " << rect.right << " "
+                 << rect.top << " l " << rect.right << " " << rect.bottom
+                 << " l " << rect.left << " " << rect.bottom << " l h f\n";
+    }
+  }
 
   auto pExtGStateDict =
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Multiply");
@@ -708,13 +713,18 @@ bool GenerateUnderlineAP(CPDF_Document* pDoc, CPDF_Dictionary* pAnnotDict) {
                                           CFX_Color(CFX_Color::kRGB, 0, 0, 0),
                                           PaintOperation::STROKE);
 
-  CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
-  rect.Normalize();
-
-  float fLineWidth = 1.0;
-  sAppStream << fLineWidth << " w " << rect.left << " "
-             << rect.bottom + fLineWidth << " m " << rect.right << " "
-             << rect.bottom + fLineWidth << " l S\n";
+  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
+  if (pArray) {
+    static constexpr float kLineWidth = 1.0f;
+    sAppStream << kLineWidth << " w ";
+    size_t nQuadPointCount = CPDF_Annot::QuadPointCount(pArray);
+    for (size_t i = 0; i < nQuadPointCount; ++i) {
+      CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict, i);
+      rect.Normalize();
+      sAppStream << rect.left << " " << rect.bottom + kLineWidth << " m "
+                 << rect.right << " " << rect.bottom + kLineWidth << " l S\n";
+    }
+  }
 
   auto pExtGStateDict =
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
@@ -817,36 +827,38 @@ bool GenerateSquigglyAP(CPDF_Document* pDoc, CPDF_Dictionary* pAnnotDict) {
                                           CFX_Color(CFX_Color::kRGB, 0, 0, 0),
                                           PaintOperation::STROKE);
 
-  CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
-  rect.Normalize();
-
-  float fLineWidth = 1.0;
-  sAppStream << fLineWidth << " w ";
-
-  const float fDelta = 2.0;
-  const float fTop = rect.bottom + fDelta;
-  const float fBottom = rect.bottom;
-
-  sAppStream << rect.left << " " << fTop << " m ";
-
-  float fX = rect.left + fDelta;
-  bool isUpwards = false;
+  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
+  if (pArray) {
+    static constexpr float kLineWidth = 1.0f;
+    static constexpr float kDelta = 2.0f;
+    sAppStream << kLineWidth << " w ";
+    size_t nQuadPointCount = CPDF_Annot::QuadPointCount(pArray);
+    for (size_t i = 0; i < nQuadPointCount; ++i) {
+      CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict, i);
+      rect.Normalize();
+
+      const float fTop = rect.bottom + kDelta;
+      const float fBottom = rect.bottom;
+      sAppStream << rect.left << " " << fTop << " m ";
+
+      float fX = rect.left + kDelta;
+      bool isUpwards = false;
+      while (fX < rect.right) {
+        sAppStream << fX << " " << (isUpwards ? fTop : fBottom) << " l ";
+        fX += kDelta;
+        isUpwards = !isUpwards;
+      }
 
-  while (fX < rect.right) {
-    sAppStream << fX << " " << (isUpwards ? fTop : fBottom) << " l ";
+      float fRemainder = rect.right - (fX - kDelta);
+      if (isUpwards)
+        sAppStream << rect.right << " " << fBottom + fRemainder << " l ";
+      else
+        sAppStream << rect.right << " " << fTop - fRemainder << " l ";
 
-    fX += fDelta;
-    isUpwards = !isUpwards;
+      sAppStream << "S\n";
+    }
   }
 
-  float fRemainder = rect.right - (fX - fDelta);
-  if (isUpwards)
-    sAppStream << rect.right << " " << fBottom + fRemainder << " l ";
-  else
-    sAppStream << rect.right << " " << fTop - fRemainder << " l ";
-
-  sAppStream << "S\n";
-
   auto pExtGStateDict =
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
   auto pResourceDict =
@@ -865,13 +877,19 @@ bool GenerateStrikeOutAP(CPDF_Document* pDoc, CPDF_Dictionary* pAnnotDict) {
                                           CFX_Color(CFX_Color::kRGB, 0, 0, 0),
                                           PaintOperation::STROKE);
 
-  CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
-  rect.Normalize();
-
-  float fLineWidth = 1.0;
-  float fY = (rect.top + rect.bottom) / 2;
-  sAppStream << fLineWidth << " w " << rect.left << " " << fY << " m "
-             << rect.right << " " << fY << " l S\n";
+  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
+  if (pArray) {
+    static constexpr float kLineWidth = 1.0f;
+    size_t nQuadPointCount = CPDF_Annot::QuadPointCount(pArray);
+    for (size_t i = 0; i < nQuadPointCount; ++i) {
+      CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict, i);
+      rect.Normalize();
+
+      float fY = (rect.top + rect.bottom) / 2;
+      sAppStream << kLineWidth << " w " << rect.left << " " << fY << " m "
+                 << rect.right << " " << fY << " l S\n";
+    }
+  }
 
   auto pExtGStateDict =
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
diff --git a/fpdfsdk/fpdfannot.cpp b/fpdfsdk/fpdfannot.cpp
index 29a6751bff..84c6c03194 100644
--- a/fpdfsdk/fpdfannot.cpp
+++ b/fpdfsdk/fpdfannot.cpp
@@ -614,7 +614,7 @@ FPDFAnnot_SetAttachmentPoints(FPDF_ANNOTATION annot,
   CPDF_Stream* pStream =
       FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
   if (pStream) {
-    CFX_FloatRect newRect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
+    CFX_FloatRect newRect = CPDF_Annot::BoundingRectFromQuadPoints(pAnnotDict);
     if (newRect.Contains(pStream->GetDict()->GetRectFor("BBox")))
       pStream->GetDict()->SetRectFor("BBox", newRect);
   }
diff --git a/fpdfsdk/fpdfannot_embeddertest.cpp b/fpdfsdk/fpdfannot_embeddertest.cpp
index 61cd2aae3d..b96460a395 100644
--- a/fpdfsdk/fpdfannot_embeddertest.cpp
+++ b/fpdfsdk/fpdfannot_embeddertest.cpp
@@ -45,6 +45,20 @@ TEST_F(FPDFAnnotEmbeddertest, RenderAnnotWithOnlyRolloverAP) {
   UnloadPage(page);
 }
 
+TEST_F(FPDFAnnotEmbeddertest, RenderMultilineMarkupAnnotWithoutAP) {
+  const char md5_hash[] = "76512832d88017668d9acc7aacd13dae";
+  // Open a file with two multiline markup annotations.
+  ASSERT_TRUE(OpenDocument("annotation_markup_multiline_no_ap.pdf"));
+  FPDF_PAGE page = LoadPage(0);
+  ASSERT_TRUE(page);
+
+  std::unique_ptr<void, FPDFBitmapDeleter> bitmap =
+      RenderLoadedPageWithFlags(page, FPDF_ANNOT);
+  CompareBitmap(bitmap.get(), 595, 842, md5_hash);
+
+  UnloadPage(page);
+}
+
 TEST_F(FPDFAnnotEmbeddertest, ExtractHighlightLongContent) {
   // Open a file with one annotation and load its first page.
   ASSERT_TRUE(OpenDocument("annotation_highlight_long_content.pdf"));
diff --git a/testing/resources/annotation_markup_multiline_no_ap.in b/testing/resources/annotation_markup_multiline_no_ap.in
new file mode 100644
index 0000000000..5107831ac9
--- /dev/null
+++ b/testing/resources/annotation_markup_multiline_no_ap.in
@@ -0,0 +1,97 @@
+{{header}}
+
+{{object 1 0}} <<
+  /Type /Catalog
+  /Pages 2 0 R
+>>
+endobj
+
+{{object 2 0}} <<
+  /Type /Pages
+  /Count 1
+  /Kids [ 3 0 R ]
+>>
+endobj
+
+{{object 3 0}} <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 595 842]
+  /Annots [
+    4 0 R
+    5 0 R
+    6 0 R
+    7 0 R
+  ]
+  /Tabs /R
+>>
+endobj
+
+{{object 4 0}} <<
+  /Type /Annot
+  /Subtype /Highlight
+  /Rect [ 108 602 506 640 ]
+  /NM (Hilight-1)
+  /F 4
+  /QuadPoints [
+    107.7896 639.9486 505.8939 639.9486 107.7896 629.1634 505.8939 629.1634
+    107.7896 626.2871 505.8939 626.2871 107.7896 615.5011 505.8939 615.5011
+    107.7896 612.6248 380.1389 612.6248 107.7896 601.8397 380.1389 601.8397
+  ]
+  /C [ 1 1 0 ]
+  /Contents ()
+>>
+endobj
+
+{{object 5 0}} <<
+  /Type /Annot
+  /Subtype /Underline
+  /Rect [ 108 552 506 590 ]
+  /NM (Underline-1)
+  /F 4
+  /QuadPoints [
+    107.7896 589.9486 505.8939 589.9486 107.7896 579.1634 505.8939 579.1634
+    107.7896 576.2871 505.8939 576.2871 107.7896 565.5011 505.8939 565.5011
+    107.7896 562.6248 380.1389 562.6248 107.7896 551.8397 380.1389 551.8397
+  ]
+  /C [ 0 0 0 ]
+  /Contents ()
+>>
+endobj
+
+{{object 6 0}} <<
+  /Type /Annot
+  /Subtype /Squiggly
+  /Rect [ 108 502 506 540 ]
+  /NM (Squiggly-1)
+  /F 4
+  /QuadPoints [
+    107.7896 539.9486 505.8939 539.9486 107.7896 529.1634 505.8939 529.1634
+    107.7896 526.2871 505.8939 526.2871 107.7896 515.5011 505.8939 515.5011
+    107.7896 512.6248 380.1389 512.6248 107.7896 501.8397 380.1389 501.8397
+  ]
+  /C [ 0 0 0 ]
+  /Contents ()
+>>
+endobj
+
+{{object 7 0}} <<
+  /Type /Annot
+  /Subtype /StrikeOut
+  /Rect [ 108 452 506 490 ]
+  /NM (StrikeOut-1)
+  /F 4
+  /QuadPoints [
+    107.7896 489.9486 505.8939 489.9486 107.7896 479.1634 505.8939 479.1634
+    107.7896 476.2871 505.8939 476.2871 107.7896 465.5011 505.8939 465.5011
+    107.7896 462.6248 380.1389 462.6248 107.7896 451.8397 380.1389 451.8397
+  ]
+  /C [ 0 0 0 ]
+  /Contents ()
+>>
+endobj
+
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/annotation_markup_multiline_no_ap.pdf b/testing/resources/annotation_markup_multiline_no_ap.pdf
new file mode 100644
index 0000000000..deac036e18
--- /dev/null
+++ b/testing/resources/annotation_markup_multiline_no_ap.pdf
@@ -0,0 +1,108 @@
+%PDF-1.7
+% ò¤ô
+
+1 0 obj <<
+  /Type /Catalog
+  /Pages 2 0 R
+>>
+endobj
+
+2 0 obj <<
+  /Type /Pages
+  /Count 1
+  /Kids [ 3 0 R ]
+>>
+endobj
+
+3 0 obj <<
+  /Type /Page
+  /Parent 2 0 R
+  /MediaBox [0 0 595 842]
+  /Annots [
+    4 0 R
+    5 0 R
+    6 0 R
+    7 0 R
+  ]
+  /Tabs /R
+>>
+endobj
+
+4 0 obj <<
+  /Type /Annot
+  /Subtype /Highlight
+  /Rect [ 108 602 506 640 ]
+  /NM (Hilight-1)
+  /F 4
+  /QuadPoints [
+    107.7896 639.9486 505.8939 639.9486 107.7896 629.1634 505.8939 629.1634
+    107.7896 626.2871 505.8939 626.2871 107.7896 615.5011 505.8939 615.5011
+    107.7896 612.6248 380.1389 612.6248 107.7896 601.8397 380.1389 601.8397
+  ]
+  /C [ 1 1 0 ]
+  /Contents ()
+>>
+endobj
+
+5 0 obj <<
+  /Type /Annot
+  /Subtype /Underline
+  /Rect [ 108 552 506 590 ]
+  /NM (Underline-1)
+  /F 4
+  /QuadPoints [
+    107.7896 589.9486 505.8939 589.9486 107.7896 579.1634 505.8939 579.1634
+    107.7896 576.2871 505.8939 576.2871 107.7896 565.5011 505.8939 565.5011
+    107.7896 562.6248 380.1389 562.6248 107.7896 551.8397 380.1389 551.8397
+  ]
+  /C [ 0 0 0 ]
+  /Contents ()
+>>
+endobj
+
+6 0 obj <<
+  /Type /Annot
+  /Subtype /Squiggly
+  /Rect [ 108 502 506 540 ]
+  /NM (Squiggly-1)
+  /F 4
+  /QuadPoints [
+    107.7896 539.9486 505.8939 539.9486 107.7896 529.1634 505.8939 529.1634
+    107.7896 526.2871 505.8939 526.2871 107.7896 515.5011 505.8939 515.5011
+    107.7896 512.6248 380.1389 512.6248 107.7896 501.8397 380.1389 501.8397
+  ]
+  /C [ 0 0 0 ]
+  /Contents ()
+>>
+endobj
+
+7 0 obj <<
+  /Type /Annot
+  /Subtype /StrikeOut
+  /Rect [ 108 452 506 490 ]
+  /NM (StrikeOut-1)
+  /F 4
+  /QuadPoints [
+    107.7896 489.9486 505.8939 489.9486 107.7896 479.1634 505.8939 479.1634
+    107.7896 476.2871 505.8939 476.2871 107.7896 465.5011 505.8939 465.5011
+    107.7896 462.6248 380.1389 462.6248 107.7896 451.8397 380.1389 451.8397
+  ]
+  /C [ 0 0 0 ]
+  /Contents ()
+>>
+endobj
+
+xref
+0 8
+0000000000 65535 f 
+0000000016 00000 n 
+0000000070 00000 n 
+0000000136 00000 n 
+0000000281 00000 n 
+0000000671 00000 n 
+0000001063 00000 n 
+0000001453 00000 n 
+trailer<< /Root 1 0 R /Size 8 >>
+startxref
+1845
+%%EOF