From kwin Thu Aug 30 13:16:25 2012 From: =?utf-8?q?Thomas_L=C3=BCbking?= Date: Thu, 30 Aug 2012 13:16:25 +0000 To: kwin Subject: Re: Review Request: Remove superfluous Compositor checks in events.cpp Message-Id: <20120830131625.23426.76370 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kwin&m=134633260024236 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============3895484399931102823==" --===============3895484399931102823== Content-Type: multipart/alternative; boundary="===============7795268208839661038==" --===============7795268208839661038== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Aug. 28, 2012, 7:20 p.m., Thomas L=C3=BCbking wrote: > > One could probably override the "->" operator to return NULL function p= ointers what makes them noop and also returns false (::isActive() etc.) but= i'd have to try myself, so i'm not sure whether that works. <- Code Fu mas= turbation ;-) > > = > > Another option was to make ::isActive (and some other functions) static= as well. *shrug* > = > Martin Gr=C3=A4=C3=9Flin wrote: > I first wanted to do the ::isActive but decided then to add just the = ::isCreated as it's kind of strange to make everything static. But override= the operator-> sound interesting but also a little bit academic ;-) The hidden danger in overriding operator::-> is that any function that shou= ld default to anything but false/0 will depending on the compositor instanc= e default to NULL instead. Given that ::isActive will be the most interesting condition for "external"= callers and the return in absence of a compositor is defined, i'd vote to = make at least that one static next to ::isCreated - and leave wonky pointer= experiments to things with a lower amount of users ;-) - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106255/#review18179 ----------------------------------------------------------- On Aug. 30, 2012, 9:23 a.m., Martin Gr=C3=A4=C3=9Flin wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106255/ > ----------------------------------------------------------- > = > (Updated Aug. 30, 2012, 9:23 a.m.) > = > = > Review request for kwin. > = > = > Description > ------- > = > Remove superfluous Compositor checks in events.cpp > = > compositing() ensures that m_compositor is not null. > = > Make the Compositor a proper Singleton > = > The Compositor class actually behaves like a Singleton so it should be > one. Therefore three static methods are added: > * self() to access the Singleton > * createCompositor() to be used by Workspace to create the instance > * isCreated() to have a simple check whether the Singleton is already > created > = > The isCreated() check is actually required as especially Clients might > be created and trying to access the Compositor before it is setup. > = > = > Diffs > ----- > = > kwin/bridge.cpp 670063e2276a506e0b3f3e29d2fef3b946032f82 = > kwin/client.cpp 569494418117f3b987f05c844debac48a88ed05d = > kwin/composite.h a4a37107ffe4981e9758c9ff65779c646ae94ffb = > kwin/composite.cpp 640ebd6a3c09e82492d79b9b2d68d752d6a0c62c = > kwin/events.cpp 23e1921bfa653fcf399378c0328733fb996b9d1f = > kwin/geometry.cpp 19a911d097c3fd87da28ede9dc9fd362310146cc = > kwin/paintredirector.cpp 55b20c4f4eae2723df46e92654c851e912cd5008 = > kwin/workspace.h 3f4bd9f93a27c832fef913e5fdbcc24518fadb4d = > kwin/workspace.cpp e03e5e765bea8d098bd86dd5118198ddccd8fd8d = > = > Diff: http://git.reviewboard.kde.org/r/106255/diff/ > = > = > Testing > ------- > = > = > Thanks, > = > Martin Gr=C3=A4=C3=9Flin > = > --===============7795268208839661038== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/106255/

On August 28th, 2012, 7:20 p.m., Thomas L= =C3=BCbking wrote:

One could=
 probably override the "->" operator to return NULL function p=
ointers what makes them noop and also returns false (::isActive() etc.) but=
 i'd have to try myself, so i'm not sure whether that works. <- =
Code Fu masturbation ;-)

Another option was to make ::isActive (and some other functions) static as =
well. *shrug*

On August 28th, 2012, 8:16 p.m., Martin Gr=C3=A4=C3=9Flin wrote:= 

I first w=
anted to do the ::isActive but decided then to add just the ::isCreated as =
it's kind of strange to make everything static. But override the operat=
or-> sound interesting but also a little bit academic ;-)
The hidden =
danger in overriding operator::-> is that any function that should defau=
lt to anything but false/0 will depending on the compositor instance defaul=
t to NULL instead.

Given that ::isActive will be the most interesting condition for "exte=
rnal" callers and the return in absence of a compositor is defined, i&=
#39;d vote to make at least that one static next to ::isCreated - and leave=
 wonky pointer experiments to things with a lower amount of users ;-)

- Thomas


On August 30th, 2012, 9:23 a.m., Martin Gr=C3=A4=C3=9Flin wrote:

Review request for kwin.
By Martin Gr=C3=A4=C3=9Flin.

Updated Aug. 30, 2012, 9:23 a.m.

Descripti= on 

Remove superfluous Compositor checks in events.cpp

compositing() ensures that m_compositor is not null.

Make the Compositor a proper Singleton

The Compositor class actually behaves like a Singleton so it should be
one. Therefore three static methods are added:
* self() to access the Singleton
* createCompositor() to be used by Workspace to create the instance
* isCreated() to have a simple check whether the Singleton is already
  created

The isCreated() check is actually required as especially Clients might
be created and trying to access the Compositor before it is setup.

Diffs=

  • kwin/bridge.cpp (670063e2276a506e0b3f3e29d= 2fef3b946032f82)
  • kwin/client.cpp (569494418117f3b987f05c844= debac48a88ed05d)
  • kwin/composite.h (a4a37107ffe4981e9758c9ff= 65779c646ae94ffb)
  • kwin/composite.cpp (640ebd6a3c09e82492d79b= 9b2d68d752d6a0c62c)
  • kwin/events.cpp (23e1921bfa653fcf399378c03= 28733fb996b9d1f)
  • kwin/geometry.cpp (19a911d097c3fd87da28ede= 9dc9fd362310146cc)
  • kwin/paintredirector.cpp (55b20c4f4eae2723= df46e92654c851e912cd5008)
  • kwin/workspace.h (3f4bd9f93a27c832fef913e5= fdbcc24518fadb4d)
  • kwin/workspace.cpp (e03e5e765bea8d098bd86d= d5118198ddccd8fd8d)

View Diff
--===============7795268208839661038==-- --===============3895484399931102823== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kwin mailing list kwin@kde.org https://mail.kde.org/mailman/listinfo/kwin --===============3895484399931102823==--