This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/102420/

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 !=3D 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	retur= n c->workspace= ()->compositingActive();	203	if = (c->workspace()= ->compositor()) {
		204	return c->workspa= ce()->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<= span class=3D"p">() && effects->compos= itingType() =3D=3D= OpenGLCompositing) {	574	if (workspace()->compositor() = && workspace()->compos= itor()->= compositingActive() && effects->compositingType() =3D=3D OpenGLComposi= ting) {

kwin/client.cpp (Diff revision 2)

void Client::resizeDecorationPixmaps()

573

        if (workspace()->compositingActive<=
span class=3D"p">() && effects->compos=
itingType() =3D=3D=
 OpenGLCompositing) {

574

        if (workspace()->compositor() =
&& workspace()->compos=
itor()->=
compositingActive() && effects->compositingType() =3D=3D OpenGLComposi=
ting) {

please change those also to if (compositing())

= = = =

kwin/client.cpp (Diff revision 2)
void Client::internalShow(allowed_t)
1188	workspace()->checkUnr= edirect();	1189	if (workspace()->compositor()) {
		1190	workspace()->comp= ositor()->checkUnredirect();
		1191	}

Is it possible at all that compositor is NULL? As far as I see we al=
ways have a compositor instance and it may just not be active, right? So al=
l those checks could probably be removed

= =

kwin/composite.h (Diff revision 2)
	public:
		45	void toggleCompositing();

New name: toggle()

= =

kwin/composite.h (Diff revision 2)
	public:
		50	bool compositingActive();

new name: isActive() and please const

= =

kwin/composite.h (Diff revision 2)
	public:
		57	void slotToggleCompositing();

what's the difference to toggleCompositing()?

= =


public:
	58	void suspendCompositing();
	59	void suspendCompositing(bool suspend);

new name: suspend()

= =

kwin/composite.h (Diff revision 2)
	public:
		60	void resetCompositing();

new name: reset()

= =

kwin/composite.h (Diff revision 2)
	public:
		62	void compositingToggled(bool active);

new name: toggled(bool active)

= =


public:
	69	bool compositingSuspended, compositingBlocked;
	70	QBasicTimer compositeTimer;
	71	KSelectionOwner* cm_selection;
	72	uint vBlankInterval, vB= lankPadding, fpsInterval<= /span>, estimatedRenderTime;
	73	int m_xrrRefreshRate;
	74	qint64 nextPaintReference;
	75	QTimer mousePollingTimer;
	76	QRegion repaints_region;
	77	Workspace* m_workspace;<= /span>
	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 clas=
s? m_variableName please :-)

= =

kwin/composite.cpp (Diff revision 2)
void Workspace::performCompositing()	void Compositor::performCompositing()
417	checkCursorPos();	463	m_wor= kspace->checkCursorPos(); <= /td>

Please check whether checkCursorPos() is called from anywhere else t=
han the compositor. If no, please move here.

= =

kwin/geometry.cpp (Diff revision 2)
void Workspace::desktopResized()
82	if (compositing())	83	if (compositing()= &&<= /span> m_compositor)
83	c= ompositeResetTimer.start(0);	84	m= _compositor-><= /span>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.

Descripti= on

This patch moves all the Workspace functionality implemented=
 in file composite.cpp to its own class Compositor. A new header file compo=
site.h was created as well. All function calls were updated.
Maybe the shortcut set to the slot 'slotToggleCompositing' in kwinb=
inding.cpp could be connected directly to the slot in class Compositor. At =
the moment it is redirected from Workspace.

Testing <= /h1>

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

Descripti= on

Testing <= /h1> Compiles and runs so far. Compositing works. However, I did = not test it very intensively. Please report any inconsistencies. Thx.

Diffs=

Testing <= /h1>

Compiles and runs so far. Compositing works. However, I did = not test it very intensively. Please report any inconsistencies. Thx.