From 688d946ffb08892f46523cc532fb167ad9084fd6 Mon Sep 17 00:00:00 2001 From: Vlad Zagorodniy Date: Tue, 23 Oct 2018 20:21:27 +0300 Subject: [PATCH] [libkwineffects] Gracefully release previous window pixmap Summary: If the cross fade animation is cancelled, we are not gracefully unreference the previous window pixmap. This change addresses that issue by using RAII approach to reference/unreference the previous window pixmap. Test Plan: Manually. The Maximize and the Morphing Popups effect still work. Reviewers: #kwin, davidedmundson Reviewed By: #kwin, davidedmundson Subscribers: davidedmundson, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D16391 --- libkwineffects/anidata.cpp | 19 ++++++++++++++++- libkwineffects/anidata_p.h | 18 +++++++++++++++- libkwineffects/kwinanimationeffect.cpp | 29 +++++++++++++++++++------- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/libkwineffects/anidata.cpp b/libkwineffects/anidata.cpp index 5748826885..bfdb007765 100644 --- a/libkwineffects/anidata.cpp +++ b/libkwineffects/anidata.cpp @@ -52,6 +52,21 @@ KeepAliveLock::~KeepAliveLock() m_window->unrefWindow(); } +PreviousWindowPixmapLock::PreviousWindowPixmapLock(EffectWindow *w) + : m_window(w) +{ + m_window->referencePreviousWindowPixmap(); +} + +PreviousWindowPixmapLock::~PreviousWindowPixmapLock() +{ + m_window->unreferencePreviousWindowPixmap(); + + // Add synthetic repaint to prevent glitches after cross-fading + // translucent windows. + effects->addRepaint(m_window->expandedGeometry()); +} + AniData::AniData() : attribute(AnimationEffect::Opacity) , customCurve(0) // Linear @@ -67,7 +82,8 @@ AniData::AniData() AniData::AniData(AnimationEffect::Attribute a, int meta_, int ms, const FPx2 &to_, QEasingCurve curve_, int delay, const FPx2 &from_, bool waitAtSource_, bool keepAtTarget_, - FullScreenEffectLockPtr fullScreenEffectLock_, bool keepAlive) + FullScreenEffectLockPtr fullScreenEffectLock_, bool keepAlive, + PreviousWindowPixmapLockPtr previousWindowPixmapLock_) : attribute(a) , curve(curve_) , from(from_) @@ -80,6 +96,7 @@ AniData::AniData(AnimationEffect::Attribute a, int meta_, int ms, const FPx2 &to , waitAtSource(waitAtSource_) , keepAtTarget(keepAtTarget_) , keepAlive(keepAlive) + , previousWindowPixmapLock(previousWindowPixmapLock_) { } diff --git a/libkwineffects/anidata_p.h b/libkwineffects/anidata_p.h index 0031ceac95..3e14980cc2 100644 --- a/libkwineffects/anidata_p.h +++ b/libkwineffects/anidata_p.h @@ -56,13 +56,28 @@ private: }; typedef QSharedPointer KeepAliveLockPtr; +/** + * References the previous window pixmap to prevent discarding. + **/ +class PreviousWindowPixmapLock +{ +public: + PreviousWindowPixmapLock(EffectWindow *w); + ~PreviousWindowPixmapLock(); + +private: + EffectWindow *m_window; + Q_DISABLE_COPY(PreviousWindowPixmapLock) +}; +typedef QSharedPointer PreviousWindowPixmapLockPtr; + class KWINEFFECTS_EXPORT AniData { public: AniData(); AniData(AnimationEffect::Attribute a, int meta, int ms, const FPx2 &to, QEasingCurve curve, int delay, const FPx2 &from, bool waitAtSource, bool keepAtTarget = false, FullScreenEffectLockPtr=FullScreenEffectLockPtr(), - bool keepAlive = true); + bool keepAlive = true, PreviousWindowPixmapLockPtr previousWindowPixmapLock = {}); inline void addTime(int t) { time += t; } inline bool isOneDimensional() const { return from[0] == from[1] && to[0] == to[1]; @@ -81,6 +96,7 @@ public: bool waitAtSource, keepAtTarget; bool keepAlive; KeepAliveLockPtr keepAliveLock; + PreviousWindowPixmapLockPtr previousWindowPixmapLock; }; } // namespace diff --git a/libkwineffects/kwinanimationeffect.cpp b/libkwineffects/kwinanimationeffect.cpp index ba5c191715..e512172e21 100644 --- a/libkwineffects/kwinanimationeffect.cpp +++ b/libkwineffects/kwinanimationeffect.cpp @@ -220,8 +220,6 @@ quint64 AnimationEffect::p_animate( EffectWindow *w, Attribute a, uint meta, int { const bool waitAtSource = from.isValid(); validate(a, meta, &from, &to, w); - if (a == CrossFadePrevious) - w->referencePreviousWindowPixmap(); Q_D(AnimationEffect); if (!d->m_isInitialized) @@ -247,7 +245,27 @@ quint64 AnimationEffect::p_animate( EffectWindow *w, Attribute a, uint meta, int fullscreen = d->m_fullScreenEffectLock.toStrongRef(); } } - it->first.append(AniData(a, meta, ms, to, curve, delay, from, waitAtSource, keepAtTarget, fullscreen, keepAlive)); + + PreviousWindowPixmapLockPtr previousPixmap; + if (a == CrossFadePrevious) { + previousPixmap = PreviousWindowPixmapLockPtr::create(w); + } + + it->first.append(AniData( + a, // Attribute + meta, // Metadata + ms, // Duration + to, // Target + curve, // Easing curve + delay, // Delay + from, // Source + waitAtSource, // Whether the animation should be kept at source + keepAtTarget, // Whether the animation is persistent + fullscreen, // Full screen effect lock + keepAlive, // Keep alive flag + previousPixmap // Previous window pixmap lock + )); + quint64 ret_id = ++d->m_animCounter; it->first.last().id = ret_id; it->second = QRect(); @@ -344,11 +362,6 @@ void AnimationEffect::prePaintScreen( ScreenPrePaintData& data, int time ) ++animCounter; } else { EffectWindow *oldW = entry.key(); - AniData *aData = &(*anim); - if (aData->attribute == KWin::AnimationEffect::CrossFadePrevious) { - oldW->unreferencePreviousWindowPixmap(); - effects->addRepaint(oldW->expandedGeometry()); - } d->m_justEndedAnimation = anim->id; animationEnded(oldW, anim->attribute, anim->meta); d->m_justEndedAnimation = 0;