Split up fullscreen able check into AbstractClient subclasses
ClosedPublic

Authored by romangg on Jan 9 2019, 4:02 PM.

Details

Summary

Most parts of this function are only relevant for X clients, in particular
the "fullscreen hack". Therefore split up the function into the AbstractClient
subclasses.

Test Plan

Manually and autotests still pass.

Diff Detail

Repository
R108 KWin
Branch
refactorIsFullScreenAble
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6864
Build 6882: arc lint + arc unit
romangg created this revision.Jan 9 2019, 4:02 PM
Restricted Application added a project: KWin. · View Herald TranscriptJan 9 2019, 4:02 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Jan 9 2019, 4:02 PM
romangg updated this revision to Diff 49088.Jan 9 2019, 4:36 PM
  • Make isFullScreenable pure virtual
romangg retitled this revision from Refactor fullscreen ablement check into AbstractClient subclasses to Split up fullscreen able check into AbstractClient subclasses.Jan 9 2019, 7:35 PM
zzag added a subscriber: zzag.Jan 9 2019, 8:20 PM
zzag added inline comments.
client.cpp
590–596

We lose this path in ShellClient::isFullScreenable. Is there a reason for thta?

romangg added inline comments.Jan 9 2019, 8:34 PM
client.cpp
590–596

Wayland native clients can't dictate the compositor if fullscreen is possible or not. so we don't need to check if their geometry is restricted. At least the call to AbstractClient::sizeForClientSize in the case of ShellClient in the current version on master would imply this.

zzag added inline comments.Jan 9 2019, 9:30 PM
client.cpp
590–596

As far as I know, it depends on compositor's policies, e.g. if max size was set for a xdg-toplevel, then compositor can disallow the fullscreen state, though this is probably unrelated. I just wanted to make sure you did this on purpose.

shell_client.cpp
951 ↗(On Diff #49088)

Should popups be fullscreen-able?

romangg added inline comments.Jan 9 2019, 10:04 PM
shell_client.cpp
951 ↗(On Diff #49088)

Wouldn't say so.

zzag added inline comments.Jan 11 2019, 10:30 AM
shell_client.cpp
951 ↗(On Diff #49088)

Then we probably have to check isPopupWindow.

zzag accepted this revision.Jan 11 2019, 10:32 AM

Looks good to me. Please land it in 5.16.

shell_client.cpp
951 ↗(On Diff #49088)

Though it should probably go in D18132.

This revision is now accepted and ready to land.Jan 11 2019, 10:32 AM

Looks good to me

romangg updated this revision to Diff 59802.Jun 14 2019, 4:23 PM

Rebase on master.

This revision was automatically updated to reflect the committed changes.