Differential D13479

[effects] Ignore previous state of WindowForceBlurRole
ClosedPublic
Actions

Authored by zzag on Jun 11 2018, 11:51 AM.

Details

Reviewers

fredrik

Group Reviewers

KWin

Commits

R108:e9ab34854d1c: [effects] Ignore previous state of WindowForceBlurRole

Summary

Currently, effects like Maximize, Slide Back have problems with setting
WindowForceBlurRole. They store previous state of WindowForceBlurRole.
This is wrong. Instead they should either ignore previous state of
WindowForceBlur or refcount forced role.

There's no need for refcounting right now. For example, if several effects
force blur or background contrast, they are most likely in a conflict.
Please notice that the Desktop Grid effect uses the Present Windows
effect only to calculate transformations.

Some other problems with the code that sets WindowForceBlurRole:

Maximize effect stores previous state of WindowForceBlurRole only for one window. It ignores the fact that there could be several active maximize animations;
Desktop Grid/Present Windows/Slide back don't clean after themselves. So, after using those effects for good amount of times, memory usage will bump.

Test Plan

Enabled blur for Konsole
Maximized Konsole
Activated Present Windows
Activated Desktop Grid
Raised another window(to trigger Slide Back)

Diff Detail

Repository

R108 KWin

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

zzag created this revision.Jun 11 2018, 11:51 AM

Restricted Application added a project: KWin. · View Herald TranscriptJun 11 2018, 11:51 AM

Restricted Application added a subscriber: kwin. · View Herald Transcript

zzag requested review of this revision.Jun 11 2018, 11:51 AM

zzag edited the summary of this revision. (Show Details)Jun 11 2018, 11:56 AM

Ping.

It would be easy to change WindowForceBlurRole from a bool to an int, and have effects increment/decrement the value though.

This revision is now accepted and ready to land.Jun 19 2018, 11:47 AM

In D13479#279988, @fredrik wrote:

It would be easy to change WindowForceBlurRole from a bool to an int, and have effects increment/decrement the value though.

So, it will be essentially refcounting. I think it can break "compatibility" with third-party effects. I proposed some time ago to add the following methods

void force(int role);
void unforce(int role);
bool hasForced(int role);

to EffectWindow, which are doing refcounting stuff.

https://paste.kde.org/pgdhehzpu

Closed by commit R108:e9ab34854d1c: [effects] Ignore previous state of WindowForceBlurRole (authored by zzag). · Explain WhyJun 19 2018, 12:23 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			effects/desktopgrid/desktopgrid.h (3 lines)
M			effects/desktopgrid/desktopgrid.cpp (3 lines)
M			effects/maximize/package/contents/code/maximize.js (4 lines)
M			effects/presentwindows/presentwindows.h (3 lines)
M			effects/presentwindows/presentwindows.cpp (5 lines)
M			effects/slideback/slideback.h (2 lines)
M			effects/slideback/slideback.cpp (3 lines)