[effects/fade] Temporarily ignore shown/hidden signals
AbandonedPublic

Authored by zzag on Mar 14 2018, 2:42 AM.

Details

Reviewers
graesslin
Group Reviewers
KWin
Plasma
VDG
Summary

The fade effect is being false triggered when switching between virtual
desktops. These false triggers cause window flickering(see video below),
which is really annoying sometimes.

As temporal countermeasure, ignore windowShown/windowHidden signals.
Ignoring previously mentioned signals won't change Plasma experience too
much, e.g. tooltips/default application launcher/etc still would fade in/out.
(I think they don't reuse the same window)

This is a little bit radical change, but currently the fade effect does more harm than good...

Test Plan
  • enable Fade effect
  • switch between virtual desktops

Diff Detail

Repository
R108 KWin
Branch
dont-fade-in-out
Lint
No Linters Available
Unit
No Unit Test Coverage
zzag created this revision.Mar 14 2018, 2:42 AM
Restricted Application added a project: KWin. · View Herald TranscriptMar 14 2018, 2:42 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Mar 14 2018, 2:42 AM
zzag edited the summary of this revision. (Show Details)Mar 14 2018, 2:43 AM

I think this should check for effect.isGrabbed to avoid two effects animating the same thing?

zzag added a comment.EditedMar 14 2018, 1:37 PM

I think this should check for effect.isGrabbed to avoid two effects animating the same thing?

Why? It occurs for example when switching one desktop to right, etc.

I think about checking presence of fullscreen effect. But I'm not sure about event ordering.

Update
Please note, if none of effects under "Virtual Desktop Switching Animation" is enabled, this won't help.
Another ugly way is to listen for desktopChanged signal and set onetime flags to not fade.
Currently, the easier is to ignore windowShown/windowHidden signals.

graesslin requested changes to this revision.Mar 17 2018, 11:46 AM
graesslin added a subscriber: graesslin.

I'm sorry but this breaks Wayland, auto-hiding panels and much more. We need a different solution for the problem. E.g. a signal beginSwitchDesktop and endSwitchDesktop or a property isCurrentlySwitchingDesktops. Also this is only an issue for X11 windows and we don't do feature changes for X11 any more.

A change which breaks Wayland for an improvement on X11 is unfortunately not acceptable anymore.

This revision now requires changes to proceed.Mar 17 2018, 11:46 AM
zzag abandoned this revision.Mar 17 2018, 1:33 PM

I'm sorry but this breaks Wayland, auto-hiding panels and much more.

Sorry, I don't use Wayland because many features I use per day basis aren't working. Should have tested on Wayland too, sorry.

We need a different solution for the problem. E.g. a signal beginSwitchDesktop and endSwitchDesktop or a property isCurrentlySwitchingDesktops.

The property sounds like a hack. Could you please tell more about begin/endSwitchDesktop approach? E.g. when endSwitchDesktop will be emitted? what's the order of desktopChanged and windowShown/windowHidden signals, etc?

Also, I think the discussion should move in an appropriate bug report.

Also this is only an issue for X11 windows and we don't do feature changes for X11 any more.

Maybe I'm wrong, but that's not only an issue for X11, that's an issue for Wayland too. ;-)

In D11305#227874, @zzag wrote:

I'm sorry but this breaks Wayland, auto-hiding panels and much more.

Sorry, I don't use Wayland because many features I use per day basis aren't working. Should have tested on Wayland too, sorry.

We need a different solution for the problem. E.g. a signal beginSwitchDesktop and endSwitchDesktop or a property isCurrentlySwitchingDesktops.

The property sounds like a hack. Could you please tell more about begin/endSwitchDesktop approach? E.g. when endSwitchDesktop will be emitted? what's the order of desktopChanged and windowShown/windowHidden signals, etc?

It's an idea I got from QAbstractItemModel - it emits signals like aboutToMoveRows. The problematic area is in Workspace where first all X11 Windows get hidden, then shown again. So basically what would be needed is emitting the signal at start and end of that method.

Also, I think the discussion should move in an appropriate bug report.

Also this is only an issue for X11 windows and we don't do feature changes for X11 any more.

Maybe I'm wrong, but that's not only an issue for X11, that's an issue for Wayland too. ;-)

Only X11 Windows get their visibility updated in that method.

zzag added a comment.Mar 17 2018, 9:35 PM

It's an idea I got from QAbstractItemModel - it emits signals like aboutToMoveRows. The problematic area is in Workspace where first all X11 Windows get hidden, then shown again. So basically what would be needed is emitting the signal at start and end of that method.

Only X11 Windows get their visibility updated in that method.

Yeah, seems Workspace::updateClientVisibilityOnDesktopChange updates visibility only for X11 clients. Why doesn't it update visibility of ShellClients too?

If it would stay that way(visibility of ShellClients won't change) then I suggest to leave it as it is right now and focus more on Wayland.
Also, this is a little bit off topic but could you please take a look at my comment in the bug 384462 and give your thoughts on that?

In D11305#228208, @zzag wrote:

It's an idea I got from QAbstractItemModel - it emits signals like aboutToMoveRows. The problematic area is in Workspace where first all X11 Windows get hidden, then shown again. So basically what would be needed is emitting the signal at start and end of that method.

Only X11 Windows get their visibility updated in that method.

Yeah, seems Workspace::updateClientVisibilityOnDesktopChange updates visibility only for X11 clients. Why doesn't it update visibility of ShellClients too?

Because it's not needed ;-)

If it would stay that way(visibility of ShellClients won't change) then I suggest to leave it as it is right now and focus more on Wayland.

We can improve this area. After all it would also improve the situation for xwayland windows.

Also, this is a little bit off topic but could you please take a look at my comment in the bug 384462 and give your thoughts on that?