[effects/cubeslide] Fix "sticky" windows detection on Wayland
ClosedPublic

Authored by poboiko on Sep 14 2018, 7:59 AM.

Details

Summary

This is second part of D15175: [effects/cubeslide] Fix visual glitches with Blur / BackgroundContrast effect, and aimed at fixing Bug 390366.
The problem was that effect relied on isManaged() to detect OSDs and notifications, that should be painted on top of the cube. On Wayland, the desktop window itself is not managed, which made it "sticky".

Instead we use isSpecialWindow() check to explicitly make notifications and OSDs sticky.

BUG: 390366
FIXED-IN: 5.15.0

Diff Detail

Repository
R108 KWin
Lint
Lint Skipped
Unit
Unit Tests Skipped
poboiko created this revision.Sep 14 2018, 7:59 AM
Restricted Application added a subscriber: kwin. · View Herald TranscriptSep 14 2018, 7:59 AM
poboiko requested review of this revision.Sep 14 2018, 7:59 AM
davidedmundson requested changes to this revision.Sep 14 2018, 8:54 AM
davidedmundson added a subscriber: davidedmundson.

if (w->isSpecialWindow()) {

and everything makes sense, that's a known wayland behavioural change. ++

// Apparently, acceptsFocus() is the only way how OSD differs from ordinary window on Wayland

This is working round KDE code. We shouldn't be doing that. I'm happy to help fix as it'll be Plasma::Dialog at fault.

I've applied this patch and removed that line, and can't see what's wrong?
I switch desktop, the OSD remains in the middle. Any notification dialogs stay where they should.

This revision now requires changes to proceed.Sep 14 2018, 8:54 AM
zzag added a subscriber: zzag.Sep 14 2018, 9:15 AM

This diff has supposed to replace !w->isManaged with w->isSpecialWindow. As @davidedmundson said, we work around KDE's code. The desktop switch OSD has to set appropriate window type (see scripts/desktopchangeosd).

I switch desktop, the OSD remains in the middle. Any notification dialogs stay where they should.

Did you replace return false with return !dontSlideStickyWindows? I think we won't face problems with notifications, only with the OSD.

poboiko updated this revision to Diff 41621.Sep 14 2018, 9:34 AM

OK, removed the workaround.

I've applied this patch and removed that line, and can't see what's wrong?
I switch desktop, the OSD remains in the middle. Any notification dialogs stay where they should.

Apparently, you have "Do not animate windows on all desktops" checked. For me, if I uncheck it, DesktopChangeOSD slides with the cube.
However, notifications indeed work fine.

zzag added a comment.Sep 14 2018, 9:52 AM

Please rebase this change on master.

poboiko updated this revision to Diff 41626.Sep 14 2018, 9:55 AM
poboiko edited the summary of this revision. (Show Details)

OK, isDesktop() change has already landed, updated the diff accordingly

davidedmundson accepted this revision.Sep 14 2018, 1:00 PM
This revision is now accepted and ready to land.Sep 14 2018, 1:00 PM
zzag added a comment.EditedSep 14 2018, 4:07 PM

@davidedmundson When this patch should be landed? When Plasma::Dialog is fixed? Or it can go in as it is right now?

Now I can reproduce the bug \o/

Or it can go in as it is right now?

May as well go now.

It's no worse than before and any dialog fixes will go in a different code area.
Besides on any non-debug animation speed the OSD doesn't actually look too bad even if it's not what's intended.

This revision was automatically updated to reflect the committed changes.