Cleanup some CPDFSDK_PageView annotation code.

fpdfsdk/cba_annotiterator.cpp

 // Copyright 2016 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.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "fpdfsdk/cba_annotiterator.h"
 
 #include "core/fpdfapi/page/cpdf_page.h"
 #include "fpdfsdk/cpdfsdk_annot.h"
@@ skipping 48 matching lines @@
 
 CPDFSDK_Annot* CBA_AnnotIterator::GetPrevAnnot(CPDFSDK_Annot* pAnnot) {
   auto iter = std::find(m_Annots.begin(), m_Annots.end(), pAnnot);
   if (iter == m_Annots.end())
     return nullptr;
   if (iter == m_Annots.begin())
     iter = m_Annots.end();
   return *(--iter);
 }
 
+void CBA_AnnotIterator::CollectAnnots(std::vector<CPDFSDK_Annot*>* pArray) {
+  for (auto pAnnot : m_pPageView->GetAnnotList()) {
+    if (pAnnot->GetAnnotSubtype() == m_nAnnotSubtype &&
+        !pAnnot->IsSignatureWidget()) {
+      pArray->push_back(pAnnot);
+    }
+  }
+}
+
+CFX_FloatRect CBA_AnnotIterator::AddToAnnotsList(
+    std::vector<CPDFSDK_Annot*>& sa,
+    size_t idx) {
+  CPDFSDK_Annot* pLeftTopAnnot = sa[idx];
+  CFX_FloatRect rcLeftTop = GetAnnotRect(pLeftTopAnnot);
+  m_Annots.push_back(pLeftTopAnnot);
+  sa.erase(sa.begin() + idx);
+  return rcLeftTop;
+}
+
+void CBA_AnnotIterator::AddSelectedToAnnots(std::vector<CPDFSDK_Annot*>& sa,
+                                            std::vector<size_t>& aSelect) {
+  for (size_t i = 0; i < aSelect.size(); ++i)
+    m_Annots.push_back(sa[aSelect[i]]);
+
+  for (int i = aSelect.size() - 1; i >= 0; --i)
+    sa.erase(sa.begin() + aSelect[i]);
+}
+
 void CBA_AnnotIterator::GenerateResults() {
   switch (m_eTabOrder) {
-    case STRUCTURE: {
-      for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
-        CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-        if (pAnnot->GetAnnotSubtype() == m_nAnnotSubtype &&
-            !pAnnot->IsSignatureWidget())
-          m_Annots.push_back(pAnnot);
-      }
+    case STRUCTURE:
+      CollectAnnots(&m_Annots);
       break;
-    }
+
     case ROW: {
       std::vector<CPDFSDK_Annot*> sa;
-      for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
-        CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-        if (pAnnot->GetAnnotSubtype() == m_nAnnotSubtype &&
-            !pAnnot->IsSignatureWidget())
-          sa.push_back(pAnnot);
-      }
+      CollectAnnots(&sa);
 
       std::sort(sa.begin(), sa.end(), CompareByLeftAscending);

[Tom Sepez, 2016/10/05 17:59:52]

super-super-super-nit:  I'd put the blank line after this so that the collect
and sort appear on adjacent lines.

[dsinclair, 2016/10/05 18:26:56]

Done.

       while (!sa.empty()) {
         int nLeftTopIndex = -1;
         FX_FLOAT fTop = 0.0f;
         for (int i = sa.size() - 1; i >= 0; i--) {
           CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
           if (rcAnnot.top > fTop) {
             nLeftTopIndex = i;
             fTop = rcAnnot.top;
           }
         }
-        if (nLeftTopIndex >= 0) {
-          CPDFSDK_Annot* pLeftTopAnnot = sa[nLeftTopIndex];
-          CFX_FloatRect rcLeftTop = GetAnnotRect(pLeftTopAnnot);
-          m_Annots.push_back(pLeftTopAnnot);
-          sa.erase(sa.begin() + nLeftTopIndex);
+        if (nLeftTopIndex < 0)
+          continue;
 
-          std::vector<int> aSelect;
-          for (size_t i = 0; i < sa.size(); ++i) {
-            CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
-            FX_FLOAT fCenterY = (rcAnnot.top + rcAnnot.bottom) / 2.0f;
-            if (fCenterY > rcLeftTop.bottom && fCenterY < rcLeftTop.top)
-              aSelect.push_back(i);
-          }
-          for (size_t i = 0; i < aSelect.size(); ++i)
-            m_Annots.push_back(sa[aSelect[i]]);
+        CFX_FloatRect rcLeftTop = AddToAnnotsList(sa, nLeftTopIndex);
 
-          for (int i = aSelect.size() - 1; i >= 0; --i)
-            sa.erase(sa.begin() + aSelect[i]);
+        std::vector<size_t> aSelect;
+        for (size_t i = 0; i < sa.size(); ++i) {
+          CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
+          FX_FLOAT fCenterY = (rcAnnot.top + rcAnnot.bottom) / 2.0f;
+          if (fCenterY > rcLeftTop.bottom && fCenterY < rcLeftTop.top)
+            aSelect.push_back(i);
         }
+        AddSelectedToAnnots(sa, aSelect);
       }
       break;
     }
+
     case COLUMN: {
       std::vector<CPDFSDK_Annot*> sa;
-      for (size_t i = 0; i < m_pPageView->CountAnnots(); ++i) {
-        CPDFSDK_Annot* pAnnot = m_pPageView->GetAnnot(i);
-        if (pAnnot->GetAnnotSubtype() == m_nAnnotSubtype &&
-            !pAnnot->IsSignatureWidget())
-          sa.push_back(pAnnot);
-      }
+      CollectAnnots(&sa);
 
       std::sort(sa.begin(), sa.end(), CompareByTopDescending);
       while (!sa.empty()) {
         int nLeftTopIndex = -1;
         FX_FLOAT fLeft = -1.0f;
         for (int i = sa.size() - 1; i >= 0; --i) {
           CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
           if (fLeft < 0) {
             nLeftTopIndex = 0;
             fLeft = rcAnnot.left;
           } else if (rcAnnot.left < fLeft) {
             nLeftTopIndex = i;
             fLeft = rcAnnot.left;
           }
         }
+        if (nLeftTopIndex < 0)
+          continue;
 
-        if (nLeftTopIndex >= 0) {
-          CPDFSDK_Annot* pLeftTopAnnot = sa[nLeftTopIndex];
-          CFX_FloatRect rcLeftTop = GetAnnotRect(pLeftTopAnnot);
-          m_Annots.push_back(pLeftTopAnnot);
-          sa.erase(sa.begin() + nLeftTopIndex);
+        CFX_FloatRect rcLeftTop = AddToAnnotsList(sa, nLeftTopIndex);
 
-          std::vector<int> aSelect;
-          for (size_t i = 0; i < sa.size(); ++i) {
-            CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
-            FX_FLOAT fCenterX = (rcAnnot.left + rcAnnot.right) / 2.0f;
-            if (fCenterX > rcLeftTop.left && fCenterX < rcLeftTop.right)
-              aSelect.push_back(i);
-          }
-          for (size_t i = 0; i < aSelect.size(); ++i)
-            m_Annots.push_back(sa[aSelect[i]]);
-
-          for (int i = aSelect.size() - 1; i >= 0; --i)
-            sa.erase(sa.begin() + aSelect[i]);
+        std::vector<size_t> aSelect;
+        for (size_t i = 0; i < sa.size(); ++i) {
+          CFX_FloatRect rcAnnot = GetAnnotRect(sa[i]);
+          FX_FLOAT fCenterX = (rcAnnot.left + rcAnnot.right) / 2.0f;
+          if (fCenterX > rcLeftTop.left && fCenterX < rcLeftTop.right)
+            aSelect.push_back(i);
         }
+        AddSelectedToAnnots(sa, aSelect);
       }
       break;
     }
   }
 }
 
 CFX_FloatRect CBA_AnnotIterator::GetAnnotRect(const CPDFSDK_Annot* pAnnot) {
   return pAnnot->GetPDFAnnot()->GetRect();
 }