[effects/dialogparent] Fix flickering of parent windows
ClosedPublic

Authored by zzag on Aug 18 2018, 12:10 PM.

Details

Summary

If a modal window is closed and some alternative effect that animates
the disappearing of windows is enabled(e.g. the Glide effect, or the
Scale effect), the Dialog Parent effect can cause flickering of the
parent window because its animation duration doesn't match duration of
those alternative effects.

Also, if the Fade effect, the Glide effect, and the Scale effect are
disabled, the Dialog Parent will keep the parent window alive for no
good reason.

This change addresses that problem by adding keepAlive property to
animate function so scripted effects have more control over lifetime
of animated windows.

If both a modal window and its parent window are closed at the same time
(and there is no effect that animates the disappearing of windows), the
Dialog Parent will stop immediately(because windowDeleted will be
emitted right after windowClosed signal).

If both a modal window and its parent window are closed at the same time
(and there is effect that animates the disappearing of windows), the
Dialog Parent won't reference the latter window. Thus, it won't cause
flickering. I.e. it will "passively" animate parent windows.

BUG: 355036
FIXED-IN: 5.15.0

Diff Detail

Repository
R108 KWin
Branch
effects-passive-dialogparent
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 3716
Build 3734: arc lint + arc unit
zzag created this revision.Aug 18 2018, 12:10 PM
Restricted Application added a project: KWin. · View Herald TranscriptAug 18 2018, 12:10 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Aug 18 2018, 12:10 PM
zzag retitled this revision from [effects/dialogparent] Fix flickering of parent window to [effects/dialogparent] Fix flickering of parent windows.Aug 18 2018, 12:10 PM
zzag edited the summary of this revision. (Show Details)Sep 18 2018, 1:03 PM

I think this patch should be rebased on the fullscreen patch.

zzag abandoned this revision.Sep 22 2018, 10:25 AM
zzag reclaimed this revision.Oct 9 2018, 2:11 PM

Needs to be rebased.

zzag edited the summary of this revision. (Show Details)Oct 9 2018, 2:12 PM
davidedmundson added inline comments.
libkwineffects/kwinanimationeffect.cpp
299

If I have two animations active and only one is keep alive, if I cancel the keep alive one should I unref the window?

387

and here

zzag added inline comments.Oct 9 2018, 3:43 PM
libkwineffects/kwinanimationeffect.cpp
299

Yes, we should unref window. I was thinking about implementing something similar to FullScreenEffectLock, e.g.

class KeepAliveLock
{
public:
    KeepAliveLock(EffectWindow *w) { w->refWindow(); }
    ~KeepAliveLock() { w->unrefWindow(); }
    ...
};
davidedmundson added inline comments.Oct 9 2018, 3:49 PM
libkwineffects/kwinanimationeffect.cpp
299

Should allow us to remove m_zombies, that'll clean things up. Lets do it.

zzag updated this revision to Diff 43284.Oct 10 2018, 10:01 AM

Use KeepAliveLock

zzag updated this revision to Diff 43286.Oct 10 2018, 10:02 AM

Fix new lines

davidedmundson accepted this revision.Oct 10 2018, 10:09 AM
This revision is now accepted and ready to land.Oct 10 2018, 10:09 AM
This revision was automatically updated to reflect the committed changes.