Skip to content

Commit

Permalink
Scale size and area by max DPI value for non-square DPI PrintSettings
Browse files Browse the repository at this point in the history
PrintSettings stores paper size and printable area in device units. The
device units for most platforms is the DPI multiplied by inches. For
printers with non-square DPI, the largest DPI value should be used.

When converting job settings to PrintSettings, the paper size and
printable area were scaled by their respective dimension's DPI instead
of the max DPI value. This meant that X and Y axes were represented
differently in paper size and printable area. Print Preview does not support rendering different dpi settings in the X and Y axis, so this
was causing incorrect renderings.

Fix places where the paper size and printable area should be scaled by
the max DPI.

(cherry picked from commit feaa7f8)

Bug: 1442457
Change-Id: I113cc58611c99795afdf5076552d05406e2912df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4508373
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Andy Phan <andyphan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1139921}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4509654
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5672_63@{#8}
Cr-Branched-From: 0e1a447-refs/branch-heads/5672@{#912}
Cr-Branched-From: 5f2a724-refs/heads/main@{#1121455}
  • Loading branch information
Andy Phan authored and Chromium LUCI CQ committed May 5, 2023
1 parent 135354b commit 500b41d
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 14 deletions.
12 changes: 6 additions & 6 deletions printing/print_settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -454,13 +454,13 @@ void PrintSettings::SetPrinterPrintableArea(
void PrintSettings::UpdatePrinterPrintableArea(
const gfx::Rect& printable_area_um) {
// Scale the page size and printable area to device units.
float x_scale = static_cast<float>(device_units_per_inch_size().width()) /
kMicronsPerInch;
float y_scale = static_cast<float>(device_units_per_inch_size().height()) /
kMicronsPerInch;

// Blink doesn't support different dpi settings in X and Y axis. Because of
// this, printers with non-square DPIs still scale page size and printable
// area using device_units_per_inch() instead of their respective dimensions
// in device_units_per_inch_size().
float scale = static_cast<float>(device_units_per_inch()) / kMicronsPerInch;
gfx::Rect printable_area_device_units =
gfx::ScaleToRoundedRect(printable_area_um, x_scale, y_scale);
gfx::ScaleToRoundedRect(printable_area_um, scale);

// Protect against misbehaving drivers. We have observed some drivers return
// incorrect values compared to page size. E.g., HP Business Inkjet 2300 PS.
Expand Down
16 changes: 8 additions & 8 deletions printing/print_settings_conversion.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,20 +101,20 @@ void SetPrintableAreaIfValid(PrintSettings& settings,
}

// Scale the page size and printable area to device units.
float x_scale =
static_cast<float>(settings.device_units_per_inch_size().width()) /
kMicronsPerInch;
float y_scale =
static_cast<float>(settings.device_units_per_inch_size().height()) /
kMicronsPerInch;
gfx::Size page_size = gfx::ScaleToRoundedSize(size_microns, x_scale, y_scale);
// Blink doesn't support different dpi settings in X and Y axis. Because of
// this, printers with non-square DPIs still scale page size and printable
// area using device_units_per_inch() instead of their respective dimensions
// in device_units_per_inch_size().
float scale =
static_cast<float>(settings.device_units_per_inch()) / kMicronsPerInch;
gfx::Size page_size = gfx::ScaleToRoundedSize(size_microns, scale);
// Flip the y-axis since the imageable area origin is at the bottom-left,
// while the gfx::Rect origin is at the top-left.
gfx::Rect printable_area = gfx::ScaleToRoundedRect(
{left_microns.value(), size_microns.height() - top_microns.value(),
right_microns.value() - left_microns.value(),
top_microns.value() - bottom_microns.value()},
x_scale, y_scale);
scale);
// Sanity check that the printable area makes sense.
if (printable_area.IsEmpty() ||
!gfx::Rect(page_size).Contains(printable_area)) {
Expand Down
48 changes: 48 additions & 0 deletions printing/print_settings_conversion_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,35 @@ const char kPrinterSettingsWithImageableArea[] = R"({
"dpiVertical": 300,
})";

#if !BUILDFLAG(IS_MAC)
const char kPrinterSettingsWithNonSquareDpi[] = R"({
"headerFooterEnabled": false,
"title": "Test Doc",
"url": "http://localhost/",
"shouldPrintBackgrounds": false,
"shouldPrintSelectionOnly": false,
"mediaSize": {
"height_microns": 297000,
"imageable_area_bottom_microns": 1000,
"imageable_area_left_microns": 0,
"imageable_area_right_microns": 180000,
"imageable_area_top_microns": 297000,
"width_microns": 210000
},
"collate": false,
"copies": 1,
"color": 2,
"duplex": 0,
"landscape": false,
"deviceName": "printer",
"scaleFactor": 100,
"rasterizePDF": false,
"pagesPerSheet": 1,
"dpiHorizontal": 800,
"dpiVertical": 50,
})";
#endif // !BUILDFLAG(IS_MAC)

const char kCustomMargins[] = R"({
"marginBottom": 10,
"marginLeft": 30,
Expand Down Expand Up @@ -243,6 +272,25 @@ TEST(PrintSettingsConversionTest, WithCustomMarginsAndImageableArea) {
EXPECT_EQ(page_setup.effective_margins(), kExpectedPageMargins);
}

#if !BUILDFLAG(IS_MAC)
TEST(PrintSettingsConversionTest, WithNonSquareDpi) {
// Check that physical size and printable area are scaled by the max DPI
// value. Not needed for macOS, which always has a square DPI.
static constexpr gfx::Size kExpectedSize{6614, 9354};
static constexpr gfx::Rect kExpectedPrintableArea{0, 0, 5669, 9323};

base::Value::Dict dict =
base::test::ParseJsonDict(kPrinterSettingsWithNonSquareDpi);
std::unique_ptr<PrintSettings> settings = PrintSettingsFromJobSettings(dict);
ASSERT_TRUE(settings);
EXPECT_EQ(settings->dpi_horizontal(), 800);
EXPECT_EQ(settings->dpi_vertical(), 50);
EXPECT_EQ(settings->page_setup_device_units().physical_size(), kExpectedSize);
EXPECT_EQ(settings->page_setup_device_units().printable_area(),
kExpectedPrintableArea);
}
#endif // !BUILDFLAG(IS_MAC)

TEST(PrintSettingsConversionTest, MissingDeviceName) {
base::Value::Dict dict = base::test::ParseJsonDict(kPrinterSettings);
EXPECT_TRUE(dict.Remove("deviceName"));
Expand Down

0 comments on commit 500b41d

Please sign in to comment.