Files
qelectrotech-source-mirror/sources/QPropertyUndoCommand/qpropertyundocommand.cpp
T
ispyisail ae6b992ae4 Give QPropertyUndoCommand::undo() its own first-time grace period
redo() has a direct-write path guarded by m_first_time: on the very
first call it writes the property immediately, and only animates on
calls after that (per setAnimated()'s documented contract). undo()
had no equivalent -- it always animated, so undo() both returned
before the property was restored (stale state visible to anything
sharing the call stack) and, with no running event loop, never
restored it at all.

The obvious fix -- reuse m_first_time in undo()'s guard too -- turns
out not to work, and I verified this with a standalone build before
picking an approach: QUndoStack::push() always calls redo() once
before any undo() can run, and redo()'s direct-write branch sets
m_first_time = true as it completes. So by the time undo() is ever
called, m_first_time has already flipped, and reusing it would make
undo() take the animate branch on every call, unconditionally --
syntactically symmetric with redo(), but behaviourally unchanged for
the exact scenario reported.

Instead, undo() gets its own m_undo_first_time flag, seeded from the
same first_time argument setAnimated() already takes, and set true by
undo()'s own direct-write branch the same way m_first_time is set by
redo()'s. That gives undo() a real, reachable direct-write path on its
own first call, independent of how many times redo() has already run.

Verified against a standalone build of just this class (as the issue's
own repro does): first redo and first undo are both now synchronous
with no event loop running; with an event loop present, both settle to
the correct value once "broken in"; behaviour for every other caller
of QPropertyUndoCommand -- everywhere that calls plain enableAnimation()
or the bare setAnimated() (first_time defaulting true) -- is provably
unchanged, since m_undo_first_time starts true either way and the
animate branch never modifies it.

Fixes #755.
2026-08-24 20:10:32 +12:00

184 lines
5.1 KiB
C++

/*
Copyright 2006-2026 The QElectroTech Team
This file is part of QElectroTech.
QElectroTech is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 2 of the License, or
(at your option) any later version.
QElectroTech is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with QElectroTech. If not, see <http://www.gnu.org/licenses/>.
*/
#include "qpropertyundocommand.h"
#include <QPropertyAnimation>
/**
@brief QPropertyUndoCommand::QPropertyUndoCommand
Default constructor with old and new value
This command don't take ownership of object
@param object
@param property_name
@param old_value
@param new_value
@param parent
*/
QPropertyUndoCommand::QPropertyUndoCommand(QObject *object,
const char *property_name,
const QVariant &old_value,
const QVariant &new_value,
QUndoCommand *parent) :
QUndoCommand(parent),
m_object(object),
m_property_name(property_name),
m_old_value(old_value),
m_new_value(new_value)
{}
/**
@brief QPropertyUndoCommand::QPropertyUndoCommand
Default constructor with old value.
Call setNewValue to setup the new value of the edited QObject
This command don't take ownership of object
@param object
@param property_name
@param old_value
@param parent
*/
QPropertyUndoCommand::QPropertyUndoCommand(QObject *object,
const char *property_name,
const QVariant &old_value,
QUndoCommand *parent) :
QUndoCommand(parent),
m_object(object),
m_property_name(property_name),
m_old_value(old_value)
{}
/**
@brief QPropertyUndoCommand::QPropertyUndoCommand
@param other
*/
QPropertyUndoCommand::QPropertyUndoCommand(const QPropertyUndoCommand *other)
{
m_object = other->m_object;
m_property_name = other->m_property_name;
m_old_value = other->m_old_value;
m_new_value = other->m_new_value;
m_animate = other->m_animate;
m_first_time = other->m_first_time;
m_undo_first_time = other->m_undo_first_time;
setText(other->text());
}
/**
@brief QPropertyUndoCommand::setNewValue
Set the new value of the property (set with redo) to new_value
@param new_value
*/
void QPropertyUndoCommand::setNewValue(const QVariant &new_value) {
m_new_value = new_value;
}
/**
@brief QPropertyUndoCommand::enableAnimation
True to enable animation
@param animate
*/
void QPropertyUndoCommand::enableAnimation (bool animate) {
m_animate = animate;
}
/**
@brief QPropertyUndoCommand::setAnimated
@param animate = true for animate this undo
@param first_time = if true,
the first animation is done at the first call of redo if false,
the first animation is done at the second call of redo.
The same rule applies to undo, tracked independently: redo() always
runs before the first undo() (QUndoStack::push() calls redo()
immediately), so by the time undo() can run at all, redo()'s own
m_first_time has already flipped true. Sharing that flag would leave
undo() with no instant path ever reachable in practice -- reusing it
is not actually symmetric, it just looks like it is. m_undo_first_time
gives undo() the same one-time grace period redo() has, on its own
first call instead of redo's.
*/
void QPropertyUndoCommand::setAnimated(bool animate, bool first_time)
{
m_animate = animate;
m_first_time = first_time;
m_undo_first_time = first_time;
}
/**
@brief QPropertyUndoCommand::mergeWith
Try to merge this command with other command
@param other
@return true if was merged, else false
*/
bool QPropertyUndoCommand::mergeWith(const QUndoCommand *other)
{
if (id() != other->id() || other->childCount()) return false;
QPropertyUndoCommand const *undo = static_cast<const QPropertyUndoCommand *>(other);
if (m_object != undo->m_object
|| m_property_name != undo->m_property_name) return false;
m_new_value = undo->m_new_value;
return true;
}
/**
@brief QPropertyUndoCommand::redo
Redo this command
*/
void QPropertyUndoCommand::redo()
{
if (m_object->property(m_property_name) != m_new_value)
{
if (m_animate && m_first_time)
{
QPropertyAnimation *animation = new QPropertyAnimation(m_object, m_property_name);
animation->setStartValue(m_old_value);
animation->setEndValue(m_new_value);
animation->start(QAbstractAnimation::DeleteWhenStopped);
}
else
{
m_object->setProperty(m_property_name, m_new_value);
m_first_time = true;
}
}
QUndoCommand::redo();
}
/**
@brief QPropertyUndoCommand::undo
Undo this command
*/
void QPropertyUndoCommand::undo()
{
if (m_object->property(m_property_name) != m_old_value)
{
if (m_animate && m_undo_first_time)
{
QPropertyAnimation *animation = new QPropertyAnimation(m_object, m_property_name);
animation->setStartValue(m_new_value);
animation->setEndValue(m_old_value);
animation->start(QAbstractAnimation::DeleteWhenStopped);
}
else
{
m_object->setProperty(m_property_name, m_old_value);
m_undo_first_time = true;
}
}
QUndoCommand::undo();
}