0

Refactor default page orientation to an enum

Changes the type of the default page orientation from an int to an enum
class, chrome_pdf::PageOrientation. The RotateClockwise() and
RotateCounterclockwise() operations step through the orientations.

Also adds a chrome_pdf::ToPDFiumRotation(PageOrientation) helper to
convert PageOrientation enums to the equivalent integer expected by
PDFium APIs like FDPF_RenderPage().

Bug: 885110
Change-Id: I05d69a8548bf5b72666e6d58136440815ba8d15c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1740649
Commit-Queue: K Moon <kmoon@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684900}
This commit is contained in:
K Moon
2019-08-07 20:05:36 +00:00
committed by Commit Bot
parent 14c4b07d0c
commit 9a62bf44b2
13 changed files with 245 additions and 57 deletions

@ -61,6 +61,8 @@ if (enable_pdf) {
"draw_utils/shadow.h",
"out_of_process_instance.cc",
"out_of_process_instance.h",
"page_orientation.cc",
"page_orientation.h",
"paint_aggregator.cc",
"paint_aggregator.h",
"paint_manager.cc",
@ -166,6 +168,7 @@ if (enable_pdf) {
"document_loader_impl_unittest.cc",
"draw_utils/coordinates_unittest.cc",
"out_of_process_instance_unittest.cc",
"page_orientation_unittest.cc",
"pdf_transform_unittest.cc",
"range_set_unittest.cc",
]
@ -197,6 +200,7 @@ if (enable_pdf) {
"pdfium/accessibility_unittest.cc",
"pdfium/findtext_unittest.cc",
"pdfium/pdfium_engine_exports_unittest.cc",
"pdfium/pdfium_page_unittest.cc",
"pdfium/pdfium_permissions_unittest.cc",
"pdfium/pdfium_print_unittest.cc",
"pdfium/pdfium_test_base.cc",

@ -20,12 +20,11 @@ DocumentLayout::Options& DocumentLayout::Options::operator=(
DocumentLayout::Options::~Options() = default;
void DocumentLayout::Options::RotatePagesClockwise() {
default_page_orientation_ = (default_page_orientation_ + 1) % 4;
default_page_orientation_ = RotateClockwise(default_page_orientation_);
}
void DocumentLayout::Options::RotatePagesCounterclockwise() {
// Use the modular additive inverse of -1 to avoid negative values.
default_page_orientation_ = (default_page_orientation_ + 3) % 4;
default_page_orientation_ = RotateCounterclockwise(default_page_orientation_);
}
DocumentLayout::DocumentLayout() = default;

@ -8,6 +8,7 @@
#include <vector>
#include "pdf/draw_utils/coordinates.h"
#include "pdf/page_orientation.h"
#include "ppapi/cpp/rect.h"
#include "ppapi/cpp/size.h"
@ -32,14 +33,9 @@ class DocumentLayout final {
~Options();
// Returns the default page orientation, encoded as an integer from 0 to 3
// (inclusive).
//
// A return value of 0 indicates the original page orientation, with each
// increment indicating clockwise rotation by an additional 90 degrees.
//
// TODO(kmoon): Return an enum (class) instead of an integer.
int default_page_orientation() const { return default_page_orientation_; }
PageOrientation default_page_orientation() const {
return default_page_orientation_;
}
// Rotates default page orientation 90 degrees clockwise.
void RotatePagesClockwise();
@ -48,8 +44,7 @@ class DocumentLayout final {
void RotatePagesCounterclockwise();
private:
// Orientations are non-negative integers modulo 4.
int default_page_orientation_ = 0;
PageOrientation default_page_orientation_ = PageOrientation::kOriginal;
};
static const draw_utils::PageInsetSizes kSingleViewInsets;

@ -35,61 +35,66 @@ inline bool PpRectEq(const pp::Rect& lhs, const pp::Rect& rhs) {
}
TEST_F(DocumentLayoutOptionsTest, DefaultConstructor) {
EXPECT_EQ(options_.default_page_orientation(), 0);
EXPECT_EQ(options_.default_page_orientation(), PageOrientation::kOriginal);
}
TEST_F(DocumentLayoutOptionsTest, CopyConstructor) {
options_.RotatePagesClockwise();
DocumentLayout::Options copy(options_);
EXPECT_EQ(copy.default_page_orientation(), 1);
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kClockwise90);
options_.RotatePagesClockwise();
EXPECT_EQ(copy.default_page_orientation(), 1);
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kClockwise90);
}
TEST_F(DocumentLayoutOptionsTest, CopyAssignment) {
options_.RotatePagesClockwise();
DocumentLayout::Options copy;
EXPECT_EQ(copy.default_page_orientation(), 0);
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kOriginal);
copy = options_;
EXPECT_EQ(copy.default_page_orientation(), 1);
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kClockwise90);
options_.RotatePagesClockwise();
EXPECT_EQ(copy.default_page_orientation(), 1);
EXPECT_EQ(copy.default_page_orientation(), PageOrientation::kClockwise90);
}
TEST_F(DocumentLayoutOptionsTest, RotatePagesClockwise) {
options_.RotatePagesClockwise();
EXPECT_EQ(options_.default_page_orientation(), 1);
EXPECT_EQ(options_.default_page_orientation(), PageOrientation::kClockwise90);
options_.RotatePagesClockwise();
EXPECT_EQ(options_.default_page_orientation(), 2);
EXPECT_EQ(options_.default_page_orientation(),
PageOrientation::kClockwise180);
options_.RotatePagesClockwise();
EXPECT_EQ(options_.default_page_orientation(), 3);
EXPECT_EQ(options_.default_page_orientation(),
PageOrientation::kClockwise270);
options_.RotatePagesClockwise();
EXPECT_EQ(options_.default_page_orientation(), 0);
EXPECT_EQ(options_.default_page_orientation(), PageOrientation::kOriginal);
}
TEST_F(DocumentLayoutOptionsTest, RotatePagesCounterclockwise) {
options_.RotatePagesCounterclockwise();
EXPECT_EQ(options_.default_page_orientation(), 3);
EXPECT_EQ(options_.default_page_orientation(),
PageOrientation::kClockwise270);
options_.RotatePagesCounterclockwise();
EXPECT_EQ(options_.default_page_orientation(), 2);
EXPECT_EQ(options_.default_page_orientation(),
PageOrientation::kClockwise180);
options_.RotatePagesCounterclockwise();
EXPECT_EQ(options_.default_page_orientation(), 1);
EXPECT_EQ(options_.default_page_orientation(), PageOrientation::kClockwise90);
options_.RotatePagesCounterclockwise();
EXPECT_EQ(options_.default_page_orientation(), 0);
EXPECT_EQ(options_.default_page_orientation(), PageOrientation::kOriginal);
}
TEST_F(DocumentLayoutTest, DefaultConstructor) {
EXPECT_EQ(layout_.options().default_page_orientation(), 0);
EXPECT_EQ(layout_.options().default_page_orientation(),
PageOrientation::kOriginal);
EXPECT_PRED2(PpSizeEq, layout_.size(), pp::Size(0, 0));
}
@ -99,7 +104,8 @@ TEST_F(DocumentLayoutTest, SetOptionsDoesNotRecomputeLayout) {
DocumentLayout::Options options;
options.RotatePagesClockwise();
layout_.set_options(options);
EXPECT_EQ(layout_.options().default_page_orientation(), 1);
EXPECT_EQ(layout_.options().default_page_orientation(),
PageOrientation::kClockwise90);
EXPECT_PRED2(PpSizeEq, layout_.size(), pp::Size(1, 2));
}

