Bug 1664258 - Increase the default PDF resolution for non-windows platforms. r=jwatt
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 11 Sep 2020 11:37:40 +0000
changeset 548305 34f188b4acfb552880d0af2f1b505f8375fffd1e
parent 548304 b76c123c607df6ea96958e632e8eb65b5628266f
child 548306 b7d28f2119a1cb8b7ac9c982d6745e73b4e53216
push id37776
push userbtara@mozilla.com
push dateFri, 11 Sep 2020 15:10:42 +0000
treeherdermozilla-central@b133e2d673e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1664258
milestone82.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1664258 - Increase the default PDF resolution for non-windows platforms. r=jwatt I've tested both webpage printing and pdf printing, in both the new UI and the system dialog, and this gives consistently better results, specially when images are involved. I've put the exact DPI value on a pref because I think that's the sensible thing to do, and allows us to test higher resolutions too. Differential Revision: https://phabricator.services.mozilla.com/D89805
modules/libpref/init/StaticPrefList.yaml
widget/nsIDeviceContextSpec.h
widget/windows/nsDeviceContextSpecWin.cpp
widget/windows/nsDeviceContextSpecWin.h
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -8617,16 +8617,40 @@
   mirror: always
 
 # Whether we allow the print progress dialog to show up.
 - name: print.show_print_progress
   type: RelaxedAtomicBool
   value: true
   mirror: always
 
+# The default DPI for printing.
+#
+# For PDF-based output, DPI should ideally be irrelevant, but in fact it is not
+# for multiple reasons:
+#
+#  * Layout code that tries to respect device pixels (e.g. for snapping glyph
+#    positions and baselines, and especially for the "GDI Classic"
+#    rendering-mode threshold for certain fonts).
+#
+#  * The limitations of the PDF format mean that we can't natively represent
+#    certain effects, such as filters, in PDF output, so we need to rasterize
+#    the parts of the document with these applied.
+#
+#  * Other rasterized things like images and such are also affected by DPI
+#    (both in the output, and the images we select via srcset, for example).
+#
+# Therefore, using a high DPI is preferable. For now, we use 144dpi to match
+# physical printer output on Windows, but higher (e.g. 300dpi) might be better
+# if it does not lead to issues such as excessive memory use.
+- name: print.default_dpi
+  type: float
+  value: 144.0f
+  mirror: always
+
 #---------------------------------------------------------------------------
 # Prefs starting with "privacy."
 #---------------------------------------------------------------------------
 
 - name: privacy.file_unique_origin
   type: bool
   value: true
   mirror: always
--- a/widget/nsIDeviceContextSpec.h
+++ b/widget/nsIDeviceContextSpec.h
@@ -3,16 +3,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsIDeviceContextSpec_h___
 #define nsIDeviceContextSpec_h___
 
 #include "gfxPoint.h"
 #include "nsISupports.h"
