QPagedPainterDevicePrivate: Remove m_pageLayout

Having it there is awkward since all the subclasses of QPagedPaintDevice,
that is QPdfWriter and QPrinter, have their own m_pageLayout via
their private engine classes so we ended up with code like
   pd->engine->setPageMargins(margins, units);
   // Set QPagedPaintDevice layout to match the current paint engine layout
   m_pageLayout = pd->engine->pageLayout();

Now we just use the subclass for it's page layout and all is simpler
since we don't need to make sure the two variables are updated to
have the same contents.

Unfortunately this means that we have to implement a dummy subclass
for QPagedPaintDevice(). That constructor doesn't make any sense since
QPagedPaintDevice is not really a leaf you want to instantiate, it's
there to provide common api for the subclasses and the
QPagedPaintDevice(QPagedPaintDevicePrivate *dd) constructor should be used.

Since it's a public class we can't remove that constructor and that's why
we have that QDummyPagedPaintDevicePrivate.

QPageLayout &QPagedPaintDevice::devicePageLayout() is also deprecated now
since there's no "device" page layout anymore. Those functions were
marked internal and as far as I can see unused outside
QPdfWriter/QPrinter so it should be fine.

[ChangeLog][QtGui] QPagedPaintDevice constructor has been deprecated
since that class is not meant to be used standalone, its two public
but internal devicePageLayout() methods are now deprecated.

Change-Id: I054601b66afcb7dd662db6247c5ed7820fbee212
Reviewed-by: Frederik Gladhorn <frederik.gladhorn@qt.io>

src/gui/painting/qpagedpaintdevice.cpp

@@ -42,6 +42,41 @@
 
 QT_BEGIN_NAMESPACE
 
+class QDummyPagedPaintDevicePrivate : public QPagedPaintDevicePrivate
+{
+    bool setPageLayout(const QPageLayout &newPageLayout) override
+    {
+        m_pageLayout = newPageLayout;
+        return m_pageLayout.isEquivalentTo(newPageLayout);
+    }
+
+    bool setPageSize(const QPageSize &pageSize) override
+    {
+        m_pageLayout.setPageSize(pageSize);
+        return m_pageLayout.pageSize().isEquivalentTo(pageSize);
+    }
+
+    bool setPageOrientation(QPageLayout::Orientation orientation) override
+    {
+        m_pageLayout.setOrientation(orientation);
+        return m_pageLayout.orientation() == orientation;
+    }
+
+    bool setPageMargins(const QMarginsF &margins, QPageLayout::Unit units) override
+    {
+        m_pageLayout.setUnits(units);
+        m_pageLayout.setMargins(margins);
+        return m_pageLayout.margins() == margins && m_pageLayout.units() == units;
+    }
+
+    QPageLayout pageLayout() const override
+    {
+        return m_pageLayout;
+    }
+
+    QPageLayout m_pageLayout;
+};
+
 QPagedPaintDevicePrivate::~QPagedPaintDevicePrivate()
 {
 }
@@ -61,9 +96,11 @@ QPagedPaintDevicePrivate::~QPagedPaintDevicePrivate()
 
 /*!
   Constructs a new paged paint device.
+
+  \deprecated
   */
 QPagedPaintDevice::QPagedPaintDevice()
-    : d(new QPagedPaintDevicePrivate)
+    : d(new QDummyPagedPaintDevicePrivate)
 {
 }
 