38
pdf/page_orientation.cc Normal file

@ -0,0 +1,38 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "pdf/page_orientation.h"
#include <type_traits>
namespace chrome_pdf {
namespace {
// Adds two PageOrientation values together. This works because the underlying
// integer values have been chosen to allow modular arithmetic.
PageOrientation AddOrientations(PageOrientation first, PageOrientation second) {
using IntType = std::underlying_type<PageOrientation>::type;
constexpr auto kOrientationCount =
static_cast<IntType>(PageOrientation::kLast) + 1;
auto first_int = static_cast<IntType>(first);
auto second_int = static_cast<IntType>(second);
return static_cast<PageOrientation>((first_int + second_int) %
kOrientationCount);
}
} // namespace
PageOrientation RotateClockwise(PageOrientation orientation) {
return AddOrientations(orientation, PageOrientation::kClockwise90);
}
PageOrientation RotateCounterclockwise(PageOrientation orientation) {
// Adding |kLast| is equivalent to rotating one step counterclockwise.
return AddOrientations(orientation, PageOrientation::kLast);
}
} // namespace chrome_pdf

39
pdf/page_orientation.h Normal file

@ -0,0 +1,39 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef PDF_PAGE_ORIENTATION_H_
#define PDF_PAGE_ORIENTATION_H_
#include <cstdint>
namespace chrome_pdf {
// Enumeration of allowed page orientations. Assigned values permit simple
// modular arithmetic on orientations.
enum class PageOrientation : uint8_t {
// Original orientation.
kOriginal = 0,
// Rotated clockwise 90 degrees.
kClockwise90 = 1,
// Rotated (clockwise) 180 degrees.
kClockwise180 = 2,
// Rotated clockwise 270 degrees (counterclockwise 90 degrees).
kClockwise270 = 3,
// Last enumeration value.
kLast = kClockwise270
};
// Rotates a page orientation clockwise by one step (90 degrees).
PageOrientation RotateClockwise(PageOrientation orientation);
// Rotates a page orientation counterclockwise by one step (90 degrees).
PageOrientation RotateCounterclockwise(PageOrientation orientation);
} // namespace chrome_pdf
#endif // PDF_PAGE_ORIENTATION_H_

