[effects/glide] Do not animate logout screen
ClosedPublic

Authored by zzag on Jun 1 2018, 10:19 AM.

Details

Summary

BUG: 391907

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 1 2018, 10:19 AM
Restricted Application added a project: KWin. · View Herald TranscriptJun 1 2018, 10:19 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Jun 1 2018, 10:19 AM
broulik added inline comments.
effects/glide/glide.h
30 ↗(On Diff #35313)

Unrelated, forward-declaration of QTimeLine is fine given it's only stored as a pointer

effects/glide/glide.kcfg
18 ↗(On Diff #35313)

Does this need to be configurable given in other places (login, logout, fade effect) it's hardcoded?

zzag added inline comments.Jun 1 2018, 11:02 AM
effects/glide/glide.h
30 ↗(On Diff #35313)

QTimeLine is external stuff, that's not part of KWin. What if definition of QTimeLine will be changed in the future?

effects/glide/glide.kcfg
18 ↗(On Diff #35313)

I think this approach is better than hard coding stuff. Again, if you insist on hard coding it, I'll hard code it.

broulik added inline comments.Jun 1 2018, 1:20 PM
effects/glide/glide.h
30 ↗(On Diff #35313)

It can only in Qt 6 and a pointer is still a pointer, all that the compiler needs to know is "this class exists" for this member variable.

zzag added inline comments.Jun 1 2018, 1:27 PM
effects/glide/glide.h
30 ↗(On Diff #35313)

Some time ago, Hugo Pereira Da Costa added a good comment on forward-declaring Qt stuff:

I think the "rule" is to not use forward declarations for classes external to once project. Reason is that the upstream library might decide to turn a class into a struct, or an alias into future changes, which will then break your code.

Anyway, if you still think QTimeLine should be forward-declared, I'll update the diff.

romangg added a subscriber: romangg.Jun 1 2018, 2:51 PM
romangg added inline comments.
effects/glide/glide.h
30 ↗(On Diff #35313)

Pls forward-declare.

effects/glide/glide.kcfg
18 ↗(On Diff #35313)

Pls hard code it.

zzag updated this revision to Diff 35344.Jun 1 2018, 3:01 PM

Hard code the blacklist.

zzag updated this revision to Diff 35347.Jun 1 2018, 3:08 PM

Move the blacklist to the KWin namespace. (for the consistency sake)

romangg accepted this revision.Jun 1 2018, 3:20 PM
This revision is now accepted and ready to land.Jun 1 2018, 3:20 PM
broulik added inline comments.Jun 1 2018, 3:37 PM
effects/glide/glide.cpp
38

Any particular reason for using a QSet? (just curious)

romangg added inline comments.Jun 1 2018, 3:47 PM
effects/glide/glide.cpp
38

Fast lookup.

This revision was automatically updated to reflect the committed changes.