[effects/slidingpopups] Overhaul the animations data struct
ClosedPublic

Authored by zzag on Jul 23 2018, 3:16 PM.

Details

Summary

The existing Data struct has some serious problems:

(a) naming is not intuitive, SlideInterface uses more cleaner terminology

so let's use that (and because Wayland is the future);

(b) fadeInTime and fadeOutTime should be slideInDuration and slideOutDuration

respectively. The Sliding popups effect doesn't fade windows, it slides
them;

(c) mWindowsData should be m_animationsData because other parts of this

effect refer to it as "anim data"(e.g. setupAnimData).

This effect uses its own Location enum class instead of KWayland::
Server::SlideInterface::Location because it would be better to not
depend on platform specific data structures.

As a side effect, this change also fixes QHash abuse. The Sliding popups
effect is still hashing windows twice in prePaintWindow and paintWindow.
But I think that's acceptable because usually there would be only one
active sliding window.

CCBUG: 331118

Diff Detail

Repository
R108 KWin
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1788
Build 1806: arc lint + arc unit
zzag created this revision.Jul 23 2018, 3:16 PM
Restricted Application added a project: KWin. · View Herald TranscriptJul 23 2018, 3:16 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Jul 23 2018, 3:16 PM

@davidedmundson I think that's the last intrusive change. The rest would be about getting code in good shape(e.g. adjusting coding style, using override).

I hope that would be okay to have one unrelated change in this diff: renaming mFadeInTime to m_slideInDuration. It's awkward to have

animData.slideInDuration = mFadeInTime;
animData.slideOutDuration = mFadeOutTime;

and I don't want to create yet another diff for such a small change (the sliding popups effect patch chain is already pretty big).

zzag added a comment.EditedJul 23 2018, 3:29 PM

Also, shouldn't

//custom fadeout
animData.slideOutDuration = std::chrono::milliseconds(d[2]);

be

animData.slideOutDuration = m_slideOutDuration;

?

That's line 318 in slidingpopups.cpp.

zzag edited the test plan for this revision. (Show Details)Jul 23 2018, 3:45 PM
zzag updated this revision to Diff 39624.Aug 13 2018, 4:55 PM

Rebase.

zzag edited the summary of this revision. (Show Details)Aug 13 2018, 5:00 PM
davidedmundson accepted this revision.Aug 13 2018, 10:05 PM
This revision is now accepted and ready to land.Aug 13 2018, 10:05 PM
This revision was automatically updated to reflect the committed changes.