@ -0,0 +1,39 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "pdf/page_orientation.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace chrome_pdf {
namespace {
TEST(PageOrientationTest, RotateClockwise) {
EXPECT_EQ(RotateClockwise(PageOrientation::kOriginal),
PageOrientation::kClockwise90);
EXPECT_EQ(RotateClockwise(PageOrientation::kClockwise90),
PageOrientation::kClockwise180);
EXPECT_EQ(RotateClockwise(PageOrientation::kClockwise180),
PageOrientation::kClockwise270);
EXPECT_EQ(RotateClockwise(PageOrientation::kClockwise270),
PageOrientation::kOriginal);
EXPECT_EQ(RotateClockwise(PageOrientation::kLast),
PageOrientation::kOriginal);
}
TEST(PageOrientationTest, RotateCounterclockwise) {
EXPECT_EQ(RotateCounterclockwise(PageOrientation::kOriginal),
PageOrientation::kClockwise270);
EXPECT_EQ(RotateCounterclockwise(PageOrientation::kClockwise90),
PageOrientation::kOriginal);
EXPECT_EQ(RotateCounterclockwise(PageOrientation::kClockwise180),
PageOrientation::kClockwise90);
EXPECT_EQ(RotateCounterclockwise(PageOrientation::kClockwise270),
PageOrientation::kClockwise180);
EXPECT_EQ(RotateCounterclockwise(PageOrientation::kLast),
PageOrientation::kClockwise180);
}
} // namespace
} // namespace chrome_pdf

@ -2632,8 +2632,19 @@ pp::Size PDFiumEngine::GetPageSize(int index) {
ConvertUnitDouble(width_in_points, kPointsPerInch, kPixelsPerInch));
int height_in_pixels = static_cast<int>(
ConvertUnitDouble(height_in_points, kPointsPerInch, kPixelsPerInch));
if (layout_.options().default_page_orientation() % 2 == 1)
std::swap(width_in_pixels, height_in_pixels);
switch (layout_.options().default_page_orientation()) {
case PageOrientation::kOriginal:
case PageOrientation::kClockwise180:
// No axis swap needed.
break;
case PageOrientation::kClockwise90:
case PageOrientation::kClockwise270:
// Rotated 90 degrees: swap axes.
std::swap(width_in_pixels, height_in_pixels);
break;
}
size = pp::Size(width_in_pixels, height_in_pixels);
}
return size;
@ -2729,8 +2740,8 @@ bool PDFiumEngine::ContinuePaint(int progressive_index,
0xFFFFFFFF);
rv = FPDF_RenderPageBitmap_Start(
new_bitmap.get(), page, start_x, start_y, size_x, size_y,
layout_.options().default_page_orientation(), GetRenderingFlags(),
this);
ToPDFiumRotation(layout_.options().default_page_orientation()),
GetRenderingFlags(), this);
progressive_paints_[progressive_index].SetBitmapAndImageData(
std::move(new_bitmap), *image_data);
}
@ -2757,7 +2768,8 @@ void PDFiumEngine::FinishPaint(int progressive_index,
// Draw the forms.
FPDF_FFLDraw(form(), bitmap, pages_[page_index]->GetPage(), start_x, start_y,
size_x, size_y, layout_.options().default_page_orientation(),
size_x, size_y,
ToPDFiumRotation(layout_.options().default_page_orientation()),
GetRenderingFlags());
FillPageSides(progressive_index);
@ -3171,8 +3183,8 @@ void PDFiumEngine::DeviceToPage(int page_index,
FPDF_BOOL ret = FPDF_DeviceToPage(
pages_[page_index]->GetPage(), 0, 0, pages_[page_index]->rect().width(),
pages_[page_index]->rect().height(),
layout_.options().default_page_orientation(), temp_x, temp_y, page_x,
page_y);
ToPDFiumRotation(layout_.options().default_page_orientation()), temp_x,
temp_y, page_x, page_y);
DCHECK(ret);
}

