From 85d27945ac3f4a82e659f8df294b75b6f302f2db Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Mon, 1 Jun 2020 11:54:45 +0200 Subject: QPainter: replace manual memory management [2/5]: state/states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Allan Sandfeld Jensen --- src/gui/painting/qpainter.cpp | 44 +++++++++++++++++++++---------------------- src/gui/painting/qpainter_p.h | 10 ++++++++-- 2 files changed, 29 insertions(+), 25 deletions(-) (limited to 'src/gui') diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp index 01400a0bc8..eca80f55f0 100644 --- a/src/gui/painting/qpainter.cpp +++ b/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 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 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(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()) { diff --git a/src/gui/painting/qpainter_p.h b/src/gui/painting/qpainter_p.h index 71e90c9717..8994bcd3e7 100644 --- a/src/gui/painting/qpainter_p.h +++ b/src/gui/painting/qpainter_p.h @@ -66,6 +66,7 @@ #include #include +#include QT_BEGIN_NAMESPACE @@ -205,8 +206,12 @@ public: QPainter *q_ptr; QPainterPrivate **d_ptrs; - QPainterState *state; - QVarLengthArray states; + std::unique_ptr state; + template + struct SmallStack : std::stack> { + void clear() { this->c.clear(); } + }; + SmallStack> savedStates; mutable std::unique_ptr dummyState; @@ -230,6 +235,7 @@ public: void updateEmulationSpecifier(QPainterState *s); void updateStateImpl(QPainterState *state); void updateState(QPainterState *state); + void updateState(std::unique_ptr &state) { updateState(state.get()); } void draw_helper(const QPainterPath &path, DrawOperation operation = StrokeAndFillDraw); void drawStretchedGradient(const QPainterPath &path, DrawOperation operation); -- cgit v1.2.3