Bug 336866

Summary: Scalein affects Plasma popups
Product: [Plasma] kwin Reporter: Kai Uwe Broulik <kde>
Component: effects-window-managementAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: dreamsoul14
Priority: NOR Flags: mgraesslin: ReviewRequest+
Version First Reported In: git master   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
URL: https://phabricator.kde.org/D3211
Latest Commit: Version Fixed/Implemented In: 5.8.4
Sentry Crash Report:

Description Kai Uwe Broulik 2014-06-29 11:48:23 UTC
The Scalein effect also scales in Plasma popups (which are slided in simultaneously).
The script checks for effect.isGrabbed(window, Effect.WindowAddedGrabRole) which seems to have no effect. The Plasma popups also don't have any special property (just WINDOW_TYPE_OVERRIDE and WINDOW_TYPE_NORMAL) we could check for.

Reproducible: Always

Steps to Reproduce:
1. Run plasmashell
2. Enable scalein
(3. Set animation speed to low)
4. Open a Plasma popup (eg. kickoff menu)
Actual Results:  
The kickoff menu is scaled in during its slide in animation

Expected Results:  
The kickoff menu is just slided into view
Comment 1 Martin Flöser 2015-01-14 08:41:08 UTC
can you please try again, I'm not able to reproduce.
Comment 2 Martin Flöser 2015-01-22 07:49:44 UTC
*** Bug 343143 has been marked as a duplicate of this bug. ***
Comment 3 Martin Flöser 2016-10-29 18:47:15 UTC
I just tried again and this works fine for me. In case you are still able to reproduce, please reopen.
Comment 4 Martin Flöser 2016-10-29 18:52:46 UTC
Reopening after further testing I'm able to reproduce. The order of the effect loading is responsible for this problem. If I active scale in, in a running session the problem does not happen.

But at KWin startup scale in seems to be loaded before sliding popups and thus the condition happens.
Comment 5 Martin Flöser 2016-10-29 18:52:59 UTC
*** Bug 323542 has been marked as a duplicate of this bug. ***
Comment 6 Martin Flöser 2016-10-30 10:29:23 UTC
Git commit 42e7ffa263ddc7a13d32126ac6356a7f1b1950b3 by Martin Gräßlin.
Committed on 30/10/2016 at 10:27.
Pushed by graesslin into branch 'Plasma/5.8'.

[autotests/effect] Add test case for Sliding Popups effect

The test case loads the sliding popups effect and the scale in effect
which also operates on added windows. As the test case shows depending
on the sequence how the effects are loaded, the window gets animated by
both effects (wrong) oor only sliding popups.

M  +1    -0    autotests/integration/effects/CMakeLists.txt
A  +222  -0    autotests/integration/effects/slidingpopups_test.cpp     [License: GPL (v2)]

http://commits.kde.org/kwin/42e7ffa263ddc7a13d32126ac6356a7f1b1950b3
Comment 7 Martin Flöser 2016-10-31 06:49:58 UTC
Git commit b84fd50319feed58791ec8577c18d88229c2d589 by Martin Gräßlin.
Committed on 31/10/2016 at 06:48.
Pushed by graesslin into branch 'Plasma/5.8'.

[autotests] Extend SlidingPopupsTest::testWithOtherEffect

Test more combinations of other effects together with sliding popups.
The problem does not only exist for scale in but for pretty much any
effect that the ordering in which the effects get loaded makes the test
pass or fail.

Some effects require OpenGL, as build.kde.org does not support OpenGL
compositing (yet), the tests only do the OpenGL cases if an OpenGL
compositor could be created.

M  +28   -0    autotests/integration/effects/slidingpopups_test.cpp

http://commits.kde.org/kwin/b84fd50319feed58791ec8577c18d88229c2d589
Comment 8 Martin Flöser 2016-10-31 08:09:20 UTC
The problem is that the effects connect to a signal emitted by EffectsHandler. Thus, first come, first serve.
Comment 9 Martin Flöser 2016-10-31 10:29:48 UTC
Possible patch at https://phabricator.kde.org/D3211
Comment 10 Martin Flöser 2016-11-07 12:06:51 UTC
Git commit fb69b791a16f4a89fd79a010ce8f67419de16004 by Martin Gräßlin.
Committed on 07/11/2016 at 10:45.
Pushed by graesslin into branch 'Plasma/5.8'.

Ensure that all Effects honour the grab roles correctly

Summary:
When windows get added some effects grab the window and want to be the
only one animating this window. For this the grab roles exists. An
effect being notified later on evaluates the grab state and does not
start the animation.

This process failed due to being dependent on the order the effects are
loaded. Window Added/Closed are signals emitted by EffectsHandler, thus
first come, first serve. The requested effect order does not play into
it.

Due to that it could happen that an Effect which should not animate,
started to animate as the grab was still there.

This change adds the possibility to be notified whenever the window data
changes. A new signal is added to EffectsHandler which is emitted
whenever the windowData changes. The interested effects connect to it
and cancel their (just started) animation for the window.

Adjusted effects are:
* ScaleIn
* Fade
* WobblyWindows

In case of WobblyWindows an additional logical error was fixed that the
animations were only run when an effect grabbed instead of the other way
around.
FIXED-IN: 5.8.4

Reviewers: #kwin, #plasma, broulik

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D3211

M  +0    -12   autotests/integration/effects/slidingpopups_test.cpp
M  +1    -0    effects.cpp
M  +19   -2    effects/fade/package/contents/code/main.js
M  +13   -1    effects/scalein/package/contents/code/main.js
M  +0    -1    effects/slidingpopups/slidingpopups.cpp
M  +29   -2    effects/wobblywindows/wobblywindows.cpp
M  +1    -0    effects/wobblywindows/wobblywindows.h
M  +18   -0    libkwineffects/kwineffects.h

http://commits.kde.org/kwin/fb69b791a16f4a89fd79a010ce8f67419de16004