In general I think all the if (workspace()->compositor()) checks can be removed as there is always a Compositor instance.
What I would like to see as a change would be in utils.h() for compositing() to not check for scene != NULL, but:
bool compositing()
{
return Workspace::self()->compositor()->isActive();
}
As a follow-up work you could try moving the ownership of scene completely to the Compositor class and make it non-static.
|
kwin/bridge.cpp
(Diff revision 2)
|
void Bridge::grabXServer(bool grab) |
| 202 |
return c->workspace()->compositingActive(); |
203 |
if (c->workspace()->compositor()) { |
|
|
204 |
return c->workspace()->compositor()->compositingActive(); |
|
|
205 |
} |
|
|
206 |
return false; |
could you please change that to:
return compositing;
and include util.h
|
kwin/client.cpp
(Diff revision 2)
|
void Client::resizeDecorationPixmaps() |
| 573 |
if (workspace()->compositingActive() && effects->compositingType() == OpenGLCompositing) { |
574 |
if (workspace()->compositor() && workspace()->compositor()->compositingActive() && effects->compositingType() == OpenGLCompositing) { |
please change those also to if (compositing())
|
kwin/client.cpp
(Diff revision 2)
|
void Client::internalShow(allowed_t) |
| 1188 |
workspace()->checkUnredirect(); |
1189 |
if (workspace()->compositor()) { |
|
|
1190 |
workspace()->compositor()->checkUnredirect(); |
|
|
1191 |
} |
Is it possible at all that compositor is NULL? As far as I see we always have a compositor instance and it may just not be active, right? So all those checks could probably be removed
new name: isActive() and please const
what's the difference to toggleCompositing()?
|
kwin/composite.h
(Diff revision 2)
|
|
public: |
|
|
58 |
void suspendCompositing(); |
|
|
59 |
void suspendCompositing(bool suspend); |
new name: suspend()
|
kwin/composite.h
(Diff revision 2)
|
|
public: |
|
|
62 |
void compositingToggled(bool active); |
new name: toggled(bool active)
|
kwin/composite.h
(Diff revision 2)
|
|
public: |
|
|
69 |
bool compositingSuspended, compositingBlocked; |
|
|
70 |
QBasicTimer compositeTimer; |
|
|
71 |
KSelectionOwner* cm_selection; |
|
|
72 |
uint vBlankInterval, vBlankPadding, fpsInterval, estimatedRenderTime; |
|
|
73 |
int m_xrrRefreshRate; |
|
|
74 |
qint64 nextPaintReference; |
|
|
75 |
QTimer mousePollingTimer; |
|
|
76 |
QRegion repaints_region; |
|
|
77 |
Workspace* m_workspace; |
|
|
78 |
|
|
|
79 |
QTimer unredirectTimer; |
|
|
80 |
bool forceUnredirectCheck; |
|
|
81 |
QTimer compositeResetTimer; // for compressing composite resets |
|
|
82 |
bool m_finishingCompositing; // finishCompositing() sets this variable while shutting down |
Could you please adjust the naming scheme, when moving to a new class? m_variableName please :-)
|
kwin/composite.cpp
(Diff revision 2)
|
void Workspace::performCompositing() |
void Compositor::performCompositing() |
| 417 |
checkCursorPos(); |
463 |
m_workspace->checkCursorPos(); |
Please check whether checkCursorPos() is called from anywhere else than the compositor. If no, please move here.
|
kwin/geometry.cpp
(Diff revision 2)
|
void Workspace::desktopResized() |
| 82 |
if (compositing()) |
83 |
if (compositing() && m_compositor) |
| 83 |
compositeResetTimer.start(0); |
84 |
m_compositor->setCompositeResetTimer(0); |
looks like a redundant check :-)
- Martin
On August 24th, 2011, 1:37 p.m., Arthur Arlt wrote:
|
Review request for kwin.
By Arthur Arlt.
Updated Aug. 24, 2011, 1:37 p.m.
Description
This patch moves all the Workspace functionality implemented in file composite.cpp to its own class Compositor. A new header file composite.h was created as well. All function calls were updated.
Maybe the shortcut set to the slot 'slotToggleCompositing' in kwinbinding.cpp could be connected directly to the slot in class Compositor. At the moment it is redirected from Workspace.
|
Testing
Compiles and runs so far. Compositing works. However, I did not test it very intensively. Please report any inconsistencies. Thx.
|
Diffs
- kwin/activation.cpp (911e9e6)
- kwin/bridge.cpp (06dde55)
- kwin/client.cpp (9d86cc4)
- kwin/composite.h (PRE-CREATION)
- kwin/composite.cpp (2b8ea6f)
- kwin/effects.cpp (bbf5a45)
- kwin/events.cpp (dd2c3a4)
- kwin/geometry.cpp (b518ae8)
- kwin/layers.cpp (3ce9903)
- kwin/toplevel.cpp (8ec1b82)
- kwin/workspace.h (2cba848)
- kwin/workspace.cpp (9aa259c)
View Diff
|