Differential D14911

[effects/maximize] Make it work on Wayland
AbandonedPublic
Actions

Authored by zzag on Aug 17 2018, 7:27 PM.

Details

Reviewers
None
Group Reviewers
KWin
Summary

The Maximize effect doesn't work on Wayland because
windowGeometryShapeChanged and windowMaximizedStateChanged are emitted
in different order:

for maximize:

  • windowGeometryShapeChanged
  • windowGeometryShapeChanged
  • windowMaximizedStateChanged
  • windowGeometryShapeChanged

for restore:

  • windowGeometryShapeChanged
  • windowMaximizedStateChanged
  • windowGeometryShapeChanged

We care only about window geometry in the last
windowGeometryShapeChanged signal. Thus, we have to start the maximize
animation only after the last windowGeometryShapeChanged signal has been
emitted, not after windowMaximizedStateChanged.

windowMaximizedStateChanged signal is used only to decide whether the
proceeding windowGeometryShapeChanged signal will be the last.

BUG: 384462

Depends on D15135

Diff Detail

Branch
effects-maximize-wayland
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2335
Build 2353: arc lint + arc unit
zzag created this revision.Aug 17 2018, 7:27 PM
Restricted Application added a project: KWin. · View Herald TranscriptAug 17 2018, 7:27 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Aug 17 2018, 7:27 PM
davidedmundson added a subscriber: davidedmundson.Aug 17 2018, 8:57 PM

Thanks for looking.

I'm not convinced by the bugzilla analysis.

Imho we shouldn't be updating the state (as far as effects are concerned) to maximised till we get the ack configure.

Which should be synced to the surface commit which comes with the new buffer size.

And if you did want to assume maximised state after set, we may as well assume size ahead of time as we need to pass that to the configure request anyway.

zzag added a comment.Aug 19 2018, 7:51 PM
In D14911#310976, @davidedmundson wrote:

Thanks for looking.

I'm not convinced by the bugzilla analysis.

Imho we shouldn't be updating the state (as far as effects are concerned) to maximised till we get the ack configure.

Which should be synced to the surface commit which comes with the new buffer size.

I'd would prefer the proposed solution. If I understand it correctly, when windowMaximizedSTateChanged signal is emitted, we can start animation.

zzag planned changes to this revision.Aug 29 2018, 8:12 AM

More useful comments.

zzag updated this revision to Diff 40657.Aug 29 2018, 4:51 PM

Edit comments

zzag added a comment.EditedAug 29 2018, 4:58 PM

Probably, I have to edit the summary because it's bad.

zzag abandoned this revision.Aug 29 2018, 9:27 PM

No need for this ugly hack. See D15150

Revision Contents
Changeset List

Diff 40657

View Options

effects/maximize/package/contents/code/maximize.js

Loading...
View Options

libkwineffects/kwineffects.h

Loading...
View Options

libkwineffects/kwineffects.cpp

Loading...