Bug 321897

Here is some short video where problem occur
wrong behaviour of "arkusz" effect

http://youtu.be/4mk5Zpf_rKw

LC_ALL="C" kcmshell4 kwincompositing

As Workaround, lower the sheet time (default is 500ms) and raise the fade out time (default is 150ms, it's in /usr/share/apps/kwin/effects/kwin4_effect_fade/contents/code/main.js) to match at eg. 300ms

@Martin
I guess the AnimationEffect should keep done Deleted's in an extra list and at their target value (until they become deleted by own or foreign unreferencing)

> I guess the AnimationEffect should keep done Deleted's in an extra list and
> at their target value (until they become deleted by own or foreign
> unreferencing)
sounds like a reasonable approach. Would be nice to know how many effects are 
operating on it, so that we don't copy the data needlessly into another list. 
Maybe we should export the refCount on the window?

(In reply to comment #4)
> sounds like a reasonable approach.
That on a second thought will introduce another issue, though:
The fade will run in 150% ie only during 30% of the sheet effect - which then basically does not happen :-\

Regardless of keeping deleted windows at their target value to prevent ugly flicker, we should think about aligning closing times.
Either trivially by just picking a constant time for all effects or by some expansion system, ie let effects set a (qMax(old, new)) property role of "MinimumCloseEffectTime" (when the window is added..) and let them use that when animating the close.

Ie. since a complex 3D transformation needs longer visualization, fade would expand to that value on demand.


> Would be nice to know how many effects are operating on it
Yes, but rather for debugging ;-)

> so that we don't copy the data needlessly into another list. 
Given the amount of windows that one would likely operate on, one would end up with a QList<QPair> and the way QList is implemented, that should not be much overhead at all.

The problem here is the lack of prediction, ie. if you've three effects, all releasing the window at the same time, two would still copy the data.

The way the scripted effects are implemented (one effect per script) we probably have too sparse animation lists anyway.

I thought about sharing the Private and have flags on whether ::prePaintScreen etc. already have been handled.

The only pitfall here is for inheriting classes which no longer could rely on execution order when calling the inherited function, ie.

void MyEffect::prePaintScreen( ScreenPrePaintData& data, int time )
{
   foo();
   AnimationEffect::prePaintScreen(data, time);
   bar();
}

may not act as expected because AnimationEffect::prePaintScreen(data, time); had been globally executed before.

No idea how much of a problem that would be.

*** Bug 322600 has been marked as a duplicate of this bug. ***

OP illustrating video:
http://youtu.be/4mk5Zpf_rKw

Bug persists with 4.10.97.

Btw, the bug was not there in 4.10.4 and older versions. At least the one I reported before it was marked as a dupe of this one. The steps to reproduce the one I'm experiencing with 4.11 but not with older versions are:

1. Run KWrite
2. Type something
3. Quit and click "Discard" in the save dialog. This will result in the KWrite window fading then pop-up and then pop-out again.

(In reply to comment #8)
> Bug persists with 4.10.97.
Because the RR commit obviously was not pushed.
We'll resolve this differently.

> Btw, the bug was not there in 4.10.4 and older versions.
Yes, we know what's causing this and how to resolve the bug.

Git commit b6390fd44c6efa7f56e486e386432c806d3f2c8e by Thomas Lübking.
Committed on 30/07/2013 at 14:09.
Pushed by luebking into branch 'KDE/4.11'.

extend fadeout animation duration to 600ms

compensated by a QuartOut shape, this keeps the
effect alive while the sheet or similar effects run
without much visual stretch

Covers issue until there's AnimationEffect::determine()
semi Fixed in 4.11.2
REVIEW: 111798

M  +2    -2    kwin/effects/fade/package/contents/code/main.js

http://commits.kde.org/kde-workspace/b6390fd44c6efa7f56e486e386432c806d3f2c8e

(In reply to comment #10)
> http://commits.kde.org/kde-workspace/b6390fd44c6efa7f56e486e386432c806d3f2c8e

Just applied this on 4.11.1. It doesn't fix the issue.

(In reply to comment #11)
> Just applied this on 4.11.1. It doesn't fix the issue.

Then you perceive something completely different.
You did restart kwin after aplpying the "patch" (to the proper javascript resource), did you?

(In reply to comment #12)
> (In reply to comment #11)
> > Just applied this on 4.11.1. It doesn't fix the issue.
> 
> Then you perceive something completely different.
> You did restart kwin after aplpying the "patch" (to the proper javascript
> resource), did you?

Yep. I logged out and also restarted KDM. The specific problem I'm talking about is the one in comment 8.

that's indeed different since the kwrite (editor, i assume?) window is no dialog, thus not affected by the sheet effect. might fall into same category, though.
please attach the output of "qdbus org.kde.kwin /KWin supportInformation"

Created attachment 82490 [details]
qdbus org.kde.kwin /KWin supportInformation

(In reply to comment #14)
> that's indeed different since the kwrite (editor, i assume?) window is no
> dialog, thus not affected by the sheet effect. might fall into same
> category, though.
> please attach the output of "qdbus org.kde.kwin /KWin supportInformation"

Attached.

Should bug 322600 be reopened?

(In reply to comment #15)

> Should bug 322600 be reopened?
Ah, that one.

Not until we figured why this still occurs for you - this bug here is not resolved "fixed" anyway.

The problem "was" that the dialogparent effect has an animationtime of 300ms, where the fade effect ran only 150ms - therefore it was reasonable an reproducible that the running dialogparent effect would cause the window to re-appear on full opacity (when the fade effect had stopped and didn't manipulate the windows opacity anymore but the dialogparent effect kept the closed window alive)

This shall no longer be the reason (the fade effect now runs 600ms *BY DEFAULT* and keeps the window nearly invisible for the last 400ms) and the issue is neither any longer reproducible here.

a) supportInformation does not print settings for scripted effects, therefore please inspect ~/.kde/share/config/kwinrc for a custom value for "FadeOutTime" - if one's there, the patch has virtually no impact.

b) Please make sure that there's no local fade effect shadowing the patched one, try altering /usr/share/apps/kwin/effects/kwin4_effect_fade/contents/code/main.js  in a way that you will notice the extended fade time (revert the patch and just set the default paramter for "FadeOutTime" to 3000 - you should be able to notice the extended fade time ;-)


@Martin
I guess the patch could/should be improved by multiplying the read "FadeOutTime" by 4 instead of manipulating just the default.

(In reply to comment #16)
> (In reply to comment #15)
>  [...]
> This shall no longer be the reason (the fade effect now runs 600ms *BY
> DEFAULT* and keeps the window nearly invisible for the last 400ms) and the
> issue is neither any longer reproducible here.
> 
> a) supportInformation does not print settings for scripted effects,
> therefore please inspect ~/.kde/share/config/kwinrc for a custom value for
> "FadeOutTime" - if one's there, the patch has virtually no impact.

Hm, FadeOutTime is not set anywhere in my kwinrc.


> b) Please make sure that there's no local fade effect shadowing the patched
> one, try altering
> /usr/share/apps/kwin/effects/kwin4_effect_fade/contents/code/main.js  in a
> way that you will notice the extended fade time (revert the patch and just
> set the default paramter for "FadeOutTime" to 3000 - you should be able to
> notice the extended fade time ;-)

If I do:

    fadeOutTime = animationTime(effect.readConfig("FadeOutTime", 3000));

then the problem is still there. It seems like readConfig() does load a setting from somewhere. If not from kwinrc, then from where?

If I force it:

    fadeOutTime = 3000;

then indeed it works. I've not set it to 300, which is just enough to get rid of the pop-back-in issue here.

somewhere out of these paths:
         kde4-config --path config

Keeps the bug a dupe, though.
I'll just push a fix multiplying the configured value (so thanks for bringing this up)

Wouldn't something like this make more sense:

    fadeOutTime = animationTime(effect.readConfig("FadeOutTime", 310));
    if (fadeOutTime < 310) {
        fadeOutTime = 310;
    }

That is, just make sure it's higher than 300?

(In reply to comment #19)
> Wouldn't something like this make more sense:

This here will eb the actual fix:
https://git.reviewboard.kde.org/r/111622/

Stretching the effect (which needs a minimum of 600ms because of this bug here) and compensating that by an altered animation curve is just a workaround.

Setting an long linear animation is no option as it makes fading feel slow and since the minimum required visual time is 150ms (= 600/4) is rather low, this shall suffice to shadow the problem until we can add the real fix.

Created attachment 82491 [details]
Updated patch

It's been actually worse: the patch actually no longer worked as expected and I always tried on the determine variant.... :-\

Please try the new attachment.

@Martin
As soon as the new patch is confirmed to work i intend to push it.

> @Martin
> As soon as the new patch is confirmed to work i intend to push it.
*nod*

(In reply to comment #21)
> Created attachment 82491 [details]
> Updated patch
> [...]
> Please try the new attachment.

Works fine now. The window stays faded out.

Git commit 28231bfe0080b79c85e7bed03eaee81c5ea809e4 by Thomas Lübking.
Committed on 25/09/2013 at 19:57.
Pushed by luebking into branch 'KDE/4.11'.

workaround bug #321897 by multiplying fadeout time

with 4 instead of just altering the default value
It seems the value is actually written for some ppl.

Worse: former patch didn't actually work as expected
anymore

M  +10   -2    kwin/effects/fade/package/contents/code/main.js

http://commits.kde.org/kde-workspace/28231bfe0080b79c85e7bed03eaee81c5ea809e4

FTR:
it seems the default parameter for effect.readConfig() is ignored and probably the config/main.xml entry used.

I just noticed that this introduces a glitch in the application launcher. When opening the launcher and starting an application, the fadeout animation of the launcher is all messed up. It pops out and in again several times per second while fading out.

(In reply to comment #26)
> I just noticed that this introduces a glitch in the application launcher.
What launcher in partcular?
Kickoff and krunner perform a slide-in (so they should be ignored by the fade effect)

(In reply to comment #27)
> What launcher in partcular?
> Kickoff and krunner perform a slide-in (so they should be ignored by the
> fade effect)

Kickoff. I have disabled the slide effect and am using fading for it.

(In reply to comment #28)

> Kickoff. I have disabled the slide effect and am using fading for it.

I bet on bug #307112 which becomes more visible due to the longer lasting animation.

In the blur effect config, disable "cache intermediate blur results" - the flicker is then hopefuly gone?

(In reply to comment #29)
> I bet on bug #307112 which becomes more visible due to the longer lasting
> animation.
> 
> In the blur effect config, disable "cache intermediate blur results" - the
> flicker is then hopefuly gone?

Yep, that's it. Disabling that option gets rid of the flicker.

Here's a video of this bug happening with the dashboard: https://dl.dropboxusercontent.com/u/8126161/fade-out_flicker.ogv

When the dashboard fades out, it flickers badly. The graphical corruption in the video (off-color squares) are only in the video, btw. Disabling "Save intermediate rendering results." in the blur effect settings resolves this issue for me as well.

Oh, forgot to mention this is with KDE 4.11.2 and confirmed on three different systems.

Please see bug #307112 - it's rather unrelated here (stretching the animation time to "slow" or so would have caused the same problem to be emphasized)

*** Bug 346442 has been marked as a duplicate of this bug. ***

The sheet effect grabs modal windows and the fade effect honors grab roles. So, marking this bug as fixed.

If we want modal windows to fade in/out with the sheet effect, then the latter should fade them. Otherwise, effects don't have clear domain boundaries and we're screwed.