Printing: Make rectangular DPI printers work

Some printers use rectangular DPI only. Not accounting for this results
in distorted printouts when printing from Chrome. Send rectangular DPI
to Pdfium so that these printers can print correctly.

Bug: 738950
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I0b9e90905b7fefc38ca1d73d176d94e8e4a3b727
Reviewed-on: https://chromium-review.googlesource.com/664249
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Jianzhou Feng <jzfeng@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531769}

chrome/browser/printing/pdf_to_emf_converter_browsertest.cc

@@ -107,7 +107,8 @@ IN_PROC_BROWSER_TEST_F(PDFToEMFConverterBrowserTest, TestSuccess) {
 
   // A4 page format.
   PdfRenderSettings pdf_settings(gfx::Rect(0, 0, 1700, 2200), gfx::Point(0, 0),
-                                 /*dpi=*/200, /*autorotate=*/false,
+                                 /*dpi=*/gfx::Size(200, 200),
+                                 /*autorotate=*/false,
                                  PdfRenderSettings::Mode::NORMAL);
 
   constexpr int kNumberOfPages = 3;

chrome/browser/printing/print_job.cc

@@ -262,13 +262,14 @@ void PrintJob::StartPdfToEmfConversion(
   DCHECK(!pdf_conversion_state_);
   pdf_conversion_state_ =
       base::MakeUnique<PdfConversionState>(page_size, content_area);
-  const int kPrinterDpi = settings().dpi();
-  PdfRenderSettings settings(
-      content_area, gfx::Point(0, 0), kPrinterDpi, /*autorotate=*/true,
+  PdfRenderSettings render_settings(
+      content_area, gfx::Point(0, 0), settings().dpi_size(),
+      /*autorotate=*/true,
       print_text_with_gdi ? PdfRenderSettings::Mode::GDI_TEXT
                           : PdfRenderSettings::Mode::NORMAL);
   pdf_conversion_state_->Start(
-      bytes, settings, base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
+      bytes, render_settings,
+      base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
 }
 
 void PrintJob::OnPdfConversionStarted(int page_count) {
@@ -281,7 +282,7 @@ void PrintJob::OnPdfConversionStarted(int page_count) {
   }
   pdf_conversion_state_->set_page_count(page_count);
   pdf_conversion_state_->GetMorePages(
-      base::Bind(&PrintJob::OnPdfPageConverted, this));
+      base::BindRepeating(&PrintJob::OnPdfPageConverted, this));
 }
 
 void PrintJob::OnPdfPageConverted(int page_number,
@@ -312,13 +313,13 @@ void PrintJob::StartPdfToTextConversion(
   DCHECK(!pdf_conversion_state_);
   pdf_conversion_state_ =
       base::MakeUnique<PdfConversionState>(gfx::Size(), gfx::Rect());
-  const int kPrinterDpi = settings().dpi();
   gfx::Rect page_area = gfx::Rect(0, 0, page_size.width(), page_size.height());
-  PdfRenderSettings settings(page_area, gfx::Point(0, 0), kPrinterDpi,
-                             /*autorotate=*/true,
-                             PdfRenderSettings::Mode::TEXTONLY);
+  PdfRenderSettings render_settings(
+      page_area, gfx::Point(0, 0), settings().dpi_size(),
+      /*autorotate=*/true, PdfRenderSettings::Mode::TEXTONLY);
   pdf_conversion_state_->Start(
-      bytes, settings, base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
+      bytes, render_settings,
+      base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
 }
 
 void PrintJob::StartPdfToPostScriptConversion(
@@ -329,13 +330,14 @@ void PrintJob::StartPdfToPostScriptConversion(
   DCHECK(!pdf_conversion_state_);
   pdf_conversion_state_ = base::MakeUnique<PdfConversionState>(
       gfx::Size(), gfx::Rect());
-  const int kPrinterDpi = settings().dpi();
-  PdfRenderSettings settings(
-      content_area, physical_offsets, kPrinterDpi, /*autorotate=*/true,
+  PdfRenderSettings render_settings(
+      content_area, physical_offsets, settings().dpi_size(),
+      /*autorotate=*/true,
       ps_level2 ? PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2
                 : PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
   pdf_conversion_state_->Start(
-      bytes, settings, base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
+      bytes, render_settings,
+      base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
 }
 #endif  // defined(OS_WIN)
 

chrome/browser/printing/print_preview_pdf_generated_browsertest.cc

@@ -369,8 +369,8 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
 
       total_height_in_pixels += height_in_pixels;
       gfx::Rect rect(width_in_pixels, height_in_pixels);
-      PdfRenderSettings settings(rect, gfx::Point(0, 0), kDpi, true,
-                                 PdfRenderSettings::Mode::NORMAL);
+      PdfRenderSettings settings(rect, gfx::Point(0, 0), gfx::Size(kDpi, kDpi),
+                                 true, PdfRenderSettings::Mode::NORMAL);
 
       int int_max = std::numeric_limits<int>::max();
       if (settings.area.width() > int_max / kColorChannels ||
@@ -386,7 +386,7 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
       ASSERT_TRUE(chrome_pdf::RenderPDFPageToBitmap(
           pdf_data.data(), pdf_data.size(), i, page_bitmap_data.data(),
           settings.area.size().width(), settings.area.size().height(),
-          settings.dpi, settings.autorotate));
+          settings.dpi.width(), settings.dpi.height(), settings.autorotate));
       FillPng(&page_bitmap_data, width_in_pixels, max_width_in_pixels,
               settings.area.size().height());
       bitmap_data.insert(bitmap_data.end(),

chrome/browser/printing/printing_message_filter.cc

@@ -269,7 +269,7 @@ void PrintingMessageFilter::OnScriptedPrintReply(
   }
   PrintHostMsg_ScriptedPrint::WriteReplyParams(reply_msg, params);
   Send(reply_msg);
-  if (params.params.dpi && params.params.document_cookie) {
+  if (!params.params.dpi.IsEmpty() && params.params.document_cookie) {
 #if defined(OS_ANDROID)
     int file_descriptor;
     const base::string16& device_name = printer_query->settings().device_name();

chrome/browser/printing/pwg_raster_converter.cc

@@ -253,18 +253,31 @@ std::unique_ptr<PwgRasterConverter> PwgRasterConverter::CreateDefault() {
 PdfRenderSettings PwgRasterConverter::GetConversionSettings(
     const cloud_devices::CloudDeviceDescription& printer_capabilities,
     const gfx::Size& page_size) {
-  int dpi = kDefaultPdfDpi;
+  gfx::Size dpi = gfx::Size(kDefaultPdfDpi, kDefaultPdfDpi);
   cloud_devices::printer::DpiCapability dpis;
   if (dpis.LoadFrom(printer_capabilities))
-    dpi = std::max(dpis.GetDefault().horizontal, dpis.GetDefault().vertical);
-
-  const double scale = static_cast<double>(dpi) / kPointsPerInch;
+    dpi = gfx::Size(dpis.GetDefault().horizontal, dpis.GetDefault().vertical);
+
+  bool page_is_landscape =
+      static_cast<double>(page_size.width()) / dpi.width() >
+      static_cast<double>(page_size.height()) / dpi.height();
+
+  // Pdfium assumes that page width is given in dpi.width(), and height in
+  // dpi.height(). If we rotate the page, we need to also swap the DPIs.
+  gfx::Size final_page_size = page_size;
+  if (page_is_landscape) {
+    final_page_size = gfx::Size(page_size.height(), page_size.width());
+    dpi = gfx::Size(dpi.height(), dpi.width());
+  }
+  double scale_x = static_cast<double>(dpi.width()) / kPointsPerInch;
+  double scale_y = static_cast<double>(dpi.height()) / kPointsPerInch;
 
   // Make vertical rectangle to optimize streaming to printer. Fix orientation
   // by autorotate.
-  gfx::Rect area(std::min(page_size.width(), page_size.height()) * scale,
-                 std::max(page_size.width(), page_size.height()) * scale);
-  return PdfRenderSettings(area, gfx::Point(0, 0), dpi, /*autorotate=*/true,
+  gfx::Rect area(final_page_size.width() * scale_x,
+                 final_page_size.height() * scale_y);
+  return PdfRenderSettings(area, gfx::Point(0, 0), dpi,
+                           /*autorotate=*/true,
                            PdfRenderSettings::Mode::NORMAL);
 }
 

chrome/browser/printing/pwg_raster_converter_browsertest.cc

@@ -116,7 +116,8 @@ IN_PROC_BROWSER_TEST_F(PdfToPwgRasterBrowserTest, TestSuccessColor) {
   GetPdfData("pdf_to_pwg_raster_test.pdf", &test_data_dir, &pdf_data);
 
   PdfRenderSettings pdf_settings(gfx::Rect(0, 0, 500, 500), gfx::Point(0, 0),
-                                 /*dpi=*/1000, /*autorotate=*/false,
+                                 /*dpi=*/gfx::Size(1000, 1000),
+                                 /*autorotate=*/false,
                                  PdfRenderSettings::Mode::NORMAL);
   PwgRasterSettings pwg_settings;
   pwg_settings.odd_page_transform = PwgRasterTransformType::TRANSFORM_NORMAL;
@@ -142,7 +143,8 @@ IN_PROC_BROWSER_TEST_F(PdfToPwgRasterBrowserTest, TestSuccessMono) {
   GetPdfData("pdf_to_pwg_raster_test.pdf", &test_data_dir, &pdf_data);
 
   PdfRenderSettings pdf_settings(gfx::Rect(0, 0, 500, 500), gfx::Point(0, 0),
-                                 /*dpi=*/1000, /*autorotate=*/false,
+                                 /*dpi=*/gfx::Size(1000, 1000),
+                                 /*autorotate=*/false,
                                  PdfRenderSettings::Mode::NORMAL);
   PwgRasterSettings pwg_settings;
   pwg_settings.odd_page_transform = PwgRasterTransformType::TRANSFORM_NORMAL;

chrome/browser/ui/webui/print_preview/extension_printer_handler_unittest.cc

@@ -801,10 +801,12 @@ TEST_F(ExtensionPrinterHandlerTest, Print_Pwg) {
   EXPECT_FALSE(pwg_raster_converter_->bitmap_settings().reverse_page_order);
   EXPECT_TRUE(pwg_raster_converter_->bitmap_settings().use_color);
 
-  EXPECT_EQ(printing::kDefaultPdfDpi,
+  EXPECT_EQ(gfx::Size(printing::kDefaultPdfDpi, printing::kDefaultPdfDpi),
             pwg_raster_converter_->conversion_settings().dpi);
   EXPECT_TRUE(pwg_raster_converter_->conversion_settings().autorotate);
-  EXPECT_EQ("0,0 208x416",  // vertically_oriented_size  * dpi / points_per_inch
+  // size = vertically_oriented_size * vertical_dpi / points_per_inch x
+  //        horizontally_oriented_size * horizontal_dpi / points_per_inch
+  EXPECT_EQ("0,0 208x416",
             pwg_raster_converter_->conversion_settings().area.ToString());
 
   const PrinterProviderPrintJob* print_job = fake_api->GetNextPendingPrintJob();
@@ -855,10 +857,12 @@ TEST_F(ExtensionPrinterHandlerTest, Print_Pwg_NonDefaultSettings) {
   EXPECT_TRUE(pwg_raster_converter_->bitmap_settings().reverse_page_order);
   EXPECT_TRUE(pwg_raster_converter_->bitmap_settings().use_color);
 
-  EXPECT_EQ(200,  // max(vertical_dpi, horizontal_dpi)
+  EXPECT_EQ(gfx::Size(200, 100),
             pwg_raster_converter_->conversion_settings().dpi);
   EXPECT_TRUE(pwg_raster_converter_->conversion_settings().autorotate);
-  EXPECT_EQ("0,0 138x277",  // vertically_oriented_size  * dpi / points_per_inch
+  // size = vertically_oriented_size * vertical_dpi / points_per_inch x
+  //        horizontally_oriented_size * horizontal_dpi / points_per_inch
+  EXPECT_EQ("0,0 138x138",
             pwg_raster_converter_->conversion_settings().area.ToString());
 
   const PrinterProviderPrintJob* print_job = fake_api->GetNextPendingPrintJob();

chrome/service/cloud_print/print_system_win.cc

@@ -419,7 +419,9 @@ class JobSpoolerWin : public PrintSystem::JobSpooler {
     }
 
     void RenderPDFPages(const base::FilePath& pdf_path) {
-      int printer_dpi = ::GetDeviceCaps(printer_dc_.Get(), LOGPIXELSX);
+      gfx::Size printer_dpi =
+          gfx::Size(::GetDeviceCaps(printer_dc_.Get(), LOGPIXELSX),
+                    ::GetDeviceCaps(printer_dc_.Get(), LOGPIXELSY));
       int dc_width = GetDeviceCaps(printer_dc_.Get(), PHYSICALWIDTH);
       int dc_height = GetDeviceCaps(printer_dc_.Get(), PHYSICALHEIGHT);
       gfx::Rect render_area(0, 0, dc_width, dc_height);
@@ -432,7 +434,7 @@ class JobSpoolerWin : public PrintSystem::JobSpooler {
     void RenderPDFPagesInSandbox(
         const base::FilePath& pdf_path,
         const gfx::Rect& render_area,
-        int render_dpi,
+        const gfx::Size& render_dpi,
         const scoped_refptr<base::SingleThreadTaskRunner>& client_task_runner) {
       DCHECK(CurrentlyOnServiceIOThread());
       std::unique_ptr<ServiceUtilityProcessHost> utility_host(

chrome/services/printing/pdf_to_emf_converter.cc

@@ -149,7 +149,8 @@ bool PdfToEmfConverter::RenderPdfPageToMetafile(int page_number,
 
   if (!chrome_pdf::RenderPDFPageToDC(
           &pdf_data_.front(), pdf_data_.size(), page_number, metafile.context(),
-          pdf_render_settings_.dpi, pdf_render_settings_.area.x() - offset_x,
+          pdf_render_settings_.dpi.width(), pdf_render_settings_.dpi.height(),
+          pdf_render_settings_.area.x() - offset_x,
           pdf_render_settings_.area.y() - offset_y,
           pdf_render_settings_.area.width(), pdf_render_settings_.area.height(),
           true, false, true, true, pdf_render_settings_.autorotate)) {

chrome/services/printing/pdf_to_pwg_raster_converter.cc

@@ -55,13 +55,13 @@ bool RenderPdfPagesToPwgRaster(base::File pdf_file,
 
     if (!chrome_pdf::RenderPDFPageToBitmap(
             data.data(), data_size, page_number, image.pixel_data(),
-            image.size().width(), image.size().height(), settings.dpi,
-            settings.autorotate)) {
+            image.size().width(), image.size().height(), settings.dpi.width(),
+            settings.dpi.height(), settings.autorotate)) {
       return false;
     }
 
     cloud_print::PwgHeaderInfo header_info;
-    header_info.dpi = gfx::Size(settings.dpi, settings.dpi);
+    header_info.dpi = settings.dpi;
     header_info.total_pages = total_page_count;
     header_info.color_space = bitmap_settings.use_color
                                   ? cloud_print::PwgHeaderInfo::SRGB

chrome/services/printing/public/interfaces/pdf_render_settings.mojom

@@ -18,7 +18,7 @@ struct PdfRenderSettings {
 
   gfx.mojom.Rect area;
   gfx.mojom.Point offsets;
-  int32 dpi;
+  gfx.mojom.Size dpi;
   bool autorotate;
   Mode mode;
 };

chrome/services/printing/public/interfaces/pdf_render_settings_struct_traits.cc

@@ -13,10 +13,9 @@ bool StructTraits<printing::mojom::PdfRenderSettingsDataView,
                   printing::PdfRenderSettings>::
     Read(printing::mojom::PdfRenderSettingsDataView data,
          printing::PdfRenderSettings* out) {
-  out->dpi = data.dpi();
   out->autorotate = data.autorotate();
   return data.ReadArea(&out->area) && data.ReadOffsets(&out->offsets) &&
-         data.ReadMode(&out->mode);
+         data.ReadDpi(&out->dpi) && data.ReadMode(&out->mode);
 }
 
 }  // namespace mojo

chrome/services/printing/public/interfaces/pdf_render_settings_struct_traits.h

@@ -78,7 +78,7 @@ class StructTraits<printing::mojom::PdfRenderSettingsDataView,
   static gfx::Point offsets(const printing::PdfRenderSettings& settings) {
     return settings.offsets;
   }
-  static int32_t dpi(const printing::PdfRenderSettings& settings) {
+  static gfx::Size dpi(const printing::PdfRenderSettings& settings) {
     return settings.dpi;
   }
   static bool autorotate(const printing::PdfRenderSettings& settings) {

components/printing/browser/print_manager_utils.cc

@@ -53,7 +53,7 @@ void RenderParamsFromPrintSettings(const PrintSettings& settings,
       settings.page_setup_device_units().printable_area().height());
   params->margin_top = settings.page_setup_device_units().content_area().y();
   params->margin_left = settings.page_setup_device_units().content_area().x();
-  params->dpi = settings.dpi();
+  params->dpi = settings.dpi_size();
   params->scale_factor = settings.scale_factor();
   params->rasterize_pdf = settings.rasterize_pdf();
   // Always use an invalid cookie.

components/printing/common/print_messages.cc

@@ -65,7 +65,7 @@ PrintMsg_Print_Params::PrintMsg_Print_Params()
       printable_area(),
       margin_top(0),
       margin_left(0),
-      dpi(0),
+      dpi(),
       scale_factor(1.0f),
       rasterize_pdf(false),
       document_cookie(0),
@@ -95,7 +95,7 @@ void PrintMsg_Print_Params::Reset() {
   printable_area = gfx::Rect();
   margin_top = 0;
   margin_left = 0;
-  dpi = 0;
+  dpi = gfx::Size();
   scale_factor = 1.0f;
   rasterize_pdf = false;
   document_cookie = 0;

components/printing/common/print_messages.h

@@ -49,7 +49,7 @@ struct PrintMsg_Print_Params {
   gfx::Rect printable_area;
   int margin_top;
   int margin_left;
-  double dpi;
+  gfx::Size dpi;
   double scale_factor;
   bool rasterize_pdf;
   int document_cookie;
@@ -126,7 +126,7 @@ IPC_STRUCT_TRAITS_BEGIN(PrintMsg_Print_Params)
   // in pixels according to dpi.
   IPC_STRUCT_TRAITS_MEMBER(page_size)
 
-  // In pixels according to dpi_x and dpi_y.
+  // In pixels according to dpi.
   IPC_STRUCT_TRAITS_MEMBER(content_size)
 
   // Physical printable area of the page in pixels according to dpi.
@@ -138,7 +138,7 @@ IPC_STRUCT_TRAITS_BEGIN(PrintMsg_Print_Params)
   // The x-offset of the printable area, in pixels according to dpi.
   IPC_STRUCT_TRAITS_MEMBER(margin_left)
 
-  // Specifies dots per inch.
+  // Specifies dots per inch in the x and y direction.
   IPC_STRUCT_TRAITS_MEMBER(dpi)
 
   // Specifies the scale factor in percent

components/printing/renderer/print_render_frame_helper.cc

@@ -117,15 +117,19 @@ int GetDPI(const PrintMsg_Print_Params* print_params) {
   // on dpi.
   return kPointsPerInch;
 #else
-  return static_cast<int>(print_params->dpi);
+  // Render using the higher of the two resolutions in both dimensions to
+  // prevent bad quality print jobs on rectantular DPI printers.
+  return static_cast<int>(
+      std::max(print_params->dpi.width(), print_params->dpi.height()));
 #endif  // defined(OS_MACOSX)
 }
 
 bool PrintMsg_Print_Params_IsValid(const PrintMsg_Print_Params& params) {
   return !params.content_size.IsEmpty() && !params.page_size.IsEmpty() &&
          !params.printable_area.IsEmpty() && params.document_cookie &&
-         params.dpi && params.margin_top >= 0 && params.margin_left >= 0 &&
-         params.dpi > kMinDpi && params.document_cookie != 0;
+         !params.dpi.IsEmpty() && params.dpi.width() > kMinDpi &&
+         params.dpi.height() > kMinDpi && params.margin_top >= 0 &&
+         params.margin_left >= 0 && params.document_cookie != 0;
 }
 
 // Helper function to check for fit to page
@@ -524,7 +528,6 @@ PrintMsg_Print_Params CalculatePrintParamsForCss(
 
   return result_params;
 }
-
 }  // namespace
 
 FrameReference::FrameReference(blink::WebLocalFrame* frame) {
@@ -1502,7 +1505,8 @@ void PrintRenderFrameHelper::Print(blink::WebLocalFrame* frame,
 
     print_settings.params.print_scaling_option = scaling_option;
     SetPrintPagesParams(print_settings);
-    if (!print_settings.params.dpi || !print_settings.params.document_cookie) {
+    if (print_settings.params.dpi.IsEmpty() ||
+        !print_settings.params.document_cookie) {
       DidFinishPrinting(OK);  // Release resources and fail silently on failure.
       return;
     }

components/printing/test/mock_printer.cc

@@ -109,7 +109,8 @@ void MockPrinter::GetDefaultPrintSettings(PrintMsg_Print_Params* params) {
 }
 
 void MockPrinter::SetDefaultPrintSettings(const PrintMsg_Print_Params& params) {
-  dpi_ = params.dpi;
+  // Use the same logic as in printing/print_settings.h
+  dpi_ = std::max(params.dpi.width(), params.dpi.height());
   selection_only_ = params.selection_only;
   should_print_backgrounds_ = params.should_print_backgrounds;
   page_size_ = params.page_size;
@@ -145,7 +146,7 @@ void MockPrinter::ScriptedPrint(int cookie,
 
   settings->Reset();
 
-  settings->params.dpi = dpi_;
+  settings->params.dpi = gfx::Size(dpi_, dpi_);
   settings->params.selection_only = selection_only_;
   settings->params.should_print_backgrounds = should_print_backgrounds_;
   settings->params.document_cookie = document_cookie_;
@@ -286,7 +287,7 @@ int MockPrinter::CreateDocumentCookie() {
 }
 
 void MockPrinter::SetPrintParams(PrintMsg_Print_Params* params) {
-  params->dpi = dpi_;
+  params->dpi = gfx::Size(dpi_, dpi_);
   params->selection_only = selection_only_;
   params->should_print_backgrounds = should_print_backgrounds_;
   params->document_cookie = document_cookie_;