chromium/src
0
Fork 0
Code Activity

Migrate pp::Rect to gfx::Rect in DocumentLayout

Browse Source 
Update DocumentLayout to use gfx::Rect instead of pp::Rect. Update
callers to expect gfx::Rect as return value instead of pp::Rect.

Bug: 1101101
Change-Id: I6835b99e1355b14b1d27511ba9e8a8bb3910d46a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2371083
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: K. Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802000}
This commit is contained in:
Ankit Kumar 🌪️
2020-08-26 22:57:14 +00:00
committed by Commit Bot
parent feae489c72
commit 0079869d46
5 changed files with 75 additions and 104 deletions

43
pdf/document_layout.cc

@ -8,8 +8,6 @@
#include "base/check_op.h" #include "base/check_op.h"
#include "base/values.h" #include "base/values.h"
#include "pdf/ppapi_migration/geometry_conversions.h"
#include "ppapi/cpp/rect.h"
#include "ppapi/cpp/var.h" #include "ppapi/cpp/var.h"
#include "ppapi/cpp/var_dictionary.h" #include "ppapi/cpp/var_dictionary.h"
#include "ui/gfx/geometry/point.h" #include "ui/gfx/geometry/point.h"
@ -32,11 +30,12 @@ int GetWidestPageWidth(const std::vector<gfx::Size>& page_sizes) {
return widest_page_width; return widest_page_width;
} }
pp::Rect InsetRect(pp::Rect rect, gfx::Rect InsetRect(const gfx::Rect& rect,
const draw_utils::PageInsetSizes& inset_sizes) { const draw_utils::PageInsetSizes& inset_sizes) {
rect.Inset(inset_sizes.left, inset_sizes.top, inset_sizes.right, gfx::Rect inset_rect(rect);
inset_sizes.bottom); inset_rect.Inset(inset_sizes.left, inset_sizes.top, inset_sizes.right,
return rect; inset_sizes.bottom);
return inset_rect;
} }
} // namespace } // namespace
@ -118,11 +117,11 @@ void DocumentLayout::ComputeSingleViewLayout(
} }
const gfx::Size& page_size = page_sizes[i]; const gfx::Size& page_size = page_sizes[i];
pp::Rect page_rect = PPRectFromRect( gfx::Rect page_rect =
draw_utils::GetRectForSingleView(page_size, document_size)); draw_utils::GetRectForSingleView(page_size, document_size);
CopyRectIfModified(page_rect, &page_layouts_[i].outer_rect); CopyRectIfModified(page_rect, page_layouts_[i].outer_rect);
CopyRectIfModified(InsetRect(page_rect, kSingleViewInsets), CopyRectIfModified(InsetRect(page_rect, kSingleViewInsets),
&page_layouts_[i].inner_rect); page_layouts_[i].inner_rect);
draw_utils::ExpandDocumentSize(page_size, &document_size); draw_utils::ExpandDocumentSize(page_size, &document_size);
} }
@ -149,19 +148,19 @@ void DocumentLayout::ComputeTwoUpViewLayout(
i, page_sizes.size(), kSingleViewInsets, kHorizontalSeparator); i, page_sizes.size(), kSingleViewInsets, kHorizontalSeparator);
const gfx::Size& page_size = page_sizes[i]; const gfx::Size& page_size = page_sizes[i];
pp::Rect page_rect; gfx::Rect page_rect;
if (i % 2 == 0) { if (i % 2 == 0) {
page_rect = PPRectFromRect(draw_utils::GetLeftRectForTwoUpView( page_rect = draw_utils::GetLeftRectForTwoUpView(
page_size, {document_size.width(), document_size.height()})); page_size, {document_size.width(), document_size.height()});
} else { } else {
page_rect = PPRectFromRect(draw_utils::GetRightRectForTwoUpView( page_rect = draw_utils::GetRightRectForTwoUpView(
page_size, {document_size.width(), document_size.height()})); page_size, {document_size.width(), document_size.height()});
document_size.Enlarge( document_size.Enlarge(
0, std::max(page_size.height(), page_sizes[i - 1].height())); 0, std::max(page_size.height(), page_sizes[i - 1].height()));
} }
CopyRectIfModified(page_rect, &page_layouts_[i].outer_rect); CopyRectIfModified(page_rect, page_layouts_[i].outer_rect);
CopyRectIfModified(InsetRect(page_rect, page_insets), CopyRectIfModified(InsetRect(page_rect, page_insets),
&page_layouts_[i].inner_rect); page_layouts_[i].inner_rect);
} }
if (page_sizes.size() % 2 == 1) { if (page_sizes.size() % 2 == 1) {
@ -176,10 +175,10 @@ void DocumentLayout::ComputeTwoUpViewLayout(
} }
} }
void DocumentLayout::CopyRectIfModified(const pp::Rect& source_rect, void DocumentLayout::CopyRectIfModified(const gfx::Rect& source_rect,
pp::Rect* destination_rect) { gfx::Rect& destination_rect) {
if (*destination_rect != source_rect) { if (destination_rect != source_rect) {
*destination_rect = source_rect; destination_rect = source_rect;
dirty_ = true; dirty_ = true;
} }
} }

