QPainter: replace manual memory management [2/5]: state/states

Use unique_ptr to indicate ownership. Use std::stack to enforce
stack-ness (QStack doesn't). Requires the addition of a clear()
function, for which we spin a SmallStack wrapper. It also helps to not
have to repeat the value_type...

Because we use unique_ptr, `state` and `states` are now exclusive
(before, `state` was always `states.back()`). Correspondingly, rename
`states` → `savedStates`.

As a drive-by, this fixes an off-by-one error in a qWarning() (checked
`> 1`, but reported one saved state too many).

Change-Id: I8faade59300401be802ddc52c64ce31b8f23dc52
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

src/gui/painting/qpainter.cpp

@@ -210,7 +210,7 @@ void QPainterPrivate::checkEmulation()
             if (!emulationEngine)
                 emulationEngine = new QEmulationPaintEngine(extended);
             extended = emulationEngine;
-            extended->setState(state);
+            extended->setState(state.get());
         }
     } else if (emulationEngine == extended) {
         extended = emulationEngine->real_engine;
@@ -221,7 +221,6 @@ void QPainterPrivate::checkEmulation()
 QPainterPrivate::~QPainterPrivate()
 {
     delete emulationEngine;
-    qDeleteAll(states);
 }
 
 
@@ -1588,15 +1587,19 @@ void QPainter::save()
         return;
     }
 
+    std::unique_ptr<QPainterState> prev;
     if (d->extended) {
-        d->state = d->extended->createState(d->states.back());
-        d->extended->setState(d->state);
+        // separate the creation of a new state from the update of d->state, since some
+        // engines access d->state directly (not via createState()'s argument)
+        std::unique_ptr<QPainterState> next(d->extended->createState(d->state.get()));
+        prev = std::exchange(d->state, std::move(next));
+        d->extended->setState(d->state.get());
     } else {
         d->updateState(d->state);
-        d->state = new QPainterState(d->states.back());
-        d->engine->state = d->state;
+        prev = std::exchange(d->state, std::make_unique<QPainterState>(d->state.get()));
+        d->engine->state = d->state.get();
     }
-    d->states.push_back(d->state);
+    d->savedStates.push(std::move(prev));
 }
 
 /*!
@@ -1613,7 +1616,7 @@ void QPainter::restore()
         printf("QPainter::restore()\n");
 #endif
     Q_D(QPainter);
-    if (d->states.size()<=1) {
+    if (d->savedStates.empty()) {
         qWarning("QPainter::restore: Unbalanced save/restore");
         return;
     } else if (!d->engine) {
@@ -1621,15 +1624,13 @@ void QPainter::restore()
         return;
     }
 
-    QPainterState *tmp = d->state;
-    d->states.pop_back();
-    d->state = d->states.back();
+    const auto tmp = std::exchange(d->state, std::move(d->savedStates.top()));
+    d->savedStates.pop();
     d->txinv = false;
 
     if (d->extended) {
         d->checkEmulation();
-        d->extended->setState(d->state);
-        delete tmp;
+        d->extended->setState(d->state.get());
         return;
     }
 
@@ -1667,8 +1668,7 @@ void QPainter::restore()
         tmp->changeFlags |= QPaintEngine::DirtyTransform;
     }
 
-    d->updateState(d->state);
-    delete tmp;
+    d->updateState(d->state.get());
 }
 
 
@@ -1701,8 +1701,7 @@ void QPainter::restore()
 
 static inline void qt_cleanup_painter_state(QPainterPrivate *d)
 {
-    qDeleteAll(d->states);
-    d->states.clear();
+    d->savedStates.clear();
     d->state = nullptr;
     d->engine = nullptr;
     d->device = nullptr;
@@ -1760,18 +1759,17 @@ bool QPainter::begin(QPaintDevice *pd)
 
     // Setup new state...
     Q_ASSERT(!d->state);
-    d->state = d->extended ? d->extended->createState(nullptr) : new QPainterState;
+    d->state.reset(d->extended ? d->extended->createState(nullptr) : new QPainterState);
     d->state->painter = this;
-    d->states.push_back(d->state);
 
     d->state->redirectionMatrix.translate(-redirectionOffset.x(), -redirectionOffset.y());
     d->state->brushOrigin = QPointF();
 
     // Slip a painter state into the engine before we do any other operations
     if (d->extended)
-        d->extended->setState(d->state);
+        d->extended->setState(d->state.get());
     else
-        d->engine->state = d->state;
+        d->engine->state = d->state.get();
 
     switch (pd->devType()) {
         case QInternal::Pixmap:
@@ -1909,8 +1907,8 @@ bool QPainter::end()
         }
     }
 
-    if (d->states.size() > 1) {
-        qWarning("QPainter::end: Painter ended with %d saved states", int(d->states.size()));
+    if (d->savedStates.size() > 0) {
+        qWarning("QPainter::end: Painter ended with %d saved states", int(d->savedStates.size()));
     }
 
     if (d->engine->autoDestruct()) {

src/gui/painting/qpainter_p.h

@@ -66,6 +66,7 @@
 #include <private/qpen_p.h>
 
 #include <memory>
+#include <stack>
 
 QT_BEGIN_NAMESPACE
 
@@ -205,8 +206,12 @@ public:
     QPainter *q_ptr;
     QPainterPrivate **d_ptrs;
 
-    QPainterState *state;
-    QVarLengthArray<QPainterState *, 8> states;
+    std::unique_ptr<QPainterState> state;
+    template <typename T, std::size_t N = 8>
+    struct SmallStack : std::stack<T, QVarLengthArray<T, N>> {
+        void clear() { this->c.clear(); }
+    };
+    SmallStack<std::unique_ptr<QPainterState>> savedStates;
 
     mutable std::unique_ptr<QPainterDummyState> dummyState;
 
@@ -230,6 +235,7 @@ public:
     void updateEmulationSpecifier(QPainterState *s);
     void updateStateImpl(QPainterState *state);
     void updateState(QPainterState *state);
+    void updateState(std::unique_ptr<QPainterState> &state) { updateState(state.get()); }
 
     void draw_helper(const QPainterPath &path, DrawOperation operation = StrokeAndFillDraw);
     void drawStretchedGradient(const QPainterPath &path, DrawOperation operation);