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

= =

kwin/composite.cpp (Diff revision 1)
void Toplevel::suspendUnredirect(bool suspend)
	800	// TODO: rename decorationPixmapsResized t= o something useful
	801	connect(decorationPaintRedirector, SIGNAL(decorationPixmapsResized()),= this, SLOT(triggerDecora= tionRepaint()));

kwin/composite.cpp (Diff revision 1)

void Toplevel::suspendUnredirect(bool suspend)

800

        // TODO: rename decorationPixmapsResized t=
o something useful

801

        connect(decorationPaintRedirector, SIGNAL(decorationPixmapsResized()),=
 this, SLOT(triggerDecora=
tionRepaint()));

*cough* :-)

= =


class EffectWindowImpl;
	33	class DecorationPaintRedirector
	34	: public PaintRedirector

please add some documentation to the class

= =


class EffectWindowImpl;
	41	bool decorationPixmapRequiresRepaint();
	42	void ensureDecorationPixmapsPainted();
	43
	44	enum CoordinateMode {
	45	DecorationRelative, // Relative to the top l= eft corner of the decoration
	46	WindowRelative // Relative to the top left corner of the window<= /span>
	47	};
	48
	49	// Decorations <->= ; Effects
	50	const QPixmap *topDecoPi= xmap() const {
	51	return &decorationPixmapTop;
	52	}
	53	const QPixmap *leftDecoP= ixmap() const {
	54	return &decorationPixmapLeft;
	55	}
	56	const QPixmap *bottomDec= oPixmap() const {
	57	return &decorationPixmapBottom;
	58	}
	59	const QPixmap *rightDeco= Pixmap() const {
	60	return &decorationPixmapRight;

could we please drop a bunch of Deco and Pixmaps here? In Client tha=
t was needed, but I would like:

bool requiresRepaint() const
void ensurePainted() const

const QPixmap *top() const

and so on.

= =


class EffectWindowImpl;
	78	QPixmap decorationPixmapLeft, decorationPixmapRight, decorationPixmapTop, decorationPixmapBottom;
	79	// we (instead of Qt) i= nitialize the Pixmaps, and have to free them

please use consistent naming: m_variableName

= =

kwin/decorationpaintredirector.cpp (Diff revision 1)
	DecorationPaintRedirector::~DecorationPaintRedirector()
		49	kDebug(1212) << "DecorationPaintRedir= ector killed!!";

please drop the debug statement - not needed.

= =


DecorationPaintRedirector::~DecorationPaintRedirector()
	231	// if (Client* c =3D qo= bject_cast< Client* >(((Scene::Window*)parent())->window())) {
	232	// c->layoutDeco= rationRects(left, top, right, bottom, mode);
	233	// }

why is here the commented code?

= = = =

kwin/deleted.cpp (Diff revision 1)
void Deleted::copyToDeleted(Toplevel* c)
		82	#if 0
82	if (!no_border) {	83	if (!no_border) {
83	client->layoutDecorationRects(decoration_left,	84	client->layou= tDecorationRects(decoration_left,
84	decoration_top,	85	= decoration_top,
85	decoration_right,	86	= decoration_right,
86	decoration_bottom,	87	= decoration_bottom,
87	Client::Win= dowRelative);	88	= Client::WindowRelative);
88	decorationPixmap= Left =3D *client->left= DecoPixmap();	89	decorationPixmap= Left =3D *client->leftDecoPixmap();
89	decorationPixmap= Right =3D *client->rig= htDecoPixmap();	90	decorationPixmap= Right =3D *client->rightDecoPixmap();
90	decorationPixmap= Top =3D *client->topDe= coPixmap();	91	decorationPixmap= Top =3D *client->topDecoPixmap();
91	decorationPixmap= Bottom =3D *client->bo= ttomDecoPixmap();	92	decorationPixmap= Bottom =3D *client->bottomDecoPixmap();
92	}	93	}

please remove :-)

= =

kwin/scene.cpp (Diff revision 1)
Scene::Window::Window(Toplevel * c)
		422	void Scene::Window::decorationChanged(KDecoration* decoration, NETWinInfo2* i= nfo)

I don't like the NETWinInfo2 here at all. I will try to think ab=
out how we can get rid off it.

= =

kwin/scene.cpp (Diff revision 1)
Scene::Window::Window(Toplevel * c)
		426	kDebug(1212) << "NEW DECORATI= ONPAINTREDIRECTOR";

Please remove the debug statement

= =

kwin/scene.cpp (Diff revision 1)
Scene::Window::Window(Toplevel * c)
	428	}
	429	else {

coding style:
if () {
} else {
}

- Martin

On August 24th, 2011, 3:48 p.m., Arthur Arlt wrote:

Review request for kwin.

By Arthur Arlt.

Updated Aug. 24, 2011, 3:48 p.m.

Descripti= on

This patch introduces the new class DecorationPaintRedirecto=
r. It inherits from PaintRedirector and covers the functionality for decora=
tion handling when compositing is active. This functionality was moved from=
 Client to this new class.
The reason for doing this is to get rid of the Client dependency in class S=
cene.
This is an initial review request. There are still some Client dependencies=
 left. But it works so far except for changing the decoration in composited=
 mode or removing no border. Doing this does not lead to a crash but the de=
corations are not updated. Newly added clients are displayed correctly with=
 the new decoration. Should be not that hard to fix it; will be done the ne=
xt days ;)

Testing <= /h1>

Works for both OpenGL and XRender on my system. However, as =
told in the description, changing the decoration (in composited mode)does n=
ot lead to a crash, but does not update the decorations for existing client=
s. Feel free to tell me the solution if it is obvious to you.

Diffs=

kwin/CMakeLists.txt (bde6626)
kwin/client.h (d9cdddf)
kwin/client.cpp (9d86cc4)
kwin/composite.cpp (2b8ea6f)
kwin/decorationpaintredirector.h (PRE-CREA= TION)
kwin/decorationpaintredirector.cpp (PRE-CR= EATION)
kwin/deleted.cpp (f465159)
kwin/paintredirector.h (bb5d67c)
kwin/paintredirector.cpp (7ed4d6f)<= /li>
kwin/scene.h (6ce39c0)
kwin/scene.cpp (8509400)
kwin/scene_opengl.cpp (cf3eb65)
kwin/scene_xrender.cpp (73dd22f)

View Diff

Descripti= on

Testing <= /h1> Works for both OpenGL and XRender on my system. However, as = told in the description, changing the decoration (in composited mode)does n= ot lead to a crash, but does not update the decorations for existing client= s. Feel free to tell me the solution if it is obvious to you.

Diffs=

Testing <= /h1>

Works for both OpenGL and XRender on my system. However, as = told in the description, changing the decoration (in composited mode)does n= ot lead to a crash, but does not update the decorations for existing client= s. Feel free to tell me the solution if it is obvious to you.