D17343: [wayland] Don't honor the idle inhibitor object if the surface is not visible

[Vlad Zagorodniy]

zzag created this revision.
zzag added a reviewer: KWin.
Herald added a project: KWin.
Herald added a subscriber: kwin.
zzag requested review of this revision.

REVISION SUMMARY
Currently, our implementation of zwp_idle_inhibitor_v1 is not fully
spec-compliant. If the associated surface is not visible, we still
honor the idle inhibitor object.

This change addresses those spec-compliance issues. If a surface is
minimized or it's not on the current virtual desktop, then the
associated inhibitor object won't be honored by KWin.

REPOSITORY
R108 KWin

BRANCH
idle-inhibit-spec-compliant

REVISION DETAIL
https://phabricator.kde.org/D17343

AFFECTED FILES
autotests/integration/idle_inhibition_test.cpp
idle_inhibition.cpp
idle_inhibition.h

To: zzag, #kwin
Cc: kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Martin Flöser]

graesslin added inline comments.

INLINE COMMENTS

idle_inhibition.cpp:112
+{
+ const auto stackingOrder = workspace()->stackingOrder();
+ for (Toplevel *toplevel : stackingOrder) {

You could use Workspace::forEachAbstractClient instead. You probably can get this to a one liner in the connect through std::bind.

idle_inhibition.h:59
+ void slotWorkspaceCreated();

You don't need the Q_SLOTS any more with the new connect syntax.

REPOSITORY
R108 KWin

REVISION DETAIL
https://phabricator.kde.org/D17343

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Vlad Zagorodniy]

zzag updated this revision to Diff 46850.
zzag added a comment.


Use Workspace::forEachAbstractClient.

REPOSITORY
R108 KWin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17343?vs=46832&id=46850

BRANCH
idle-inhibit-spec-compliant

REVISION DETAIL
https://phabricator.kde.org/D17343

AFFECTED FILES
autotests/integration/idle_inhibition_test.cpp
idle_inhibition.cpp
idle_inhibition.h

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Vlad Zagorodniy]

zzag added inline comments.

INLINE COMMENTS

graesslin wrote in idle_inhibition.h:59
You don't need the Q_SLOTS any more with the new connect syntax.

Yes, I know, I just prefer to be explicit.

REPOSITORY
R108 KWin

REVISION DETAIL
https://phabricator.kde.org/D17343

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Vlad Zagorodniy]

zzag marked 3 inline comments as done.

REPOSITORY
R108 KWin

REVISION DETAIL
https://phabricator.kde.org/D17343

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Vlad Zagorodniy]

zzag added a dependent revision: D17353: Use appropriate sequence algorithms in IdleInhibition.

REPOSITORY
R108 KWin

REVISION DETAIL
https://phabricator.kde.org/D17343

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Martin Flöser]

graesslin added inline comments.

INLINE COMMENTS

idle_inhibition.cpp:113
+ workspace()->forEachAbstractClient([this] (AbstractClient *c) {
+ if (auto client = qobject_cast<ShellClient *>(c)) {
+ update(client);

I don't think we need to cast here at all. AbstractClient has a surface() as well.

REPOSITORY
R108 KWin

REVISION DETAIL
https://phabricator.kde.org/D17343

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Vlad Zagorodniy]

zzag updated this revision to Diff 46858.
zzag added a comment.


Use more AbstractClient.

REPOSITORY
R108 KWin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17343?vs=46850&id=46858

BRANCH
idle-inhibit-spec-compliant

REVISION DETAIL
https://phabricator.kde.org/D17343

AFFECTED FILES
autotests/integration/idle_inhibition_test.cpp
idle_inhibition.cpp
idle_inhibition.h

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Vlad Zagorodniy]

zzag marked an inline comment as done.

REPOSITORY
R108 KWin

REVISION DETAIL
https://phabricator.kde.org/D17343

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Vlad Zagorodniy]

zzag planned changes to this revision.
zzag added a comment.


Don't honor the idle inhibitor if the client leaves the current virtual desktop.

REPOSITORY
R108 KWin

REVISION DETAIL
https://phabricator.kde.org/D17343

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

[Vlad Zagorodniy]

zzag updated this revision to Diff 46881.
zzag added a comment.


Don't honor the idle inhibitor if the client leaves the current virtual desktop.

(I hope that all cases when a client can become invisible were taken into account)

REPOSITORY
R108 KWin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17343?vs=46858&id=46881

BRANCH
idle-inhibit-spec-compliant

REVISION DETAIL
https://phabricator.kde.org/D17343

AFFECTED FILES
autotests/integration/idle_inhibition_test.cpp
idle_inhibition.cpp
idle_inhibition.h

To: zzag, #kwin
Cc: graesslin, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart