Clarify page rectangles in DocumentLayout
This change splits DocumentLayout::page_rect() into two methods,
page_rect() and page_bounds_rect(), to better mirror the corresponding
APIs in PDFEngine, GetPageRect() (page rectangle before insets) and
GetPageBoundsRect() (page rectangle after insets).
The existing page_rect() method is equivalent to PDFEngine's
GetPageBoundsRect(), which is confusing, so it has been renamed to
page_bounds_rect(). The new page_rect() method is equivalent to
PDFEngine's GetPageRect(). (This will be useful in a future change.)
DocumentLayout is now responsible for adding insets to the page
rectangles returned by draw_utils::GetRectForSingleView(), etc.,
eliminating the need to pass the insets to these functions.
Bug: 885110
Change-Id: I4ef345333abbb5b75892c32ffff8dee24e20ef05
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762601
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: K Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689735}
diff --git a/pdf/document_layout.cc b/pdf/document_layout.cc
index b1d3172..c1ef704 100644
--- a/pdf/document_layout.cc
+++ b/pdf/document_layout.cc
@@ -19,6 +19,13 @@
return widest_page_width;
}
+pp::Rect InsetRect(pp::Rect rect,
+ const draw_utils::PageInsetSizes& inset_sizes) {
+ rect.Inset(inset_sizes.left, inset_sizes.top, inset_sizes.right,
+ inset_sizes.bottom);
+ return rect;
+}
+
} // namespace
const draw_utils::PageInsetSizes DocumentLayout::kSingleViewInsets{
@@ -48,7 +55,7 @@
const std::vector<pp::Size>& page_sizes) {
set_size({GetWidestPageWidth(page_sizes), 0});
- page_rects_.resize(page_sizes.size());
+ page_layouts_.resize(page_sizes.size());
for (size_t i = 0; i < page_sizes.size(); ++i) {
if (i != 0) {
// Add space for bottom separator.
@@ -56,8 +63,10 @@
}
const pp::Size& page_size = page_sizes[i];
- page_rects_[i] =
- draw_utils::GetRectForSingleView(page_size, size_, kSingleViewInsets);
+ pp::Rect page_rect = draw_utils::GetRectForSingleView(page_size, size_);
+ page_layouts_[i].outer_rect = page_rect;
+ page_layouts_[i].inner_rect = InsetRect(page_rect, kSingleViewInsets);
+
draw_utils::ExpandDocumentSize(page_size, &size_);
}
}
@@ -66,21 +75,24 @@
const std::vector<pp::Size>& page_sizes) {
set_size({GetWidestPageWidth(page_sizes), 0});
- page_rects_.resize(page_sizes.size());
+ page_layouts_.resize(page_sizes.size());
for (size_t i = 0; i < page_sizes.size(); ++i) {
draw_utils::PageInsetSizes page_insets =
draw_utils::GetPageInsetsForTwoUpView(
i, page_sizes.size(), kSingleViewInsets, kHorizontalSeparator);
const pp::Size& page_size = page_sizes[i];
+ pp::Rect page_rect;
if (i % 2 == 0) {
- page_rects_[i] = draw_utils::GetLeftRectForTwoUpView(
- page_size, {size_.width(), size_.height()}, page_insets);
+ page_rect = draw_utils::GetLeftRectForTwoUpView(
+ page_size, {size_.width(), size_.height()});
} else {
- page_rects_[i] = draw_utils::GetRightRectForTwoUpView(
- page_size, {size_.width(), size_.height()}, page_insets);
+ page_rect = draw_utils::GetRightRectForTwoUpView(
+ page_size, {size_.width(), size_.height()});
EnlargeHeight(std::max(page_size.height(), page_sizes[i - 1].height()));
}
+ page_layouts_[i].outer_rect = page_rect;
+ page_layouts_[i].inner_rect = InsetRect(page_rect, page_insets);
}
if (page_sizes.size() % 2 == 1) {
diff --git a/pdf/document_layout.h b/pdf/document_layout.h
index 2e95c59..5b395d6 100644
--- a/pdf/document_layout.h
+++ b/pdf/document_layout.h
@@ -74,12 +74,19 @@
// TODO(kmoon): Get rid of this method.
void set_size(const pp::Size& size) { size_ = size; }
- size_t page_count() const { return page_rects_.size(); }
+ size_t page_count() const { return page_layouts_.size(); }
// Gets the layout rectangle for a page. Only valid after computing a layout.
const pp::Rect& page_rect(size_t page_index) const {
DCHECK_LT(page_index, page_count());
- return page_rects_[page_index];
+ return page_layouts_[page_index].outer_rect;
+ }
+
+ // Gets the layout rectangle for a page's bounds (which excludes additional
+ // regions like page shadows). Only valid after computing a layout.
+ const pp::Rect& page_bounds_rect(size_t page_index) const {
+ DCHECK_LT(page_index, page_count());
+ return page_layouts_[page_index].inner_rect;
}
// Computes layout that represent |page_sizes| formatted for single view.
@@ -98,13 +105,21 @@
void EnlargeHeight(int height);
private:
+ // Layout of a single page.
+ struct PageLayout {
+ // Bounding rectangle for the page with decorations.
+ pp::Rect outer_rect;
+
+ // Bounding rectangle for the page without decorations.
+ pp::Rect inner_rect;
+ };
+
Options options_;
// Layout's total size.
pp::Size size_;
- // Page layout rectangles.
- std::vector<pp::Rect> page_rects_;
+ std::vector<PageLayout> page_layouts_;
};
} // namespace chrome_pdf
diff --git a/pdf/document_layout_unittest.cc b/pdf/document_layout_unittest.cc
index f38a6a6..1e4a5a4 100644
--- a/pdf/document_layout_unittest.cc
+++ b/pdf/document_layout_unittest.cc
@@ -123,20 +123,38 @@
{300, 400}, {400, 500}, {300, 400}, {200, 300}};
layout_.ComputeSingleViewLayout(page_sizes);
ASSERT_EQ(4u, layout_.page_count());
- EXPECT_PRED2(PpRectEq, pp::Rect(55, 3, 290, 390), layout_.page_rect(0));
- EXPECT_PRED2(PpRectEq, pp::Rect(5, 407, 390, 490), layout_.page_rect(1));
- EXPECT_PRED2(PpRectEq, pp::Rect(55, 911, 290, 390), layout_.page_rect(2));
- EXPECT_PRED2(PpRectEq, pp::Rect(105, 1315, 190, 290), layout_.page_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(50, 0, 300, 400), layout_.page_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(0, 404, 400, 500), layout_.page_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(50, 908, 300, 400), layout_.page_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(100, 1312, 200, 300), layout_.page_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(55, 3, 290, 390),
+ layout_.page_bounds_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(5, 407, 390, 490),
+ layout_.page_bounds_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(55, 911, 290, 390),
+ layout_.page_bounds_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(105, 1315, 190, 290),
+ layout_.page_bounds_rect(3));
EXPECT_PRED2(PpSizeEq, pp::Size(400, 1612), layout_.size());
page_sizes = {{240, 300}, {320, 400}, {250, 360}, {300, 600}, {270, 555}};
layout_.ComputeSingleViewLayout(page_sizes);
ASSERT_EQ(5u, layout_.page_count());
- EXPECT_PRED2(PpRectEq, pp::Rect(45, 3, 230, 290), layout_.page_rect(0));
- EXPECT_PRED2(PpRectEq, pp::Rect(5, 307, 310, 390), layout_.page_rect(1));
- EXPECT_PRED2(PpRectEq, pp::Rect(40, 711, 240, 350), layout_.page_rect(2));
- EXPECT_PRED2(PpRectEq, pp::Rect(15, 1075, 290, 590), layout_.page_rect(3));
- EXPECT_PRED2(PpRectEq, pp::Rect(30, 1679, 260, 545), layout_.page_rect(4));
+ EXPECT_PRED2(PpRectEq, pp::Rect(40, 0, 240, 300), layout_.page_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(0, 304, 320, 400), layout_.page_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(35, 708, 250, 360), layout_.page_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(10, 1072, 300, 600), layout_.page_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(25, 1676, 270, 555), layout_.page_rect(4));
+ EXPECT_PRED2(PpRectEq, pp::Rect(45, 3, 230, 290),
+ layout_.page_bounds_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(5, 307, 310, 390),
+ layout_.page_bounds_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(40, 711, 240, 350),
+ layout_.page_bounds_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(15, 1075, 290, 590),
+ layout_.page_bounds_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(30, 1679, 260, 545),
+ layout_.page_bounds_rect(4));
EXPECT_PRED2(PpSizeEq, pp::Size(320, 2231), layout_.size());
}
@@ -146,31 +164,57 @@
{826, 1066}, {1066, 826}, {826, 1066}, {826, 900}};
layout_.ComputeTwoUpViewLayout(page_sizes);
ASSERT_EQ(4u, layout_.page_count());
- EXPECT_PRED2(PpRectEq, pp::Rect(245, 3, 820, 1056), layout_.page_rect(0));
- EXPECT_PRED2(PpRectEq, pp::Rect(1067, 3, 1060, 816), layout_.page_rect(1));
- EXPECT_PRED2(PpRectEq, pp::Rect(245, 1069, 820, 1056), layout_.page_rect(2));
- EXPECT_PRED2(PpRectEq, pp::Rect(1067, 1069, 820, 890), layout_.page_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(240, 0, 826, 1066), layout_.page_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(1066, 0, 1066, 826), layout_.page_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(240, 1066, 826, 1066), layout_.page_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(1066, 1066, 826, 900), layout_.page_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(245, 3, 820, 1056),
+ layout_.page_bounds_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(1067, 3, 1060, 816),
+ layout_.page_bounds_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(245, 1069, 820, 1056),
+ layout_.page_bounds_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(1067, 1069, 820, 890),
+ layout_.page_bounds_rect(3));
EXPECT_PRED2(PpSizeEq, pp::Size(2132, 2132), layout_.size());
// Test case where the widest page is on the left.
page_sizes = {{1066, 826}, {820, 1056}, {820, 890}, {826, 1066}};
layout_.ComputeTwoUpViewLayout(page_sizes);
ASSERT_EQ(4u, layout_.page_count());
- EXPECT_PRED2(PpRectEq, pp::Rect(5, 3, 1060, 816), layout_.page_rect(0));
- EXPECT_PRED2(PpRectEq, pp::Rect(1067, 3, 814, 1046), layout_.page_rect(1));
- EXPECT_PRED2(PpRectEq, pp::Rect(251, 1059, 814, 880), layout_.page_rect(2));
- EXPECT_PRED2(PpRectEq, pp::Rect(1067, 1059, 820, 1056), layout_.page_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(0, 0, 1066, 826), layout_.page_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(1066, 0, 820, 1056), layout_.page_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(246, 1056, 820, 890), layout_.page_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(1066, 1056, 826, 1066), layout_.page_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(5, 3, 1060, 816),
+ layout_.page_bounds_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(1067, 3, 814, 1046),
+ layout_.page_bounds_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(251, 1059, 814, 880),
+ layout_.page_bounds_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(1067, 1059, 820, 1056),
+ layout_.page_bounds_rect(3));
EXPECT_PRED2(PpSizeEq, pp::Size(2132, 2122), layout_.size());
// Test case where there's an odd # of pages.
page_sizes = {{200, 300}, {400, 200}, {300, 600}, {250, 500}, {300, 400}};
layout_.ComputeTwoUpViewLayout(page_sizes);
ASSERT_EQ(5u, layout_.page_count());
- EXPECT_PRED2(PpRectEq, pp::Rect(205, 3, 194, 290), layout_.page_rect(0));
- EXPECT_PRED2(PpRectEq, pp::Rect(401, 3, 394, 190), layout_.page_rect(1));
- EXPECT_PRED2(PpRectEq, pp::Rect(105, 303, 294, 590), layout_.page_rect(2));
- EXPECT_PRED2(PpRectEq, pp::Rect(401, 303, 244, 490), layout_.page_rect(3));
- EXPECT_PRED2(PpRectEq, pp::Rect(105, 903, 290, 390), layout_.page_rect(4));
+ EXPECT_PRED2(PpRectEq, pp::Rect(200, 0, 200, 300), layout_.page_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(400, 0, 400, 200), layout_.page_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(100, 300, 300, 600), layout_.page_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(400, 300, 250, 500), layout_.page_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(100, 900, 300, 400), layout_.page_rect(4));
+ EXPECT_PRED2(PpRectEq, pp::Rect(205, 3, 194, 290),
+ layout_.page_bounds_rect(0));
+ EXPECT_PRED2(PpRectEq, pp::Rect(401, 3, 394, 190),
+ layout_.page_bounds_rect(1));
+ EXPECT_PRED2(PpRectEq, pp::Rect(105, 303, 294, 590),
+ layout_.page_bounds_rect(2));
+ EXPECT_PRED2(PpRectEq, pp::Rect(401, 303, 244, 490),
+ layout_.page_bounds_rect(3));
+ EXPECT_PRED2(PpRectEq, pp::Rect(105, 903, 290, 390),
+ layout_.page_bounds_rect(4));
EXPECT_PRED2(PpSizeEq, pp::Size(800, 1300), layout_.size());
}
diff --git a/pdf/draw_utils/coordinates.cc b/pdf/draw_utils/coordinates.cc
index 0c199c83..49c272f2 100644
--- a/pdf/draw_utils/coordinates.cc
+++ b/pdf/draw_utils/coordinates.cc
@@ -81,13 +81,9 @@
}
pp::Rect GetRectForSingleView(const pp::Size& rect_size,
- const pp::Size& document_size,
- const PageInsetSizes& page_insets) {
+ const pp::Size& document_size) {
pp::Rect page_rect({0, document_size.height()}, rect_size);
CenterRectHorizontally(document_size.width(), &page_rect);
- page_rect.Inset(page_insets.left, page_insets.top, page_insets.right,
- page_insets.bottom);
-
return page_rect;
}
@@ -147,25 +143,17 @@
}
pp::Rect GetLeftRectForTwoUpView(const pp::Size& rect_size,
- const pp::Point& position,
- const PageInsetSizes& page_insets) {
+ const pp::Point& position) {
DCHECK_LE(rect_size.width(), position.x());
- pp::Rect left_rect(position.x() - rect_size.width(), position.y(),
- rect_size.width(), rect_size.height());
- left_rect.Inset(page_insets.left, page_insets.top, page_insets.right,
- page_insets.bottom);
- return left_rect;
+ return pp::Rect(position.x() - rect_size.width(), position.y(),
+ rect_size.width(), rect_size.height());
}
pp::Rect GetRightRectForTwoUpView(const pp::Size& rect_size,
- const pp::Point& position,
- const PageInsetSizes& page_insets) {
- pp::Rect right_rect(position.x(), position.y(), rect_size.width(),
- rect_size.height());
- right_rect.Inset(page_insets.left, page_insets.top, page_insets.right,
- page_insets.bottom);
- return right_rect;
+ const pp::Point& position) {
+ return pp::Rect(position.x(), position.y(), rect_size.width(),
+ rect_size.height());
}
} // namespace draw_utils
diff --git a/pdf/draw_utils/coordinates.h b/pdf/draw_utils/coordinates.h
index 952122c9..ed83464 100644
--- a/pdf/draw_utils/coordinates.h
+++ b/pdf/draw_utils/coordinates.h
@@ -75,11 +75,9 @@
int horizontal_separator);
// Given |rect_size| and |document_size| create a horizontally centered
-// pp::Rect placed at the bottom of the current document, and then inset it with
-// |page_insets|.
+// pp::Rect placed at the bottom of the current document.
pp::Rect GetRectForSingleView(const pp::Size& rect_size,
- const pp::Size& document_size,
- const PageInsetSizes& page_insets);
+ const pp::Size& document_size);
// Given |rect| in document coordinates, a |position| in screen coordinates,
// and a |zoom| factor, returns the rectangle in screen coordinates (i.e.
@@ -125,20 +123,15 @@
int bottom_separator);
// Given |rect_size|, create a pp::Rect where the top-right corner lies at
-// |position|, and then inset it with the corresponding values of |page_insets|,
-// i.e. inset on left side with |page_insets.left|.
-// The width of |rect_size| must be less than or equal to the x value for
-// |position|.
+// |position|. The width of |rect_size| must be less than or equal to the x
+// value for |position|.
pp::Rect GetLeftRectForTwoUpView(const pp::Size& rect_size,
- const pp::Point& position,
- const PageInsetSizes& page_insets);
+ const pp::Point& position);
// Given |rect_size|, create a pp::Rect where the top-left corner lies at
-// |position|, and then inset it with the corresponding values of |page_insets|,
-// i.e. inset on left side with |page_insets.left|.
+// |position|.
pp::Rect GetRightRectForTwoUpView(const pp::Size& rect_size,
- const pp::Point& position,
- const PageInsetSizes& page_insets);
+ const pp::Point& position);
} // namespace draw_utils
} // namespace chrome_pdf
diff --git a/pdf/draw_utils/coordinates_unittest.cc b/pdf/draw_utils/coordinates_unittest.cc
index c55c14b..8a502afb 100644
--- a/pdf/draw_utils/coordinates_unittest.cc
+++ b/pdf/draw_utils/coordinates_unittest.cc
@@ -151,16 +151,16 @@
TEST(CoordinateTest, GetRectForSingleView) {
// Test portrait pages.
- CompareRect({55, 503, 190, 390},
- GetRectForSingleView({200, 400}, {300, 500}, kSingleViewInsets));
- CompareRect({55, 603, 90, 330},
- GetRectForSingleView({100, 340}, {200, 600}, kSingleViewInsets));
+ CompareRect({50, 500, 200, 400},
+ GetRectForSingleView({200, 400}, {300, 500}));
+ CompareRect({50, 600, 100, 340},
+ GetRectForSingleView({100, 340}, {200, 600}));
// Test landscape pages.
- CompareRect({5, 1003, 490, 440},
- GetRectForSingleView({500, 450}, {500, 1000}, kSingleViewInsets));
- CompareRect({30, 1503, 640, 190},
- GetRectForSingleView({650, 200}, {700, 1500}, kSingleViewInsets));
+ CompareRect({0, 1000, 500, 450},
+ GetRectForSingleView({500, 450}, {500, 1000}));
+ CompareRect({25, 1500, 650, 200},
+ GetRectForSingleView({650, 200}, {700, 1500}));
}
TEST(CoordinateTest, GetScreenRect) {
@@ -275,33 +275,22 @@
}
TEST(CoordinateTest, GetLeftRectForTwoUpView) {
- CompareRect({105, 103, 194, 390},
- GetLeftRectForTwoUpView({200, 400}, {300, 100}, kLeftInsets));
- CompareRect({5, 3, 294, 390},
- GetLeftRectForTwoUpView({300, 400}, {300, 0}, kLeftInsets));
-
- // Test rect smaller than shadow insets returns empty rect.
- CompareRect({10, 3, 0, 0},
- GetLeftRectForTwoUpView({5, 5}, {10, 0}, kLeftInsets));
+ CompareRect({100, 100, 200, 400},
+ GetLeftRectForTwoUpView({200, 400}, {300, 100}));
+ CompareRect({0, 0, 300, 400}, GetLeftRectForTwoUpView({300, 400}, {300, 0}));
// Test empty rect gets positioned.
- CompareRect({105, 3, 0, 0},
- GetLeftRectForTwoUpView({0, 0}, {100, 0}, kLeftInsets));
+ CompareRect({100, 0, 0, 0}, GetLeftRectForTwoUpView({0, 0}, {100, 0}));
}
TEST(CoordinateTest, GetRightRectForTwoUpView) {
- CompareRect({301, 103, 194, 390},
- GetRightRectForTwoUpView({200, 400}, {300, 100}, kRightInsets));
- CompareRect({301, 3, 294, 390},
- GetRightRectForTwoUpView({300, 400}, {300, 0}, kRightInsets));
-
- // Test rect smaller than shadow insets returns empty rect.
- CompareRect({11, 3, 0, 0},
- GetRightRectForTwoUpView({5, 5}, {10, 0}, kRightInsets));
+ CompareRect({300, 100, 200, 400},
+ GetRightRectForTwoUpView({200, 400}, {300, 100}));
+ CompareRect({300, 0, 300, 400},
+ GetRightRectForTwoUpView({300, 400}, {300, 0}));
// Test empty rect gets positioned.
- CompareRect({101, 3, 0, 0},
- GetRightRectForTwoUpView({0, 0}, {100, 0}, kRightInsets));
+ CompareRect({100, 0, 0, 0}, GetRightRectForTwoUpView({0, 0}, {100, 0}));
}
} // namespace draw_utils
diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc
index 4098594..23b03be 100644
--- a/pdf/pdfium/pdfium_engine.cc
+++ b/pdf/pdfium/pdfium_engine.cc
@@ -2403,7 +2403,7 @@
// TODO(kmoon): This should be the only method that sets |PDFiumPage::rect_|.
void PDFiumEngine::ApplyCurrentLayoutToPages(bool reload) {
for (size_t i = 0; i < layout_.page_count(); ++i) {
- const pp::Rect& page_rect = layout_.page_rect(i);
+ const pp::Rect& page_rect = layout_.page_bounds_rect(i);
if (!reload) {
// The page is marked as not being available even if |doc_complete| is
// true because FPDFAvail_IsPageAvail() still has to be called for this