Commit 92d076ed7a6ec209fdfc3b92a86a510622d1330f
1 parent
e9e2eef5
Refactor `Pages::all`: inline common case, rename existing implementation to `ca…
…che`, update calls, streamline initialization, and enhance assertion usage.
Showing
2 changed files
with
14 additions
and
10 deletions
libqpdf/QPDF_pages.cc
| @@ -4,6 +4,7 @@ | @@ -4,6 +4,7 @@ | ||
| 4 | #include <qpdf/QPDFObjectHandle_private.hh> | 4 | #include <qpdf/QPDFObjectHandle_private.hh> |
| 5 | #include <qpdf/QTC.hh> | 5 | #include <qpdf/QTC.hh> |
| 6 | #include <qpdf/QUtil.hh> | 6 | #include <qpdf/QUtil.hh> |
| 7 | +#include <qpdf/Util.hh> | ||
| 7 | 8 | ||
| 8 | // In support of page manipulation APIs, these methods internally maintain state about pages in a | 9 | // In support of page manipulation APIs, these methods internally maintain state about pages in a |
| 9 | // pair of data structures: all_pages, which is a vector of page objects, and pageobj_to_pages_pos, | 10 | // pair of data structures: all_pages, which is a vector of page objects, and pageobj_to_pages_pos, |
| @@ -46,7 +47,7 @@ QPDF::getAllPages() | @@ -46,7 +47,7 @@ QPDF::getAllPages() | ||
| 46 | } | 47 | } |
| 47 | 48 | ||
| 48 | std::vector<QPDFObjectHandle> const& | 49 | std::vector<QPDFObjectHandle> const& |
| 49 | -Pages::all() | 50 | +Pages::cache() |
| 50 | { | 51 | { |
| 51 | // Note that pushInheritedAttributesToPage may also be used to initialize m->all_pages. | 52 | // Note that pushInheritedAttributesToPage may also be used to initialize m->all_pages. |
| 52 | if (all_pages.empty() && !invalid_page_found) { | 53 | if (all_pages.empty() && !invalid_page_found) { |
| @@ -253,7 +254,7 @@ Pages::update_cache() | @@ -253,7 +254,7 @@ Pages::update_cache() | ||
| 253 | all_pages.clear(); | 254 | all_pages.clear(); |
| 254 | pageobj_to_pages_pos.clear(); | 255 | pageobj_to_pages_pos.clear(); |
| 255 | pushed_inherited_attributes_to_pages = false; | 256 | pushed_inherited_attributes_to_pages = false; |
| 256 | - all(); | 257 | + cache(); |
| 257 | } | 258 | } |
| 258 | 259 | ||
| 259 | void | 260 | void |
| @@ -309,9 +310,9 @@ Pages::pushInheritedAttributesToPage(bool allow_changes, bool warn_skipped_keys) | @@ -309,9 +310,9 @@ Pages::pushInheritedAttributesToPage(bool allow_changes, bool warn_skipped_keys) | ||
| 309 | return; | 310 | return; |
| 310 | } | 311 | } |
| 311 | 312 | ||
| 312 | - // Calling getAllPages() resolves any duplicated page objects, repairs broken nodes, and detects | 313 | + // Calling cache() resolves any duplicated page objects, repairs broken nodes, and detects |
| 313 | // loops, so we don't have to do those activities here. | 314 | // loops, so we don't have to do those activities here. |
| 314 | - (void)all(); | 315 | + (void)cache(); |
| 315 | 316 | ||
| 316 | // key_ancestors is a mapping of page attribute keys to a stack of Pages nodes that contain | 317 | // key_ancestors is a mapping of page attribute keys to a stack of Pages nodes that contain |
| 317 | // values for them. | 318 | // values for them. |
| @@ -321,10 +322,9 @@ Pages::pushInheritedAttributesToPage(bool allow_changes, bool warn_skipped_keys) | @@ -321,10 +322,9 @@ Pages::pushInheritedAttributesToPage(bool allow_changes, bool warn_skipped_keys) | ||
| 321 | key_ancestors, | 322 | key_ancestors, |
| 322 | allow_changes, | 323 | allow_changes, |
| 323 | warn_skipped_keys); | 324 | warn_skipped_keys); |
| 324 | - if (!key_ancestors.empty()) { | ||
| 325 | - throw std::logic_error( | ||
| 326 | - "key_ancestors not empty after pushing inherited attributes to pages"); | ||
| 327 | - } | 325 | + util::assertion( |
| 326 | + key_ancestors.empty(), | ||
| 327 | + "key_ancestors not empty after pushing inherited attributes to pages"); | ||
| 328 | pushed_inherited_attributes_to_pages = true; | 328 | pushed_inherited_attributes_to_pages = true; |
| 329 | ever_pushed_inherited_attributes_to_pages_ = true; | 329 | ever_pushed_inherited_attributes_to_pages_ = true; |
| 330 | } | 330 | } |
libqpdf/qpdf/QPDF_private.hh
| @@ -865,7 +865,11 @@ class QPDF::Doc::Pages: Common | @@ -865,7 +865,11 @@ class QPDF::Doc::Pages: Common | ||
| 865 | { | 865 | { |
| 866 | } | 866 | } |
| 867 | 867 | ||
| 868 | - std::vector<QPDFObjectHandle> const& all(); | 868 | + std::vector<QPDFObjectHandle> const& |
| 869 | + all() | ||
| 870 | + { | ||
| 871 | + return !all_pages.empty() ? all_pages : cache(); | ||
| 872 | + } | ||
| 869 | 873 | ||
| 870 | bool | 874 | bool |
| 871 | empty() | 875 | empty() |
| @@ -906,7 +910,7 @@ class QPDF::Doc::Pages: Common | @@ -906,7 +910,7 @@ class QPDF::Doc::Pages: Common | ||
| 906 | std::map<std::string, std::vector<QPDFObjectHandle>>&, | 910 | std::map<std::string, std::vector<QPDFObjectHandle>>&, |
| 907 | bool allow_changes, | 911 | bool allow_changes, |
| 908 | bool warn_skipped_keys); | 912 | bool warn_skipped_keys); |
| 909 | - | 913 | + std::vector<QPDFObjectHandle> const& cache(); |
| 910 | void getAllPagesInternal( | 914 | void getAllPagesInternal( |
| 911 | QPDFObjectHandle cur_pages, | 915 | QPDFObjectHandle cur_pages, |
| 912 | QPDFObjGen::set& visited, | 916 | QPDFObjGen::set& visited, |