Bug 398100 - Effects that check the managed property do not work on Wayland
Summary: Effects that check the managed property do not work on Wayland
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: effects-various (show other bugs)
Version: 5.13.4
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-31 15:03 UTC by Vlad Zahorodnii
Modified: 2018-10-19 15:26 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vlad Zahorodnii 2018-08-31 15:03:53 UTC
Some effects check the managed property to decide whether they have to animate given window.

On Wayland, there is no concept of managed/unmanaged windows. Thus, isManaged always returns false for wayland clients.

Currently, the following effects check the managed prop:
* Glide
* Dim Screen
* Cube Slide
* Scale
* Dim Inactive
* Translucency
Comment 1 Vlad Zahorodnii 2018-08-31 15:05:54 UTC
Also, what makes hard to abstract what those effects check is a bug(?) in Qt: tooltips on Wayland are not xdg_popups.
Comment 2 Michael D 2018-09-28 10:45:40 UTC
Perhaps this is related (but if not I'll make a new report) but menus are animated under wayland but not X. E.g. if the glide effect is enabled, it is applied to right-click menus under wayland, but not in X.
Comment 3 David Edmundson 2018-10-03 23:57:29 UTC
Whilst generally I want to abstract X and wayland, in this case the concept of unmanaged doesn't apply. I don't want to go down the route of faking things for compatibility - wayland breaks effects in many ways already anyway.

Effects should be already doing something along the lines of:

if (!unmanaged && !popup) do stuff 

>Qt: tooltips on Wayland are not xdg_popups.

They should be. 

I remember discussing both of these things in a phab request somewhere.
Comment 4 Vlad Zahorodnii 2018-10-04 06:40:28 UTC
If we are about to do the following check

> if (!unmanaged && !popup) do stuff 

or more concrete

    if (!w->isManaged() && w->isPopupWindow()) {
        return false;
    }

then we don't need to check whether window is managed, I guess, checking whether window is a popup should be enough because I landed recently https://phabricator.kde.org/D15353 (combobox popups will be seen as popups).

Then we also don't have to introduce isX11Client/isWaylandClient to libkwineffects, which is really good.
Comment 5 Vlad Zahorodnii 2018-10-06 17:55:22 UTC
Just for tracking: the Cube Slide effect is no longer checking the managed property.
Comment 6 Vlad Zahorodnii 2018-10-09 08:22:13 UTC
Git commit 769f2659ddfff8096b826238595a26e5f21c39ad by Vlad Zagorodniy.
Committed on 09/10/2018 at 08:04.
Pushed by vladz into branch 'master'.

[effects] Make Scale and Glide effects Wayland-friendly

Summary:
The Scale effect and the Glide effect have to animate only ordinary
windows(i.e. the ones that are considered to be apps).

On X11, in order to distinguish ordinary windows from combo box popups,
popup menus, and other popups, those effects check whether given window
is managed.

On Wayland, there is no concept of managed/unmanaged windows.

XDG Shell protocol defines 2 surface roles:
* xdg_toplevel;
* and, xdg_popup.

The former can be used to implement typical windows, the ones that can
be minimized, maximized, etc.

The latter can be used to implement tooltips, popup menus, etc. Thus,
that's a good criteria to filter popup windows.

Reviewers: #kwin, graesslin, davidedmundson

Reviewed By: #kwin, graesslin, davidedmundson

Subscribers: davidedmundson, kwin

Tags: #kwin

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

M  +8    -1    effects/glide/glide.cpp
M  +8    -1    effects/scale/scale.cpp
M  +21   -0    libkwineffects/kwineffects.cpp
M  +38   -0    libkwineffects/kwineffects.h
M  +11   -0    shell_client.cpp
M  +2    -0    shell_client.h
M  +28   -0    toplevel.h

https://commits.kde.org/kwin/769f2659ddfff8096b826238595a26e5f21c39ad
Comment 7 Vlad Zahorodnii 2018-10-19 11:38:21 UTC
Git commit 9f3a447a43e35146f67dc27d176e5b00ceb0665e by Vlad Zagorodniy.
Committed on 19/10/2018 at 11:38.
Pushed by vladz into branch 'master'.

[effects/diminactive] Dim Wayland clients

Summary:
There is no concept of managed windows in Wayland, so every time we call
managed() on a Wayland client, it will return false.

This change addresses that problem by invoking managed() only for X11 clients.

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: kwin

Tags: #kwin

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

M  +5    -1    effects/diminactive/diminactive.cpp

https://commits.kde.org/kwin/9f3a447a43e35146f67dc27d176e5b00ceb0665e
Comment 8 Vlad Zahorodnii 2018-10-19 14:50:54 UTC
Git commit 4e545f5594fc8445046f7b6a20b2c313a7891785 by Vlad Zagorodniy.
Committed on 19/10/2018 at 14:49.
Pushed by vladz into branch 'master'.

[effects/dimscreen] Make it work on Wayland

Summary:
There is no concept of managed windows in Wayland, so every time we call
managed() on a Wayland client, it will return false. We need to call
that method only for X11 clients.

The resource name part for authentication agents is empty because
KWayland can't get their executable paths.

Test Plan:
Before:
{F6338545}

After:
{F6338546}

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: davidedmundson, kwin

Tags: #kwin

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

M  +19   -1    effects/dimscreen/dimscreen.cpp

https://commits.kde.org/kwin/4e545f5594fc8445046f7b6a20b2c313a7891785
Comment 9 Vlad Zahorodnii 2018-10-19 14:59:13 UTC
Just for the record: now, only the Translucency effect "is not working" on Wayland. Other effects **should** work on Wayland.
Comment 10 Vlad Zahorodnii 2018-10-19 15:24:59 UTC
Git commit 8063740efb868a6711d747c2e2559bd57398ead6 by Vlad Zagorodniy.
Committed on 19/10/2018 at 15:24.
Pushed by vladz into branch 'master'.

[effects/translucency] Make inactive Wayland clients translucent

Summary:
There is no concept of managed windows in Wayland, so every time we call
managed() on a Wayland client, it will return false.

This change addresses that problem by invoking managed() only for X11 clients.

Test Plan:
* Open KCM of the Translucency effect and decrease opacity of inactive windows;
* Open Dolphin;
* Click on desktop.

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: kwin

Tags: #kwin

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

M  +2    -1    effects/translucency/package/contents/code/main.js

https://commits.kde.org/kwin/8063740efb868a6711d747c2e2559bd57398ead6