14
pdf/document_layout.h

@ -11,7 +11,7 @@
#include "base/check_op.h" #include "base/check_op.h"
#include "pdf/draw_utils/coordinates.h" #include "pdf/draw_utils/coordinates.h"
#include "pdf/page_orientation.h" #include "pdf/page_orientation.h"
#include "ppapi/cpp/rect.h" #include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
namespace base { namespace base {
@ -113,14 +113,14 @@ class DocumentLayout final {
size_t page_count() const { return page_layouts_.size(); } size_t page_count() const { return page_layouts_.size(); }
// Gets the layout rectangle for a page. Only valid after computing a layout. // Gets the layout rectangle for a page. Only valid after computing a layout.
const pp::Rect& page_rect(size_t page_index) const { const gfx::Rect& page_rect(size_t page_index) const {
DCHECK_LT(page_index, page_count()); DCHECK_LT(page_index, page_count());
return page_layouts_[page_index].outer_rect; return page_layouts_[page_index].outer_rect;
} }
// Gets the layout rectangle for a page's bounds (which excludes additional // Gets the layout rectangle for a page's bounds (which excludes additional
// regions like page shadows). Only valid after computing a layout. // regions like page shadows). Only valid after computing a layout.
const pp::Rect& page_bounds_rect(size_t page_index) const { const gfx::Rect& page_bounds_rect(size_t page_index) const {
DCHECK_LT(page_index, page_count()); DCHECK_LT(page_index, page_count());
return page_layouts_[page_index].inner_rect; return page_layouts_[page_index].inner_rect;
} }
@ -139,16 +139,16 @@ class DocumentLayout final {
// Layout of a single page. // Layout of a single page.
struct PageLayout { struct PageLayout {
// Bounding rectangle for the page with decorations. // Bounding rectangle for the page with decorations.
pp::Rect outer_rect; gfx::Rect outer_rect;
// Bounding rectangle for the page without decorations. // Bounding rectangle for the page without decorations.
pp::Rect inner_rect; gfx::Rect inner_rect;
}; };
// Copies |source_rect| to |destination_rect|, setting |dirty_| to true if // Copies |source_rect| to |destination_rect|, setting |dirty_| to true if
// |destination_rect| is modified as a result. // |destination_rect| is modified as a result.
void CopyRectIfModified(const pp::Rect& source_rect, void CopyRectIfModified(const gfx::Rect& source_rect,
pp::Rect* destination_rect); gfx::Rect& destination_rect);
Options options_; Options options_;

118
pdf/document_layout_unittest.cc

@ -5,6 +5,7 @@
#include "pdf/document_layout.h" #include "pdf/document_layout.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
namespace chrome_pdf { namespace chrome_pdf {
@ -21,13 +22,6 @@ class DocumentLayoutTest : public testing::Test {
DocumentLayout layout_; DocumentLayout layout_;
}; };
// TODO(kmoon): Need to use this with EXPECT_PRED2 instead of just using
// EXPECT_EQ, due to ADL issues with pp::Rect's operator== (defined in global
// namespace, instead of in "pp").
inline bool PpRectEq(const pp::Rect& lhs, const pp::Rect& rhs) {
return lhs == rhs;
}
TEST_F(DocumentLayoutOptionsTest, DefaultConstructor) { TEST_F(DocumentLayoutOptionsTest, DefaultConstructor) {
EXPECT_EQ(options_.default_page_orientation(), PageOrientation::kOriginal); EXPECT_EQ(options_.default_page_orientation(), PageOrientation::kOriginal);
EXPECT_FALSE(options_.two_up_view_enabled()); EXPECT_FALSE(options_.two_up_view_enabled());
@ -194,38 +188,29 @@ TEST_F(DocumentLayoutTest, ComputeSingleViewLayout) {
{300, 400}, {400, 500}, {300, 400}, {200, 300}}; {300, 400}, {400, 500}, {300, 400}, {200, 300}};
layout_.ComputeSingleViewLayout(page_sizes); layout_.ComputeSingleViewLayout(page_sizes);
ASSERT_EQ(4u, layout_.page_count()); ASSERT_EQ(4u, layout_.page_count());
EXPECT_PRED2(PpRectEq, pp::Rect(50, 0, 300, 400), layout_.page_rect(0)); EXPECT_EQ(gfx::Rect(50, 0, 300, 400), layout_.page_rect(0));
EXPECT_PRED2(PpRectEq, pp::Rect(0, 404, 400, 500), layout_.page_rect(1)); EXPECT_EQ(gfx::Rect(0, 404, 400, 500), layout_.page_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(50, 908, 300, 400), layout_.page_rect(2)); EXPECT_EQ(gfx::Rect(50, 908, 300, 400), layout_.page_rect(2));
EXPECT_PRED2(PpRectEq, pp::Rect(100, 1312, 200, 300), layout_.page_rect(3)); EXPECT_EQ(gfx::Rect(100, 1312, 200, 300), layout_.page_rect(3));
EXPECT_PRED2(PpRectEq, pp::Rect(55, 3, 290, 390), EXPECT_EQ(gfx::Rect(55, 3, 290, 390), layout_.page_bounds_rect(0));
layout_.page_bounds_rect(0)); EXPECT_EQ(gfx::Rect(5, 407, 390, 490), layout_.page_bounds_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(5, 407, 390, 490), EXPECT_EQ(gfx::Rect(55, 911, 290, 390), layout_.page_bounds_rect(2));
layout_.page_bounds_rect(1)); EXPECT_EQ(gfx::Rect(105, 1315, 190, 290), layout_.page_bounds_rect(3));
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_EQ(gfx::Size(400, 1612), layout_.size()); EXPECT_EQ(gfx::Size(400, 1612), layout_.size());
page_sizes = {{240, 300}, {320, 400}, {250, 360}, {300, 600}, {270, 555}}; page_sizes = {{240, 300}, {320, 400}, {250, 360}, {300, 600}, {270, 555}};
layout_.ComputeSingleViewLayout(page_sizes); layout_.ComputeSingleViewLayout(page_sizes);
ASSERT_EQ(5u, layout_.page_count()); ASSERT_EQ(5u, layout_.page_count());
EXPECT_PRED2(PpRectEq, pp::Rect(40, 0, 240, 300), layout_.page_rect(0)); EXPECT_EQ(gfx::Rect(40, 0, 240, 300), layout_.page_rect(0));
EXPECT_PRED2(PpRectEq, pp::Rect(0, 304, 320, 400), layout_.page_rect(1)); EXPECT_EQ(gfx::Rect(0, 304, 320, 400), layout_.page_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(35, 708, 250, 360), layout_.page_rect(2)); EXPECT_EQ(gfx::Rect(35, 708, 250, 360), layout_.page_rect(2));
EXPECT_PRED2(PpRectEq, pp::Rect(10, 1072, 300, 600), layout_.page_rect(3)); EXPECT_EQ(gfx::Rect(10, 1072, 300, 600), layout_.page_rect(3));
EXPECT_PRED2(PpRectEq, pp::Rect(25, 1676, 270, 555), layout_.page_rect(4)); EXPECT_EQ(gfx::Rect(25, 1676, 270, 555), layout_.page_rect(4));
EXPECT_PRED2(PpRectEq, pp::Rect(45, 3, 230, 290), EXPECT_EQ(gfx::Rect(45, 3, 230, 290), layout_.page_bounds_rect(0));
layout_.page_bounds_rect(0)); EXPECT_EQ(gfx::Rect(5, 307, 310, 390), layout_.page_bounds_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(5, 307, 310, 390), EXPECT_EQ(gfx::Rect(40, 711, 240, 350), layout_.page_bounds_rect(2));
layout_.page_bounds_rect(1)); EXPECT_EQ(gfx::Rect(15, 1075, 290, 590), layout_.page_bounds_rect(3));
EXPECT_PRED2(PpRectEq, pp::Rect(40, 711, 240, 350), EXPECT_EQ(gfx::Rect(30, 1679, 260, 545), layout_.page_bounds_rect(4));
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_EQ(gfx::Size(320, 2231), layout_.size()); EXPECT_EQ(gfx::Size(320, 2231), layout_.size());
} }
@ -235,57 +220,44 @@ TEST_F(DocumentLayoutTest, ComputeTwoUpViewLayout) {
{826, 1066}, {1066, 826}, {826, 1066}, {826, 900}}; {826, 1066}, {1066, 826}, {826, 1066}, {826, 900}};
layout_.ComputeTwoUpViewLayout(page_sizes); layout_.ComputeTwoUpViewLayout(page_sizes);
ASSERT_EQ(4u, layout_.page_count()); ASSERT_EQ(4u, layout_.page_count());
EXPECT_PRED2(PpRectEq, pp::Rect(240, 0, 826, 1066), layout_.page_rect(0)); EXPECT_EQ(gfx::Rect(240, 0, 826, 1066), layout_.page_rect(0));
EXPECT_PRED2(PpRectEq, pp::Rect(1066, 0, 1066, 826), layout_.page_rect(1)); EXPECT_EQ(gfx::Rect(1066, 0, 1066, 826), layout_.page_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(240, 1066, 826, 1066), layout_.page_rect(2)); EXPECT_EQ(gfx::Rect(240, 1066, 826, 1066), layout_.page_rect(2));
EXPECT_PRED2(PpRectEq, pp::Rect(1066, 1066, 826, 900), layout_.page_rect(3)); EXPECT_EQ(gfx::Rect(1066, 1066, 826, 900), layout_.page_rect(3));
EXPECT_PRED2(PpRectEq, pp::Rect(245, 3, 820, 1056), EXPECT_EQ(gfx::Rect(245, 3, 820, 1056), layout_.page_bounds_rect(0));
layout_.page_bounds_rect(0)); EXPECT_EQ(gfx::Rect(1067, 3, 1060, 816), layout_.page_bounds_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(1067, 3, 1060, 816), EXPECT_EQ(gfx::Rect(245, 1069, 820, 1056), layout_.page_bounds_rect(2));
layout_.page_bounds_rect(1)); EXPECT_EQ(gfx::Rect(1067, 1069, 820, 890), layout_.page_bounds_rect(3));
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_EQ(gfx::Size(2132, 2132), layout_.size()); EXPECT_EQ(gfx::Size(2132, 2132), layout_.size());
// Test case where the widest page is on the left. // Test case where the widest page is on the left.
page_sizes = {{1066, 826}, {820, 1056}, {820, 890}, {826, 1066}}; page_sizes = {{1066, 826}, {820, 1056}, {820, 890}, {826, 1066}};
layout_.ComputeTwoUpViewLayout(page_sizes); layout_.ComputeTwoUpViewLayout(page_sizes);
ASSERT_EQ(4u, layout_.page_count()); ASSERT_EQ(4u, layout_.page_count());
EXPECT_PRED2(PpRectEq, pp::Rect(0, 0, 1066, 826), layout_.page_rect(0)); EXPECT_EQ(gfx::Rect(0, 0, 1066, 826), layout_.page_rect(0));
EXPECT_PRED2(PpRectEq, pp::Rect(1066, 0, 820, 1056), layout_.page_rect(1)); EXPECT_EQ(gfx::Rect(1066, 0, 820, 1056), layout_.page_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(246, 1056, 820, 890), layout_.page_rect(2)); EXPECT_EQ(gfx::Rect(246, 1056, 820, 890), layout_.page_rect(2));
EXPECT_PRED2(PpRectEq, pp::Rect(1066, 1056, 826, 1066), layout_.page_rect(3)); EXPECT_EQ(gfx::Rect(1066, 1056, 826, 1066), layout_.page_rect(3));
EXPECT_PRED2(PpRectEq, pp::Rect(5, 3, 1060, 816), EXPECT_EQ(gfx::Rect(5, 3, 1060, 816), layout_.page_bounds_rect(0));
layout_.page_bounds_rect(0)); EXPECT_EQ(gfx::Rect(1067, 3, 814, 1046), layout_.page_bounds_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(1067, 3, 814, 1046), EXPECT_EQ(gfx::Rect(251, 1059, 814, 880), layout_.page_bounds_rect(2));
layout_.page_bounds_rect(1)); EXPECT_EQ(gfx::Rect(1067, 1059, 820, 1056), layout_.page_bounds_rect(3));
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_EQ(gfx::Size(2132, 2122), layout_.size()); EXPECT_EQ(gfx::Size(2132, 2122), layout_.size());
// Test case where there's an odd # of pages. // Test case where there's an odd # of pages.
page_sizes = {{200, 300}, {400, 200}, {300, 600}, {250, 500}, {300, 400}}; page_sizes = {{200, 300}, {400, 200}, {300, 600}, {250, 500}, {300, 400}};
layout_.ComputeTwoUpViewLayout(page_sizes); layout_.ComputeTwoUpViewLayout(page_sizes);
ASSERT_EQ(5u, layout_.page_count()); ASSERT_EQ(5u, layout_.page_count());
EXPECT_PRED2(PpRectEq, pp::Rect(200, 0, 200, 300), layout_.page_rect(0)); EXPECT_EQ(gfx::Rect(200, 0, 200, 300), layout_.page_rect(0));
EXPECT_PRED2(PpRectEq, pp::Rect(400, 0, 400, 200), layout_.page_rect(1)); EXPECT_EQ(gfx::Rect(400, 0, 400, 200), layout_.page_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(100, 300, 300, 600), layout_.page_rect(2)); EXPECT_EQ(gfx::Rect(100, 300, 300, 600), layout_.page_rect(2));
EXPECT_PRED2(PpRectEq, pp::Rect(400, 300, 250, 500), layout_.page_rect(3)); EXPECT_EQ(gfx::Rect(400, 300, 250, 500), layout_.page_rect(3));
EXPECT_PRED2(PpRectEq, pp::Rect(100, 900, 300, 400), layout_.page_rect(4)); EXPECT_EQ(gfx::Rect(100, 900, 300, 400), layout_.page_rect(4));
EXPECT_PRED2(PpRectEq, pp::Rect(205, 3, 194, 290), EXPECT_EQ(gfx::Rect(205, 3, 194, 290), layout_.page_bounds_rect(0));
layout_.page_bounds_rect(0)); EXPECT_EQ(gfx::Rect(401, 3, 394, 190), layout_.page_bounds_rect(1));
EXPECT_PRED2(PpRectEq, pp::Rect(401, 3, 394, 190), EXPECT_EQ(gfx::Rect(105, 303, 294, 590), layout_.page_bounds_rect(2));
layout_.page_bounds_rect(1)); EXPECT_EQ(gfx::Rect(401, 303, 244, 490), layout_.page_bounds_rect(3));
EXPECT_PRED2(PpRectEq, pp::Rect(105, 303, 294, 590), EXPECT_EQ(gfx::Rect(105, 903, 290, 390), layout_.page_bounds_rect(4));
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_EQ(gfx::Size(800, 1300), layout_.size()); EXPECT_EQ(gfx::Size(800, 1300), layout_.size());
} }

2
pdf/out_of_process_instance.cc

@ -1159,7 +1159,7 @@ void OutOfProcessInstance::ProposeDocumentLayout(const DocumentLayout& layout) {
dimensions.Set(kJSLayoutOptions, VarFromValue(layout.options().ToValue())); dimensions.Set(kJSLayoutOptions, VarFromValue(layout.options().ToValue()));
pp::VarArray page_dimensions_array; pp::VarArray page_dimensions_array;
for (size_t i = 0; i < layout.page_count(); ++i) { for (size_t i = 0; i < layout.page_count(); ++i) {
const pp::Rect& page_rect = layout.page_rect(i); const gfx::Rect& page_rect = layout.page_rect(i);
pp::VarDictionary page_dimensions; pp::VarDictionary page_dimensions;
page_dimensions.Set(kJSPageX, pp::Var(page_rect.x())); page_dimensions.Set(kJSPageX, pp::Var(page_rect.x()));
page_dimensions.Set(kJSPageY, pp::Var(page_rect.y())); page_dimensions.Set(kJSPageY, pp::Var(page_rect.y()));

2
pdf/pdfium/pdfium_engine.cc

@ -2698,7 +2698,7 @@ void PDFiumEngine::RefreshCurrentDocumentLayout() {
DCHECK_EQ(pages_.size(), layout_.page_count()); DCHECK_EQ(pages_.size(), layout_.page_count());
for (size_t i = 0; i < layout_.page_count(); ++i) { for (size_t i = 0; i < layout_.page_count(); ++i) {
// TODO(kmoon): This should be the only place that sets |PDFiumPage::rect_|. // TODO(kmoon): This should be the only place that sets |PDFiumPage::rect_|.
pages_[i]->set_rect(RectFromPPRect(layout_.page_bounds_rect(i))); pages_[i]->set_rect(layout_.page_bounds_rect(i));
} }
layout_.clear_dirty(); layout_.clear_dirty();