Declare it in the source file for internal usage of ApplicationX11 and
Compositor classes. Additionally use modern style connect and do other
code style updates.
Details
- Reviewers
zzag - Group Reviewers
KWin - Maniphest Tasks
- T11071: Rework compositing pipeline
- Commits
- R108:ead1b3456313: KSelectionOwner usage in ApplicationX11 and Compositor classes
Manually in X session and Wayland windowed.
Diff Detail
- Repository
- R108 KWin
- Branch
- compStyle1
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 13127 Build 13145: arc lint + arc unit
Hmm, I wonder whether Application or Platform could provide/create desired Compositor instance. So, there's would be generic Compositor class and X11 specific CompositorX11 class with all that manager selection headache.
Do you mean the same thing as I did in the first "Structure" idea in T11071? As in:
Split up Compositor class into X and Wayland subclasses.
composite.cpp | ||
---|---|---|
72–92 | It's perfectly fine to keep declaration of CompositorSelectionOwner class in the header file. It's also worth to point out that KSelectionOwner is a QObject, which means CompositorSelectionOwner needs to have a Q_OBJECT macro. | |
main_x11.h | ||
27–40 | I don't see why we need to move this class to the cpp file. It can stay here. |
composite.cpp | ||
---|---|---|
72–92 | It's a scope question. CompositorSelectionOwner is only used internally by Compositor class while the header is included in many other source files. When CompositorSelectionOwner in the header this is not obvious, when it's in the source file it is. In regards to the macro: according to docs it is only needed when the class uses slots and signals. Well it does so why does it compile? Would it just fail at runtime? Alternatives are to put the CompositorSelectionOwner class in a separate file or include the moc in the composite.cpp file. |
Well it does so why does it compile? Would it just fail at runtime?
Adding and emitting a signal in this subclass will fail at compile time.
Anything that uses metaObject will fail at runtime. Old style connects to new slots won't work silently and it used to be a common infuriating trap to debug.
composite.cpp | ||
---|---|---|
72–92 |
Not really, you need Q_OBJECT for other things too. In general, it's much easier to add Q_OBJECT and not worry about it. |
I still don't see why KWinSelectionOwner and CompositorSelectionOwner have to be defined in cpp files. They can stay in header files.
I reasoned for this in a reply to your inline comment already. If you don't agree with the reason brought for you can argument against it.