QAbstractAnimation::setCurrentTime() - fix bindable property usage
The setter is complicated, because it reads the values of four properties (loopCount, totalCurrentTime, currentLoop, and direction) and modifies the values of two properties (totalCurrentTime and currentLoop). For the read-only properties, inroduce local variables, so that the binding evaluation is done only once, and re-use the local variables afterwards. For the read-write properties, use {set}ValueBypassingBindings() methods, because we are anyway explicitly calling notify() afterwards. Intorduce a newCurrentLoop local variable to calculate the new value of currentLoop, and update the actual property only once. Use the newCurrentLoop and msecs variables instead of accessing the actual properties when determining if we need to notify about property changes. The other complexity is that the setter calls a virtual updateCurrentTime() method in the middle of the calculation. Subclasses can override the default implementation and modify the properties there (at least QSequentialAnimationGroup updates totalCurrentTime property under certain conditions). Keep it in mind and re-read the property values after the call to the virtual method. As a drive-by, make some pre-existing variables const. Task-number: QTBUG-116346 Pick-to: 6.6 6.5 Change-Id: I0a48bcabde15333e80ea2d54e308cb9438b310e7 Reviewed-by: Ulf Hermann <ulf.hermann@qt.io> Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
This commit is contained in:
parent
516d828f24
commit
bfc2d69c61
@ -1301,46 +1301,59 @@ void QAbstractAnimation::setCurrentTime(int msecs)
|
||||
msecs = qMax(msecs, 0);
|
||||
|
||||
// Calculate new time and loop.
|
||||
int dura = duration();
|
||||
int totalDura = dura <= 0 ? dura : ((d->loopCount < 0) ? -1 : dura * d->loopCount);
|
||||
const int dura = duration();
|
||||
const int totalLoopCount = d->loopCount;
|
||||
const int totalDura = dura <= 0 ? dura : ((totalLoopCount < 0) ? -1 : dura * totalLoopCount);
|
||||
if (totalDura != -1)
|
||||
msecs = qMin(totalDura, msecs);
|
||||
|
||||
const int oldCurrentTime = d->totalCurrentTime;
|
||||
d->totalCurrentTime = msecs;
|
||||
d->totalCurrentTime.removeBindingUnlessInWrapper();
|
||||
|
||||
const int oldCurrentTime = d->totalCurrentTime.valueBypassingBindings();
|
||||
d->totalCurrentTime.setValueBypassingBindings(msecs);
|
||||
|
||||
QAbstractAnimation::Direction currentDirection = d->direction;
|
||||
|
||||
// Update new values.
|
||||
int oldLoop = d->currentLoop;
|
||||
d->currentLoop = ((dura <= 0) ? 0 : (msecs / dura));
|
||||
if (d->currentLoop == d->loopCount) {
|
||||
const int oldLoop = d->currentLoop.valueBypassingBindings();
|
||||
int newCurrentLoop = (dura <= 0) ? 0 : (msecs / dura);
|
||||
if (newCurrentLoop == totalLoopCount) {
|
||||
//we're at the end
|
||||
d->currentTime = qMax(0, dura);
|
||||
d->currentLoop = qMax(0, d->loopCount - 1);
|
||||
newCurrentLoop = qMax(0, totalLoopCount - 1);
|
||||
} else {
|
||||
if (d->direction == Forward) {
|
||||
if (currentDirection == Forward) {
|
||||
d->currentTime = (dura <= 0) ? msecs : (msecs % dura);
|
||||
} else {
|
||||
d->currentTime = (dura <= 0) ? msecs : ((msecs - 1) % dura) + 1;
|
||||
if (d->currentTime == dura)
|
||||
d->currentLoop = d->currentLoop - 1;
|
||||
newCurrentLoop = newCurrentLoop - 1;
|
||||
}
|
||||
}
|
||||
d->currentLoop.setValueBypassingBindings(newCurrentLoop);
|
||||
|
||||
// this is a virtual function, so it can update the properties as well
|
||||
updateCurrentTime(d->currentTime);
|
||||
if (d->currentLoop != oldLoop)
|
||||
|
||||
// read the property values again
|
||||
newCurrentLoop = d->currentLoop.valueBypassingBindings();
|
||||
currentDirection = d->direction;
|
||||
const int newTotalCurrentTime = d->totalCurrentTime.valueBypassingBindings();
|
||||
|
||||
if (newCurrentLoop != oldLoop)
|
||||
d->currentLoop.notify();
|
||||
|
||||
/* Notify before calling stop: As seen in tst_QSequentialAnimationGroup::clear
|
||||
* we might delete the animation when stop is called. Thus after stop no member
|
||||
* of the object must be used anymore.
|
||||
*/
|
||||
if (oldCurrentTime != d->totalCurrentTime)
|
||||
if (oldCurrentTime != newTotalCurrentTime)
|
||||
d->totalCurrentTime.notify();
|
||||
// All animations are responsible for stopping the animation when their
|
||||
// own end state is reached; in this case the animation is time driven,
|
||||
// and has reached the end.
|
||||
if ((d->direction == Forward && d->totalCurrentTime == totalDura)
|
||||
|| (d->direction == Backward && d->totalCurrentTime == 0)) {
|
||||
if ((currentDirection == Forward && newTotalCurrentTime == totalDura)
|
||||
|| (currentDirection == Backward && newTotalCurrentTime == 0)) {
|
||||
stop();
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user