@ -231,7 +231,7 @@ pp::FloatRect PDFiumPage::GetCharBounds(int char_index) {
}
PDFiumPage::Area PDFiumPage::GetCharIndex(const pp::Point& point,
int rotation,
PageOrientation orientation,
int* char_index,
int* form_type,
LinkTarget* target) {
@ -240,9 +240,9 @@ PDFiumPage::Area PDFiumPage::GetCharIndex(const pp::Point& point,
pp::Point point2 = point - rect_.point();
double new_x;
double new_y;
FPDF_BOOL ret =
FPDF_DeviceToPage(GetPage(), 0, 0, rect_.width(), rect_.height(),
rotation, point2.x(), point2.y(), &new_x, &new_y);
FPDF_BOOL ret = FPDF_DeviceToPage(
GetPage(), 0, 0, rect_.width(), rect_.height(),
ToPDFiumRotation(orientation), point2.x(), point2.y(), &new_x, &new_y);
DCHECK(ret);
// hit detection tolerance, in points.
@ -426,8 +426,9 @@ int PDFiumPage::GetLink(int char_index, LinkTarget* target) {
double top;
FPDFText_GetCharBox(GetTextPage(), char_index, &left, &right, &bottom, &top);
pp::Point origin(
PageToScreen(pp::Point(), 1.0, left, top, right, bottom, 0).point());
pp::Point origin(PageToScreen(pp::Point(), 1.0, left, top, right, bottom,
PageOrientation::kOriginal)
.point());
for (size_t i = 0; i < links_.size(); ++i) {
for (const auto& rect : links_[i].rects) {
if (rect.Contains(origin)) {
@ -490,8 +491,8 @@ void PDFiumPage::CalculateLinks() {
double right;
double bottom;
FPDFLink_GetRect(links, i, j, &left, &top, &right, &bottom);
pp::Rect rect =
PageToScreen(pp::Point(), 1.0, left, top, right, bottom, 0);
pp::Rect rect = PageToScreen(pp::Point(), 1.0, left, top, right, bottom,
PageOrientation::kOriginal);
if (rect.IsEmpty())
continue;
link.rects.push_back(rect);
@ -550,7 +551,7 @@ pp::Rect PDFiumPage::PageToScreen(const pp::Point& offset,
double top,
double right,
double bottom,
int rotation) const {
PageOrientation orientation) const {
if (!available_)
return pp::Rect();
@ -571,13 +572,13 @@ pp::Rect PDFiumPage::PageToScreen(const pp::Point& offset,
int new_bottom;
FPDF_BOOL ret = FPDF_PageToDevice(
page(), static_cast<int>(start_x), static_cast<int>(start_y),
static_cast<int>(ceil(size_x)), static_cast<int>(ceil(size_y)), rotation,
left, top, &new_left, &new_top);
static_cast<int>(ceil(size_x)), static_cast<int>(ceil(size_y)),
ToPDFiumRotation(orientation), left, top, &new_left, &new_top);
DCHECK(ret);
ret = FPDF_PageToDevice(
page(), static_cast<int>(start_x), static_cast<int>(start_y),
static_cast<int>(ceil(size_x)), static_cast<int>(ceil(size_y)), rotation,
right, bottom, &new_right, &new_bottom);
static_cast<int>(ceil(size_x)), static_cast<int>(ceil(size_y)),
ToPDFiumRotation(orientation), right, bottom, &new_right, &new_bottom);
DCHECK(ret);
// If the PDF is rotated, the horizontal/vertical coordinates could be
@ -637,4 +638,21 @@ PDFiumPage::Link::Link(const Link& that) = default;
PDFiumPage::Link::~Link() = default;
int ToPDFiumRotation(PageOrientation orientation) {
// Could static_cast<int>(orientation), but using an exhaustive switch will
// trigger an error if we ever change the definition of PageOrientation.
switch (orientation) {
case PageOrientation::kOriginal:
return 0;
case PageOrientation::kClockwise90:
return 1;
case PageOrientation::kClockwise180:
return 2;
case PageOrientation::kClockwise270:
return 3;
}
NOTREACHED();
return 0;
}
} // namespace chrome_pdf

@ -10,6 +10,7 @@
#include "base/optional.h"
#include "base/strings/string16.h"
#include "pdf/page_orientation.h"
#include "pdf/pdf_engine.h"
#include "ppapi/cpp/rect.h"
#include "third_party/pdfium/public/cpp/fpdf_scopers.h"
@ -88,7 +89,7 @@ class PDFiumPage {
// Target is optional. It will be filled in for WEBLINK_AREA or
// DOCLINK_AREA only.
Area GetCharIndex(const pp::Point& point,
int rotation,
PageOrientation orientation,
int* char_index,
int* form_type,
LinkTarget* target);
@ -116,7 +117,7 @@ class PDFiumPage {
double top,
double right,
double bottom,
int rotation) const;
PageOrientation orientation) const;
const PDFEngine::PageFeatures* GetPageFeatures();
@ -181,6 +182,10 @@ class PDFiumPage {
DISALLOW_COPY_AND_ASSIGN(PDFiumPage);
};
// Converts page orientations to the PDFium equivalents, as defined by
// FPDF_RenderPage().
int ToPDFiumRotation(PageOrientation orientation);
} // namespace chrome_pdf
#endif // PDF_PDFIUM_PDFIUM_PAGE_H_

@ -0,0 +1,31 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "pdf/pdfium/pdfium_page.h"
#include "base/logging.h"
#include "base/test/gtest_util.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace chrome_pdf {
namespace {
TEST(PDFiumPageTest, ToPDFiumRotation) {
EXPECT_EQ(ToPDFiumRotation(PageOrientation::kOriginal), 0);
EXPECT_EQ(ToPDFiumRotation(PageOrientation::kClockwise90), 1);
EXPECT_EQ(ToPDFiumRotation(PageOrientation::kClockwise180), 2);
EXPECT_EQ(ToPDFiumRotation(PageOrientation::kClockwise270), 3);
}
TEST(PDFiumPageDeathTest, ToPDFiumRotation) {
PageOrientation invalid_orientation = static_cast<PageOrientation>(-1);
#if DCHECK_IS_ON()
EXPECT_DCHECK_DEATH(ToPDFiumRotation(invalid_orientation));
#else
EXPECT_EQ(ToPDFiumRotation(invalid_orientation), 0);
#endif
}
} // namespace
} // namespace chrome_pdf

@ -54,7 +54,7 @@ void PDFiumRange::SetCharCount(int char_count) {
const std::vector<pp::Rect>& PDFiumRange::GetScreenRects(
const pp::Point& offset,
double zoom,
int rotation) const {
PageOrientation orientation) const {
if (offset == cached_screen_rects_offset_ &&
zoom == cached_screen_rects_zoom_) {
return cached_screen_rects_;
@ -82,8 +82,8 @@ const std::vector<pp::Rect>& PDFiumRange::GetScreenRects(
double right;
double bottom;
FPDFText_GetRect(page_->GetTextPage(), i, &left, &top, &right, &bottom);
pp::Rect rect =
page_->PageToScreen(offset, zoom, left, top, right, bottom, rotation);
pp::Rect rect = page_->PageToScreen(offset, zoom, left, top, right, bottom,
orientation);
if (rect.IsEmpty())
continue;
cached_screen_rects_.push_back(rect);

@ -9,6 +9,7 @@
#include <vector>
#include "base/strings/string16.h"
#include "pdf/page_orientation.h"
#include "pdf/pdfium/pdfium_page.h"
#include "ppapi/cpp/rect.h"
@ -38,9 +39,10 @@ class PDFiumRange {
int char_count() const { return char_count_; }
// Gets bounding rectangles of range in screen coordinates.
const std::vector<pp::Rect>& GetScreenRects(const pp::Point& offset,
double zoom,
int rotation) const;
const std::vector<pp::Rect>& GetScreenRects(
const pp::Point& offset,
double zoom,
PageOrientation orientation) const;
// Gets the string of characters in this range.
base::string16 GetText() const;