+#include "mozilla/StaticPrefs_print.h"
 
 class nsIWidget;
 class nsIPrintSettings;
 
 namespace mozilla {
 namespace gfx {
 class DrawEventRecorder;
 class PrintTarget;
@@ -57,24 +58,24 @@ class nsIDeviceContextSpec : public nsIS
     return NS_OK;
   }
 
   /**
    * Override to return something other than the default.
    *
    * @return DPI for printing.
    */
-  virtual float GetDPI() { return 72.0f; }
+  virtual float GetDPI() { return mozilla::StaticPrefs::print_default_dpi(); }
 
   /**
    * Override to return something other than the default.
    *
    * @return the printing scale to be applied to the context for printing.
    */
-  virtual float GetPrintingScale() { return 1.0f; }
+  virtual float GetPrintingScale() { return 72.0f / GetDPI(); }
 
   /**
    * Override to return something other than the default.
    *
    * @return the point to translate the context to for printing.
    */
   virtual gfxPoint GetPrintingTranslate() { return gfxPoint(0, 0); }
 
--- a/widget/windows/nsDeviceContextSpecWin.cpp
+++ b/widget/windows/nsDeviceContextSpecWin.cpp
@@ -54,24 +54,17 @@ using namespace mozilla::widget;
 #endif
 
 static const wchar_t kDriverName[] = L"WINSPOOL";
 
 //----------------------------------------------------------------------------------
 //---------------
 // static members
 //----------------------------------------------------------------------------------
-nsDeviceContextSpecWin::nsDeviceContextSpecWin()
-    : mDevMode(nullptr)
-#ifdef MOZ_ENABLE_SKIA_PDF
-      ,
-      mPrintViaSkPDF(false)
-#endif
-{
-}
+nsDeviceContextSpecWin::nsDeviceContextSpecWin() = default;
 
 //----------------------------------------------------------------------------------
 
 NS_IMPL_ISUPPORTS(nsDeviceContextSpecWin, nsIDeviceContextSpec)
 
 nsDeviceContextSpecWin::~nsDeviceContextSpecWin() {
   SetDevMode(nullptr);
 
@@ -330,48 +323,33 @@ already_AddRefed<PrintTarget> nsDeviceCo
     // The PrintTargetWindows takes over ownership of this DC
     return PrintTargetWindows::CreateOrNull(dc);
   }
 
   return nullptr;
 }
 
 float nsDeviceContextSpecWin::GetDPI() {
+  if (mOutputFormat == nsIPrintSettings::kOutputFormatPDF || mPrintViaSkPDF) {
+    return nsIDeviceContextSpec::GetDPI();
+  }
   // To match the previous printing code we need to return 144 when printing to
   // a Windows surface.
-  // For PDF-based output, DPI should ideally be irrelevant, but in fact it is
-  // not because of layout/rendering code that tries to respect device pixels
-  // (e.g. for snapping glyph positions and baselines, and especially for the
-  // "GDI Classic" rendering-mode threshold for certain fonts). Therefore,
-  // using a high DPI is preferable. For now, we use 144dpi to match physical-
-  // printer output, but higher (e.g. 300dpi) might be better if it does not
-  // lead to issues such as excessive memory use.
-#ifdef MOZ_ENABLE_SKIA_PDF
-  if (mPrintViaSkPDF) {
-    return 72.0f;  // XXX should we use a higher value here, too?
-  }
-#endif
   return 144.0f;
 }
 
 float nsDeviceContextSpecWin::GetPrintingScale() {
   MOZ_ASSERT(mPrintSettings);
-#ifdef MOZ_ENABLE_SKIA_PDF
-  if (mPrintViaSkPDF) {
-    return 72.0f / GetDPI();
-  }
-#endif
-  // For PDF output, nominal resolution is 72dpi, but our "device DPI" is
-  // higher to avoid low-res glyph spacing artifacts, so we need to return
-  // the appropriate scale here.
-  if (mOutputFormat == nsIPrintSettings::kOutputFormatPDF) {
-    return 72.0f / GetDPI();
+  if (mOutputFormat == nsIPrintSettings::kOutputFormatPDF || mPrintViaSkPDF) {
+    return nsIDeviceContextSpec::GetPrintingScale();
   }
 
   // The print settings will have the resolution stored from the real device.
+  //
+  // FIXME: Shouldn't we use this in GetDPI then instead of hard-coding 144.0?
   int32_t resolution;
   mPrintSettings->GetResolution(&resolution);
   return float(resolution) / GetDPI();
 }
 
 gfxPoint nsDeviceContextSpecWin::GetPrintingTranslate() {
   // The underlying surface on windows is the size of the printable region. When
   // the region is smaller than the actual paper size the (0, 0) coordinate
--- a/widget/windows/nsDeviceContextSpecWin.h
+++ b/widget/windows/nsDeviceContextSpecWin.h
@@ -62,32 +62,29 @@ class nsDeviceContextSpecWin : public ns
   void SetDeviceName(const nsAString& aDeviceName);
   void SetDriverName(const nsAString& aDriverName);
   void SetDevMode(LPDEVMODEW aDevMode);
 
   virtual ~nsDeviceContextSpecWin();
 
   nsString mDriverName;
   nsString mDeviceName;
-  LPDEVMODEW mDevMode;
+  LPDEVMODEW mDevMode = nullptr;
 
   nsCOMPtr<nsIPrintSettings> mPrintSettings;
   int16_t mOutputFormat = nsIPrintSettings::kOutputFormatNative;
 
   // A temporary file to create an "anonymous" print target. See bug 1664253,
   // this should ideally not be needed.
   nsCOMPtr<nsIFile> mTempFile;
 
-#ifdef MOZ_ENABLE_SKIA_PDF
-
   // This variable is independant of nsIPrintSettings::kOutputFormatPDF.
   // It controls both whether normal printing is done via PDF using Skia and
   // whether print-to-PDF uses Skia.
-  bool mPrintViaSkPDF;
-#endif
+  bool mPrintViaSkPDF = false;
 };
 
 //-------------------------------------------------------------------------
 // Printer List
 //-------------------------------------------------------------------------
 class nsPrinterListWin final : public nsPrinterListBase {
  public:
   NS_IMETHOD InitPrintSettingsFromPrinter(const nsAString&,