Review Request: Drop Xinerama related options in KWin

[Martin Gräßlin]

> On Jan. 22, 2012, 7:54 p.m., Thomas Lübking wrote:
> > kwin/geometry.cpp, line 250
> > <http://git.reviewboard.kde.org/r/103756/diff/1/?file=47685#file47685line250>
> >
> >     should possibly be
> >     
> >     if (is_multihead)
> >        screen = screen_number;
> >     else if (screen == -1)
> >      ....??
> >     
> >     is screen_number part of the "make kwin work on real multihead" thing? looks like it's statically assigned to DefaultScreen(dpy) in main.cpp what does not cover the case when one head has multiple (randr) screens?!

I'm not sure why it is that way. Last commit which changed it: "Replace -1 with active screen in clientArea() if needed."
So no idea whether this is related to multi head at all. -1 means active screen, I think.
> does not cover the case when one head has multiple (randr) screens?!
I think it is a save assumption to say that someone using multi head won't use xrandr as well. Even if not I think we can make that a limitation. If someone wants multi head he has to live with it.


> On Jan. 22, 2012, 7:54 p.m., Thomas Lübking wrote:
> > kwin/geometry.cpp, line 255
> > <http://git.reviewboard.kde.org/r/103756/diff/1/?file=47685#file47685line255>
> >
> >     not your change, but looks like senseless code duplication.
> >     esp: why does the is_multihead block test "screen < screenarea[desktop].size()" but uses screenarea[desktop][screen_number]?
> >     
> >     Otherwise the blocks are equal but for screen/screen_number

will keep that for now as well and probably have a more close look on why we do all these calculations in the first place. Seems all a bit redundant.


> On Jan. 22, 2012, 7:54 p.m., Thomas Lübking wrote:
> > kwin/geometry.cpp, line 283
> > <http://git.reviewboard.kde.org/r/103756/diff/1/?file=47685#file47685line283>
> >
> >     since we acquire sarea/warea above (including some sanity check), can't we just return it here?
> >     
> >     Otherwise it maybe shouldn't be calculated / fetched against screenarea[desktop] unconditionally but just for MaximizeArea & resp. WorkArea

will keep it for now as well.


> On Jan. 22, 2012, 7:54 p.m., Thomas Lübking wrote:
> > kwin/manage.cpp, line 229
> > <http://git.reviewboard.kde.org/r/103756/diff/1/?file=47686#file47686line229>
> >
> >     afaics this option is dead.
> >     I've that key nowhere in no config and it wasn't provided by the xinerama kcm either

yes, you are right, it's nowhere set and I dropped it.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103756/#review10006
-----------------------------------------------------------


On Jan. 22, 2012, 10:21 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103756/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2012, 10:21 a.m.)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Description
> -------
> 
> As discussed on the mailinglist: drop of the xinerama related options and the kcm. Given that the show unmanaged windows on option is in fact not used, I dropped the complete KCM.
> 
> 
> Diffs
> -----
> 
>   kcontrol/CMakeLists.txt 8cd9a46 
>   kcontrol/xinerama/CMakeLists.txt fe332e5 
>   kcontrol/xinerama/Messages.sh f4aa134 
>   kcontrol/xinerama/interface_util.h 8a4fcd1 
>   kcontrol/xinerama/kcmxinerama.h 18b9241 
>   kcontrol/xinerama/kcmxinerama.cpp a456b2c 
>   kcontrol/xinerama/test_kcm_xinerama.cpp a358a2c 
>   kcontrol/xinerama/xinerama.desktop e8ce525 
>   kcontrol/xinerama/xineramawidget.h 2c446a2 
>   kcontrol/xinerama/xineramawidget.cpp df9cb2f 
>   kcontrol/xinerama/xineramawidget.ui 90ec4d4 
>   kwin/geometry.cpp a414e26 
>   kwin/manage.cpp c551eac 
>   kwin/options.h 9dc29cf 
>   kwin/options.cpp d496569 
>   kwin/tabbox/tabbox.cpp 3051316 
>   kwin/toplevel.cpp ffe7f0c 
>   kwin/workspace.cpp 69b4ecb 
> 
> Diff: http://git.reviewboard.kde.org/r/103756/diff/diff
> 
> 
> Testing
> -------
> 
> Fullscreen: works
> Maximize: works
> Movment: works
> Placement: works
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20120126/2fcfb443/attachment.html>