[PDF] Support fractional page sizes in Thumbnail class
Currently, the Thumbnail class gets instantiated with a page size in points, represented by gfx::Size, which uses integer values. The production caller gets float values from PDFium and truncates them when instantiating Thumbnails. Switch the class to take a gfx::SizeF instead, so callers can pass the page size in without truncating. This may make thumbnail size calculations slightly more accurate. It also reduces decision making that has to happen if more PDF code starts using gfx::SizeF for page sizes. The size value can pass straight in, without the need to make a decision on whether to floor or round the size value. Add some new test cases to show how the code behaves with fractional page sizes. Change-Id: Iff8da05c58daaf0a7f1df485ff2e98d2da1784fc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6287607 Reviewed-by: Alan Screen <awscreen@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/main@{#1423228}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
2d97c1abe0
commit
6dd47791b9
@ -2622,7 +2622,7 @@ class PdfViewWebPluginInkTest : public PdfViewWebPluginTest {
|
||||
blink::WebInputEventResult::kHandledApplication);
|
||||
}
|
||||
|
||||
void SendThumbnail(std::string_view message_id, const gfx::Size& page_size) {
|
||||
void SendThumbnail(std::string_view message_id, const gfx::SizeF& page_size) {
|
||||
base::Value::Dict reply;
|
||||
reply.Set("type", "getThumbnailReply");
|
||||
reply.Set("messageId", message_id);
|
||||
|
@ -1878,9 +1878,9 @@ Thumbnail PDFiumPage::CreateThumbnail(float device_pixel_ratio) {
|
||||
CHECK(available());
|
||||
|
||||
FPDF_PAGE page = GetPage();
|
||||
gfx::Size page_size(base::saturated_cast<int>(FPDF_GetPageWidthF(page)),
|
||||
base::saturated_cast<int>(FPDF_GetPageHeightF(page)));
|
||||
return Thumbnail(page_size, device_pixel_ratio);
|
||||
return Thumbnail(
|
||||
gfx::SizeF(FPDF_GetPageWidthF(page), FPDF_GetPageHeightF(page)),
|
||||
device_pixel_ratio);
|
||||
}
|
||||
|
||||
void PDFiumPage::MarkAvailable() {
|
||||
|
@ -15,6 +15,8 @@
|
||||
#include "base/numerics/checked_math.h"
|
||||
#include "base/values.h"
|
||||
#include "ui/gfx/geometry/size.h"
|
||||
#include "ui/gfx/geometry/size_conversions.h"
|
||||
#include "ui/gfx/geometry/size_f.h"
|
||||
|
||||
namespace chrome_pdf {
|
||||
|
||||
@ -48,23 +50,24 @@ constexpr int kPdfPageMaxDimension = 14400;
|
||||
constexpr int kPdfMaxAspectRatio = kPdfPageMaxDimension / kPdfPageMinDimension;
|
||||
|
||||
// Limit the proportions within PDF limits to handle pathological PDF pages.
|
||||
gfx::Size LimitAspectRatio(gfx::Size page_size) {
|
||||
gfx::SizeF LimitAspectRatio(gfx::SizeF page_size) {
|
||||
// Bump up any lengths of 0 to 1.
|
||||
page_size.SetToMax(gfx::Size(1, 1));
|
||||
|
||||
if (page_size.height() / page_size.width() > kPdfMaxAspectRatio)
|
||||
return gfx::Size(kPdfPageMinDimension, kPdfPageMaxDimension);
|
||||
if (page_size.width() / page_size.height() > kPdfMaxAspectRatio)
|
||||
return gfx::Size(kPdfPageMaxDimension, kPdfPageMinDimension);
|
||||
page_size.SetToMax(gfx::SizeF(1, 1));
|
||||
|
||||
if (page_size.height() / page_size.width() > kPdfMaxAspectRatio) {
|
||||
return gfx::SizeF(kPdfPageMinDimension, kPdfPageMaxDimension);
|
||||
}
|
||||
if (page_size.width() / page_size.height() > kPdfMaxAspectRatio) {
|
||||
return gfx::SizeF(kPdfPageMaxDimension, kPdfPageMinDimension);
|
||||
}
|
||||
return page_size;
|
||||
}
|
||||
|
||||
// Calculate the size of a thumbnail image in device pixels using `page_size` in
|
||||
// any units and `device_pixel_ratio`.
|
||||
gfx::Size CalculateBestFitSize(const gfx::Size& page_size,
|
||||
gfx::Size CalculateBestFitSize(const gfx::SizeF& page_size,
|
||||
float device_pixel_ratio) {
|
||||
gfx::Size safe_page_size = LimitAspectRatio(page_size);
|
||||
gfx::SizeF safe_page_size = LimitAspectRatio(page_size);
|
||||
|
||||
// Return the larger of the unrotated and rotated sizes to over-sample the PDF
|
||||
// page so that the thumbnail looks good in different orientations.
|
||||
@ -76,10 +79,11 @@ gfx::Size CalculateBestFitSize(const gfx::Size& page_size,
|
||||
std::max(safe_page_size.width(), safe_page_size.height());
|
||||
float scale = std::max(scale_portrait, scale_landscape) * device_pixel_ratio;
|
||||
|
||||
// Using gfx::ScaleToFlooredSize() is fine because `scale` will not yield an
|
||||
// Using gfx::ToFlooredSize() is fine because `scale` will not yield an
|
||||
// empty size unless `device_pixel_ratio` is very small (close to 0).
|
||||
// However, `device_pixel_ratio` support is limited to between 0.25 and 2.
|
||||
gfx::Size scaled_size = gfx::ScaleToFlooredSize(safe_page_size, scale);
|
||||
gfx::Size scaled_size =
|
||||
gfx::ToFlooredSize(gfx::ScaleSize(safe_page_size, scale));
|
||||
if (scaled_size.GetCheckedArea().ValueOrDefault(kMaxThumbnailPixels + 1) >
|
||||
kMaxThumbnailPixels) {
|
||||
// Recalculate `scale` to accommodate pixel size limit such that:
|
||||
@ -87,7 +91,7 @@ gfx::Size CalculateBestFitSize(const gfx::Size& page_size,
|
||||
// kMaxThumbnailPixels;
|
||||
scale = std::sqrt(static_cast<float>(kMaxThumbnailPixels) /
|
||||
safe_page_size.width() / safe_page_size.height());
|
||||
return gfx::ScaleToFlooredSize(safe_page_size, scale);
|
||||
return gfx::ToFlooredSize(gfx::ScaleSize(safe_page_size, scale));
|
||||
}
|
||||
|
||||
return scaled_size;
|
||||
@ -107,7 +111,7 @@ size_t CalculateImageDataSize(int stride, int height) {
|
||||
|
||||
} // namespace
|
||||
|
||||
Thumbnail::Thumbnail(const gfx::Size& page_size, float device_pixel_ratio)
|
||||
Thumbnail::Thumbnail(const gfx::SizeF& page_size, float device_pixel_ratio)
|
||||
: device_pixel_ratio_(std::clamp(device_pixel_ratio,
|
||||
kMinDevicePixelRatio,
|
||||
kMaxDevicePixelRatio)),
|
||||
|
@ -9,6 +9,10 @@
|
||||
#include "base/values.h"
|
||||
#include "ui/gfx/geometry/size.h"
|
||||
|
||||
namespace gfx {
|
||||
class SizeF;
|
||||
}
|
||||
|
||||
namespace chrome_pdf {
|
||||
|
||||
class Thumbnail;
|
||||
@ -17,7 +21,8 @@ using SendThumbnailCallback = base::OnceCallback<void(Thumbnail)>;
|
||||
|
||||
class Thumbnail final {
|
||||
public:
|
||||
Thumbnail(const gfx::Size& page_size, float device_pixel_ratio);
|
||||
// `page_size` is in points.
|
||||
Thumbnail(const gfx::SizeF& page_size, float device_pixel_ratio);
|
||||
Thumbnail(Thumbnail&& other) noexcept;
|
||||
Thumbnail& operator=(Thumbnail&& other) noexcept;
|
||||
~Thumbnail();
|
||||
|
@ -6,6 +6,7 @@
|
||||
|
||||
#include "testing/gtest/include/gtest/gtest.h"
|
||||
#include "ui/gfx/geometry/size.h"
|
||||
#include "ui/gfx/geometry/size_f.h"
|
||||
|
||||
namespace chrome_pdf {
|
||||
|
||||
@ -16,7 +17,7 @@ constexpr float kDeviceToPixelHigh = 2;
|
||||
|
||||
struct BestFitSizeParams {
|
||||
float device_pixel_ratio;
|
||||
gfx::Size page_size;
|
||||
gfx::SizeF page_size;
|
||||
gfx::Size expected_thumbnail_size;
|
||||
};
|
||||
|
||||
@ -45,8 +46,26 @@ TEST(ThumbnailTest, CalculateBestFitSizeNormal) {
|
||||
{kDeviceToPixelHigh, {50, 1500}, {46, 1399}}, // Super tall
|
||||
};
|
||||
|
||||
for (const auto& params : kBestFitSizeTestParams)
|
||||
for (const auto& params : kBestFitSizeTestParams) {
|
||||
TestBestFitSize(params);
|
||||
}
|
||||
}
|
||||
|
||||
TEST(ThumbnailTest, CalculateBestFitSizeFractional) {
|
||||
static constexpr BestFitSizeParams kBestFitSizeTestParams[] = {
|
||||
// ANSI Letter-ish
|
||||
{kDeviceToPixelLow, {611.976f, 791.968f}, {108, 140}},
|
||||
// ISO 216 A4
|
||||
{kDeviceToPixelLow, {595.35f, 841.995f}, {108, 152}},
|
||||
// ANSI Letter-ish
|
||||
{kDeviceToPixelHigh, {611.976f, 791.968f}, {216, 280}},
|
||||
// ISO 216 A4
|
||||
{kDeviceToPixelHigh, {595.35f, 841.995f}, {214, 303}},
|
||||
};
|
||||
|
||||
for (const auto& params : kBestFitSizeTestParams) {
|
||||
TestBestFitSize(params);
|
||||
}
|
||||
}
|
||||
|
||||
TEST(ThumbnailTest, CalculateBestFitSizeLargeAspectRatio) {
|
||||
@ -63,8 +82,9 @@ TEST(ThumbnailTest, CalculateBestFitSizeLargeAspectRatio) {
|
||||
{kDeviceToPixelHigh, {1, 9999999}, {3, 17701}}, // Very tall
|
||||
};
|
||||
|
||||
for (const auto& params : kBestFitSizeTestParams)
|
||||
for (const auto& params : kBestFitSizeTestParams) {
|
||||
TestBestFitSize(params);
|
||||
}
|
||||
}
|
||||
|
||||
TEST(ThumbnailTest, CalculateBestFitSizeNoOverflow) {
|
||||
@ -73,8 +93,9 @@ TEST(ThumbnailTest, CalculateBestFitSizeNoOverflow) {
|
||||
{kDeviceToPixelHigh, {9999999, 9999999}, {255, 255}}, // Very large
|
||||
};
|
||||
|
||||
for (const auto& params : kBestFitSizeTestParams)
|
||||
for (const auto& params : kBestFitSizeTestParams) {
|
||||
TestBestFitSize(params);
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
Reference in New Issue
Block a user