@@ -263,7 +300,7 @@ QPagedPaintDevicePrivate *QPagedPaintDevice::dd()
   */
 void QPagedPaintDevice::setPageSize(PageSize size)
 {
-    d->m_pageLayout.setPageSize(QPageSize(QPageSize::PageSizeId(size)));
+    d->setPageSize(QPageSize(QPageSize::PageSizeId(size)));
 }
 
 /*!
@@ -271,7 +308,7 @@ void QPagedPaintDevice::setPageSize(PageSize size)
   */
 QPagedPaintDevice::PageSize QPagedPaintDevice::pageSize() const
 {
-    return PageSize(d->m_pageLayout.pageSize().id());
+    return PageSize(d->pageLayout().pageSize().id());
 }
 
 /*!
@@ -282,7 +319,7 @@ QPagedPaintDevice::PageSize QPagedPaintDevice::pageSize() const
 */
 void QPagedPaintDevice::setPageSizeMM(const QSizeF &size)
 {
-    d->m_pageLayout.setPageSize(QPageSize(size, QPageSize::Millimeter));
+    d->setPageSize(QPageSize(size, QPageSize::Millimeter));
 }
 
 /*!
@@ -290,7 +327,7 @@ void QPagedPaintDevice::setPageSizeMM(const QSizeF &size)
   */
 QSizeF QPagedPaintDevice::pageSizeMM() const
 {
-    return d->m_pageLayout.pageSize().size(QPageSize::Millimeter);
+    return d->pageLayout().pageSize().size(QPageSize::Millimeter);
 }
 
 /*!
@@ -305,8 +342,7 @@ QSizeF QPagedPaintDevice::pageSizeMM() const
   */
 void QPagedPaintDevice::setMargins(const Margins &margins)
 {
-    d->m_pageLayout.setUnits(QPageLayout::Millimeter);
+    d->setPageMargins(QMarginsF(margins.left, margins.top, margins.right, margins.bottom), QPageLayout::Millimeter);
-    d->m_pageLayout.setMargins(QMarginsF(margins.left, margins.top, margins.right, margins.bottom));
 }
 
 /*!
@@ -318,7 +354,7 @@ void QPagedPaintDevice::setMargins(const Margins &margins)
   */
 QPagedPaintDevice::Margins QPagedPaintDevice::margins() const
 {
-    QMarginsF margins = d->m_pageLayout.margins(QPageLayout::Millimeter);
+    QMarginsF margins = d->pageLayout().margins(QPageLayout::Millimeter);
     Margins result;
     result.left = margins.left();
     result.top = margins.top();
@@ -413,7 +449,7 @@ bool QPagedPaintDevice::setPageOrientation(QPageLayout::Orientation orientation)
 
 bool QPagedPaintDevice::setPageMargins(const QMarginsF &margins)
 {
-    return d->setPageMargins(margins);
+    return setPageMargins(margins, pageLayout().units());
 }
 
 /*!
@@ -458,23 +494,30 @@ QPageLayout QPagedPaintDevice::pageLayout() const
 /*!
     \internal
 
+    \deprecated
+
     Returns the internal device page layout.
 */
 
 QPageLayout QPagedPaintDevice::devicePageLayout() const
 {
-    return d->m_pageLayout;
+    qWarning("QPagedPaintDevice::devicePageLayout() is deprecated, just use QPagedPaintDevice::pageLayout()");
+    return d->pageLayout();
 }
 
 /*!
     \internal
 
+    \deprecated
+
     Returns the internal device page layout.
 */
 
 QPageLayout &QPagedPaintDevice::devicePageLayout()
 {
-    return d->m_pageLayout;
+    qWarning("QPagedPaintDevice::devicePageLayout() is deprecated, you shouldn't be using this at all.");
+    static QPageLayout dummy;
+    return dummy;
 }
 
 QT_END_NAMESPACE

src/gui/painting/qpagedpaintdevice.h

@@ -55,7 +55,7 @@ class QPagedPaintDevicePrivate;
 class Q_GUI_EXPORT QPagedPaintDevice : public QPaintDevice
 {
 public:
-    QPagedPaintDevice();
+    QT_DEPRECATED QPagedPaintDevice();
     ~QPagedPaintDevice();
 
     virtual bool newPage() = 0;
@@ -243,8 +243,8 @@ public:
 protected:
     QPagedPaintDevice(QPagedPaintDevicePrivate *dd);
     QPagedPaintDevicePrivate *dd();
-    QPageLayout devicePageLayout() const;
+    QT_DEPRECATED QPageLayout devicePageLayout() const;
-    QPageLayout &devicePageLayout();
+    QT_DEPRECATED QPageLayout &devicePageLayout();
     friend class QPagedPaintDevicePrivate;
     QPagedPaintDevicePrivate *d;
 };

src/gui/painting/qpagedpaintdevice_p.h

@@ -60,8 +60,7 @@ class Q_GUI_EXPORT QPagedPaintDevicePrivate
 {
 public:
     QPagedPaintDevicePrivate()
-        : m_pageLayout(QPageSize(QPageSize::A4), QPageLayout::Portrait, QMarginsF(0, 0, 0, 0)),
+        : fromPage(0),
-          fromPage(0),
           toPage(0),
           pageOrderAscending(true),
           printSelectionOnly(false)
@@ -70,46 +69,19 @@ public:
 
     virtual ~QPagedPaintDevicePrivate();
 
-    // ### Qt6 Remove these and make public class methods virtual
-    virtual bool setPageLayout(const QPageLayout &newPageLayout)
-    {
-        m_pageLayout = newPageLayout;
-        return m_pageLayout.isEquivalentTo(newPageLayout);;
-    }
 
-    virtual bool setPageSize(const QPageSize &pageSize)
+    virtual bool setPageLayout(const QPageLayout &newPageLayout) = 0;
-    {
-        m_pageLayout.setPageSize(pageSize);
-        return m_pageLayout.pageSize().isEquivalentTo(pageSize);
-    }
 
-    virtual bool setPageOrientation(QPageLayout::Orientation orientation)
+    virtual bool setPageSize(const QPageSize &pageSize) = 0;
-    {
-        m_pageLayout.setOrientation(orientation);
-        return m_pageLayout.orientation() == orientation;
-    }
 
-    virtual bool setPageMargins(const QMarginsF &margins)
+    virtual bool setPageOrientation(QPageLayout::Orientation orientation) = 0;
-    {
-        return setPageMargins(margins, m_pageLayout.units());
-    }
 
-    virtual bool setPageMargins(const QMarginsF &margins, QPageLayout::Unit units)
+    virtual bool setPageMargins(const QMarginsF &margins, QPageLayout::Unit units) = 0;
-    {
-        m_pageLayout.setUnits(units);
-        m_pageLayout.setMargins(margins);
-        return m_pageLayout.margins() == margins && m_pageLayout.units() == units;
-    }
 
-    virtual QPageLayout pageLayout() const
+    virtual QPageLayout pageLayout() const = 0;
-    {
-        return m_pageLayout;
-    }
 
     static inline QPagedPaintDevicePrivate *get(QPagedPaintDevice *pd) { return pd->d; }
 
-    QPageLayout m_pageLayout;
-
     // These are currently required to keep QPrinter functionality working in QTextDocument::print()
     int fromPage;
     int toPage;

src/gui/painting/qpdfwriter.cpp

@@ -83,41 +83,28 @@ public:
     {
         // Try to set the paint engine page layout
         pd->engine->setPageLayout(newPageLayout);
-        // Set QPagedPaintDevice layout to match the current paint engine layout
+        return pageLayout().isEquivalentTo(newPageLayout);
-        m_pageLayout = pd->engine->pageLayout();
-        return m_pageLayout.isEquivalentTo(newPageLayout);
     }
 
     bool setPageSize(const QPageSize &pageSize) override
     {
         // Try to set the paint engine page size
         pd->engine->setPageSize(pageSize);
-        // Set QPagedPaintDevice layout to match the current paint engine layout
+        return pageLayout().pageSize().isEquivalentTo(pageSize);
-        m_pageLayout = pd->engine->pageLayout();
-        return m_pageLayout.pageSize().isEquivalentTo(pageSize);
     }
 
     bool setPageOrientation(QPageLayout::Orientation orientation) override
     {
         // Set the print engine value
         pd->engine->setPageOrientation(orientation);
-        // Set QPagedPaintDevice layout to match the current paint engine layout
+        return pageLayout().orientation() == orientation;
-        m_pageLayout = pd->engine->pageLayout();
-        return m_pageLayout.orientation() == orientation;
-    }
-
-    bool setPageMargins(const QMarginsF &margins) override
-    {
-        return setPageMargins(margins, pageLayout().units());
     }
 
     bool setPageMargins(const QMarginsF &margins, QPageLayout::Unit units) override
     {
         // Try to set engine margins
         pd->engine->setPageMargins(margins, units);
-        // Set QPagedPaintDevice layout to match the current paint engine layout
+        return pageLayout().margins() == margins && pageLayout().units() == units;
-        m_pageLayout = pd->engine->pageLayout();
-        return m_pageLayout.margins() == margins && m_pageLayout.units() == units;
     }
 
     QPageLayout pageLayout() const override
@@ -150,9 +137,6 @@ QPdfWriter::QPdfWriter(const QString &filename)
     Q_D(QPdfWriter);
 
     d->engine->setOutputFilename(filename);
-
-    // Set QPagedPaintDevice layout to match the current paint engine layout
-    devicePageLayout() = d->engine->pageLayout();
 }
 
 /*!
@@ -165,9 +149,6 @@ QPdfWriter::QPdfWriter(QIODevice *device)
     Q_D(QPdfWriter);
 
     d->engine->d_func()->outDevice = device;
-
-    // Set QPagedPaintDevice layout to match the current paint engine layout
-    devicePageLayout() = d->engine->pageLayout();
 }
 
 /*!

src/printsupport/kernel/qprinter.cpp

@@ -224,8 +224,8 @@ void QPrinterPrivate::setProperty(QPrintEngine::PrintEnginePropertyKey key, cons
 class QPrinterPagedPaintDevicePrivate : public QPagedPaintDevicePrivate
 {
 public:
-    QPrinterPagedPaintDevicePrivate(QPrinterPrivate *d)
+    QPrinterPagedPaintDevicePrivate(QPrinter *p)
-        : QPagedPaintDevicePrivate(), pd(d)
+        : QPagedPaintDevicePrivate(), m_printer(p)
     {}
 
     virtual ~QPrinterPagedPaintDevicePrivate()
@@ -233,6 +233,8 @@ public:
 
     bool setPageLayout(const QPageLayout &newPageLayout) override
     {
+        QPrinterPrivate *pd = QPrinterPrivate::get(m_printer);
+
         if (pd->paintEngine->type() != QPaintEngine::Pdf
             && pd->printEngine->printerState() == QPrinter::Active) {
             qWarning("QPrinter::setPageLayout: Cannot be changed while printer is active");
@@ -242,14 +244,13 @@ public:
         // Try to set the print engine page layout
         pd->setProperty(QPrintEngine::PPK_QPageLayout, QVariant::fromValue(newPageLayout));
 
-        // Set QPagedPaintDevice layout to match the current print engine value
-        m_pageLayout = pageLayout();
-
         return pageLayout().isEquivalentTo(newPageLayout);
     }
 
     bool setPageSize(const QPageSize &pageSize) override
     {
+        QPrinterPrivate *pd = QPrinterPrivate::get(m_printer);
+
         if (pd->paintEngine->type() != QPaintEngine::Pdf
             && pd->printEngine->printerState() == QPrinter::Active) {
             qWarning("QPrinter::setPageLayout: Cannot be changed while printer is active");
@@ -260,46 +261,38 @@ public:
         // Try to set the print engine page size
         pd->setProperty(QPrintEngine::PPK_QPageSize, QVariant::fromValue(pageSize));
 
-        // Set QPagedPaintDevice layout to match the current print engine value
-        m_pageLayout = pageLayout();
-
         return pageLayout().pageSize().isEquivalentTo(pageSize);
     }
 
     bool setPageOrientation(QPageLayout::Orientation orientation) override
     {
+        QPrinterPrivate *pd = QPrinterPrivate::get(m_printer);
+
         // Set the print engine value
         pd->setProperty(QPrintEngine::PPK_Orientation, orientation);
 
-        // Set QPagedPaintDevice layout to match the current print engine value
-        m_pageLayout = pageLayout();
-
         return pageLayout().orientation() == orientation;
     }
 
-    bool setPageMargins(const QMarginsF &margins) override
-    {
-        return setPageMargins(margins, pageLayout().units());
-    }
-
     bool setPageMargins(const QMarginsF &margins, QPageLayout::Unit units) override
     {
+        QPrinterPrivate *pd = QPrinterPrivate::get(m_printer);
+
         // Try to set print engine margins
         QPair<QMarginsF, QPageLayout::Unit> pair = qMakePair(margins, units);
         pd->setProperty(QPrintEngine::PPK_QPageMargins, QVariant::fromValue(pair));
 
-        // Set QPagedPaintDevice layout to match the current print engine value
-        m_pageLayout = pageLayout();
-
         return pageLayout().margins() == margins && pageLayout().units() == units;
     }
 
     QPageLayout pageLayout() const override
     {
+        QPrinterPrivate *pd = QPrinterPrivate::get(m_printer);
+
         return pd->printEngine->property(QPrintEngine::PPK_QPageLayout).value<QPageLayout>();
     }
 
-    QPrinterPrivate *pd;
+    QPrinter *m_printer;
 };
 
 
@@ -554,11 +547,9 @@ public:
     Creates a new printer object with the given \a mode.
 */
 QPrinter::QPrinter(PrinterMode mode)
-    : QPagedPaintDevice(),
+    : QPagedPaintDevice(new QPrinterPagedPaintDevicePrivate(this)),
       d_ptr(new QPrinterPrivate(this))
 {
-    delete d;
-    d = new QPrinterPagedPaintDevicePrivate(d_func());
     d_ptr->init(QPrinterInfo(), mode);
 }
 
@@ -568,11 +559,9 @@ QPrinter::QPrinter(PrinterMode mode)
     Creates a new printer object with the given \a printer and \a mode.
 */
 QPrinter::QPrinter(const QPrinterInfo& printer, PrinterMode mode)
-    : QPagedPaintDevice(),
+    : QPagedPaintDevice(new QPrinterPagedPaintDevicePrivate(this)),
       d_ptr(new QPrinterPrivate(this))
 {
-    delete d;
-    d = new QPrinterPagedPaintDevicePrivate(d_func());
     d_ptr->init(printer, mode);
 }
 
@@ -1469,8 +1458,6 @@ void QPrinter::setFullPage(bool fp)
     Q_D(QPrinter);
     // Set the print engine
     d->setProperty(QPrintEngine::PPK_FullPage, fp);
-    // Set QPagedPaintDevice layout to match the current print engine value
-    devicePageLayout() = pageLayout();
 }
 
 

src/printsupport/kernel/qprinter_p.h

@@ -94,6 +94,10 @@ public:
 
     }
 
+    static QPrinterPrivate *get(QPrinter *printer) {
+        return printer->d_ptr.get();
+    }
+
     void init(const QPrinterInfo &printer, QPrinter::PrinterMode mode);
 
     QPrinterInfo findValidPrinter(const QPrinterInfo &printer = QPrinterInfo());