Differential D14560

[effects/sheet] Grab modal windows
ClosedPublic
Actions

Authored by zzag on Aug 2 2018, 2:05 PM.

Details

Reviewers

davidedmundson

Group Reviewers

KWin

Commits

R108:10e99f64c1e1: [effects/sheet] Grab modal windows

Summary

If both the Glide effect and the Sheet effect are enabled,
they will conflict. Expected behavior would be:

the Sheet effect animates only modal windows;
the Glide effect animates the rest of normal windows.

In order to resolve the conflict, the Sheet effect has to grab
modal windows. Because it's quite specialized effect, we have
to ignore whether modal windows have been grabbed by the
Glide effect.

Test Plan

Enabled both the Glide effect and the Sheet effect;
Opened Kate;
Opened "Open file" dialog;
Closed the dialog.

Diff Detail

Repository

R108 KWin

Branch

effects-resolve-conflict-between-glide-and-sheet

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 1458
Build 1476: arc lint + arc unit

zzag created this revision.Aug 2 2018, 2:05 PM

Restricted Application added a project: KWin. · View Herald TranscriptAug 2 2018, 2:05 PM

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

zzag requested review of this revision.Aug 2 2018, 2:05 PM

Harbormaster completed remote builds in B1458: Diff 38961.Aug 2 2018, 2:05 PM

zzag edited the summary of this revision. (Show Details)Aug 2 2018, 2:07 PM

zzag edited the test plan for this revision. (Show Details)

zzag retitled this revision from [effects] Resolve conflict between Glide and Sheet to [effects/sheet] Grab modal windows.Aug 2 2018, 2:16 PM

zzag edited the summary of this revision. (Show Details)

davidedmundson added a subscriber: davidedmundson.Aug 7 2018, 1:13 PM

davidedmundson added inline comments.

effects/sheet/sheet.cpp
141	why the cast?

What would this look like in practice? Video? :D

zzag added inline comments.Aug 7 2018, 2:24 PM

effects/sheet/sheet.cpp
141	Long story short: because other effects do the same. df2b3170cbfddd3ac671c711e5839e2a51523e26 introduced grab roles (before that, effects used proxies to tell other effects what windows to ignore). At that time, Effect wasn't a QObject. So, the only viable option was to cast to void. Because other effects do w->data(WindowAddedGrabRole).value<void>(), we need the cast.

In D14560#304901, @abetts wrote:

What would this look like in practice? Video? :D

Oh nice! Question, maybe it it just the video, but does this patch address the appear/disappear issue that happens after you click cancel in the video? When you click cancel the animation begins by fading and sliding the modal window backward, but then there is a appear/disappear flash that makes it look like the fade/slide animation had a problem. It shows in SEC 5 of the video.

In D14560#304920, @abetts wrote:

Oh nice! Question, maybe it it just the video, but does this patch address the appear/disappear issue that happens after you click cancel in the video? When you click cancel the animation begins by fading and sliding the modal window backward, but then there is a appear/disappear flash that makes it look like the fade/slide animation had a problem. It shows in SEC 5 of the video.

This patch fixes exactly that problem.

In D14560#304961, @zzag wrote:

In D14560#304920, @abetts wrote:

Oh nice! Question, maybe it it just the video, but does this patch address the appear/disappear issue that happens after you click cancel in the video? When you click cancel the animation begins by fading and sliding the modal window backward, but then there is a appear/disappear flash that makes it look like the fade/slide animation had a problem. It shows in SEC 5 of the video.

This patch fixes exactly that problem.