Skip to content

Commit

Permalink
QPainterStateGuard: make movable, add test coverage
Browse files Browse the repository at this point in the history
There's no real reason not to be able to move a state guard.

Since a moved-from QPainterStateGuard must not have a restore-
level count, we have to implement the move-constructor explicily.
Add a test case, which verifies that we don't end up with an
un-balanced save/restore count, and that verifies that we would
trigger the Q_ASSERT if we do.

Address comment from header review,
amends 9ecf47a.

Task-number: QTBUG-132090
Pick-to: 6.9
Change-Id: I1db135bf48c0fa0a7bac4fdae7b7263c356b5eb6
Reviewed-by: Juha Vuolle <juha.vuolle@qt.io>
  • Loading branch information
vohi authored and Juha Vuolle committed Dec 19, 2024
1 parent 8928b0f commit ec3a5f4
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 1 deletion.
19 changes: 19 additions & 0 deletions src/gui/painting/qpainterstateguard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,25 @@ QT_BEGIN_NAMESPACE
QPainter's state.
*/

/*!
\fn QPainterStateGuard::QPainterStateGuard(QPainterStateGuard &&other)
Move-constructs a painter state guard from \a other.
*/

/*!
\fn QPainterStateGuard &QPainterStateGuard::operator=(QPainterStateGuard &&other)
Move-assigns \a other to this painter state guard.
*/

/*!
\fn void QPainterStateGuard::swap(QPainterStateGuard &other)
Swaps the \a other with this painter state guard. This operation is very
fast and never fails.
*/

/*!
\fn QPainterStateGuard::~QPainterStateGuard()
Destroys the QPainterStateGuard instance and calls restore() as often as save()
Expand Down
13 changes: 12 additions & 1 deletion src/gui/painting/qpainterstateguard.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,24 @@ QT_BEGIN_NAMESPACE

class QPainterStateGuard
{
Q_DISABLE_COPY_MOVE(QPainterStateGuard)
Q_DISABLE_COPY(QPainterStateGuard)
public:
enum class InitialState : quint8 {
Save,
NoSave,
};

QPainterStateGuard(QPainterStateGuard &&other) noexcept
: m_painter(std::exchange(other.m_painter, nullptr))
, m_level(std::exchange(other.m_level, 0))
{}
QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QPainterStateGuard)
void swap(QPainterStateGuard &other) noexcept
{
qt_ptr_swap(m_painter, other.m_painter);
std::swap(m_level, other.m_level);
}

Q_NODISCARD_CTOR
explicit QPainterStateGuard(QPainter *painter, InitialState state = InitialState::Save)
: m_painter(painter)
Expand Down
1 change: 1 addition & 0 deletions tests/auto/gui/painting/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ add_subdirectory(qpagelayout)
add_subdirectory(qpageranges)
add_subdirectory(qpagesize)
add_subdirectory(qpainter)
add_subdirectory(qpainterstateguard)
if (QT_FEATURE_pdf)
add_subdirectory(qpdfwriter)
endif()
Expand Down
19 changes: 19 additions & 0 deletions tests/auto/gui/painting/qpainterstateguard/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright (C) 2024 The Qt Company Ltd.
# SPDX-License-Identifier: BSD-3-Clause

#####################################################################
## tst_qpainterstateguard Test:
#####################################################################

if(NOT QT_BUILD_STANDALONE_TESTS AND NOT QT_BUILDING_QT)
cmake_minimum_required(VERSION 3.16)
project(tst_qpainterstateguard LANGUAGES CXX)
find_package(Qt6BuildInternals REQUIRED COMPONENTS STANDALONE_TEST)
endif()

qt_internal_add_test(tst_qpainterstateguard
SOURCES
tst_qpainterstateguard.cpp
LIBRARIES
Qt::Gui
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright (C) 2024 The Qt Company Ltd.
// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only

#define Q_ASSERT(cond) ((cond) ? static_cast<void>(0) : qCritical(#cond))

#include <QTest>

#include <QtGui/qpixmap.h>
#include <QtGui/qpainter.h>
#include <QtGui/qpainterstateguard.h>

class tst_QPainterStateGuard : public QObject
{
Q_OBJECT

private slots:
void basics_data();
void basics();
};

void tst_QPainterStateGuard::basics_data()
{
QTest::addColumn<QPainterStateGuard::InitialState>("initialState");
QTest::addColumn<bool>("expectWarning");

QTest::addRow("Save") << QPainterStateGuard::InitialState::Save << false;
QTest::addRow("NoSave") << QPainterStateGuard::InitialState::NoSave << true;
}

void tst_QPainterStateGuard::basics()
{
QFETCH(const QPainterStateGuard::InitialState, initialState);
QFETCH(const bool, expectWarning);

if (expectWarning) {
QTest::ignoreMessage(QtCriticalMsg, "m_level > 0");
QTest::ignoreMessage(QtWarningMsg, "QPainter::restore: Unbalanced save/restore");
} else {
QTest::failOnWarning("QPainter::restore: Unbalanced save/restore");
}
QPixmap pixmap(100, 100);
QPainter painter(&pixmap);
const QPen defaultPen = painter.pen();

{
auto makeGuard = [initialState](QPainter *p) -> QPainterStateGuard {
return QPainterStateGuard(p, initialState);
};
QPainterStateGuard guard = makeGuard(&painter);

painter.setPen(Qt::red);
QCOMPARE(painter.pen().color(), Qt::red);

QPainterStateGuard guard2 = std::move(guard);

QCOMPARE(painter.pen().color(), Qt::red);

guard2.restore();
}

if (expectWarning)
QCOMPARE_NE(painter.pen(), defaultPen);
else
QCOMPARE(painter.pen(), defaultPen);
}

QTEST_MAIN(tst_QPainterStateGuard)

#include "tst_qpainterstateguard.moc"

0 comments on commit ec3a5f4

Please sign in to comment.