Move non-Compositor functions out of composite.cpp source file
ClosedPublic

Authored by romangg on Jun 7 2019, 7:32 PM.

Details

Summary

Compositing today is ubiquitous. There is no reason to keep compositing
specific functions of Toplevel, Client and Workspace classes in the
composite.cpp source file. Instead let these definitions be separated.

Test Plan

Compiles

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.
romangg created this revision.Jun 7 2019, 7:32 PM
Restricted Application added a project: KWin. · View Herald TranscriptJun 7 2019, 7:32 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Jun 7 2019, 7:32 PM
zzag added a subscriber: zzag.EditedJun 11 2019, 10:35 AM

In general, +1, but this sort of changes raises a dilemma: do we want to keep git history or have tidy code? If we do same thing for activation.cpp and geometry.cpp a lot of git history will be lost. I think we have to keep history of geometry.cpp because geometry handling in kwin is super weird.

I have some coding style nitpicks.

composite.cpp
38

This include directive should be before line 33.

58

QQuickWindow has to go before QTextStream.

63

stdio.h is deprecated in C++, include cstdio instead.

toplevel.cpp
20

Unrelated whitespace change.

In D21654#478038, @zzag wrote:

In general, +1, but this sort of changes raises a dilemma: do we want to keep git history or have tidy code? If do same thing for activation.cpp and geometry.cpp a lot of git history will be lost. I think we have to keep history of geometry.cpp because geometry handling in kwin is super weird.

In my opinion tidy code is more important than git history in every aspect. This mantra of not changing git history too much blocks us unnecessarily of moving KWin forward. Also geometry.cpp needs at some point in the future some major structural changes making most of its previous git history difficult or impossible to be understood.

zzag accepted this revision.Jun 11 2019, 10:58 AM

This code doesn't contain any magic so I guess we can move all that code where it belongs. Please wait for another "Ship it" from KWin because this is not one man decision.

Also, don't forget to address my nitpicks once the change is ready to go.

This revision is now accepted and ready to land.Jun 11 2019, 10:58 AM
This revision was automatically updated to reflect the committed changes.
romangg marked 3 inline comments as done.