From kde-panel-devel Sun Jul 31 15:01:43 2011 From: "Commit Hook" Date: Sun, 31 Jul 2011 15:01:43 +0000 To: kde-panel-devel Subject: Re: Review Request: Move desktop layout ownership to KWin Message-Id: <20110731150143.18093.52244 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=131212457201151 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============0039349744==" --===============0039349744== Content-Type: multipart/alternative; boundary="===============3228320619039203231==" --===============3228320619039203231== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102073/#review5274 ----------------------------------------------------------- This review has been submitted with commit 4bf7761307a2453a63d918c60e13c269= 80727830 by Martin Gr=C3=A4=C3=9Flin to branch master. - Commit On July 24, 2011, 11:01 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/102073/ > ----------------------------------------------------------- > = > (Updated July 24, 2011, 11:01 a.m.) > = > = > Review request for kwin and Plasma. > = > = > Summary > ------- > = > Even if NETWM spec says that the Desktop Layout is owned and maintained b= y a pager and not by the window manager, I disagree. It has been causing pr= oblems for years and is just wrong. Therefore this patch: > = > * moves the configuration of the number of rows from Pager Config UI to t= he Virtual Desktops KCM > * Number of rows is stored in the desktop section of KWin config file > * Virtual Desktops KCM updates the layout on change > * KWin sets the desktop layout on startup (as it sets the number of deskt= ops and names) > * Pager gets the number of rows from the NetRootInfo instead of its own c= onfig > * Pager connects to KWin's reconfigure dbus signal to update the layout (= KWindowSystem does not emit a signal) > = > This renders a major break with the NetWM spec: > * There is no process claiming the pager management selection any more > * If another pager is used together with KWin, KWin will ignore the setti= ngs of the pager > * If another window manager is used together with Plasma, the layout used= by the window manager may not reflect the layout shown in the pager > = > As I think the concept of having Pager setting the desktop layout is brok= en, I will inform the NetWM spec mailing list when this patch is merged int= o master and recommend a change in the spec. Given that both GNOME Shell an= d Unity do have a combined Window Manager/Desktop Shell combination I expec= t them to ignore this part of the spec anyway. > = > Unrelated question: what is the use case of pager in a non-X11 environmen= t and why are there ifdefs? > = > = > Diffs > ----- > = > plasma/desktop/applets/pager/pager.h c482b21 = > kwin/workspace.cpp 1a48194 = > kwin/kcmkwin/kwindesktop/main.ui e431d64 = > kwin/kcmkwin/kwindesktop/main.cpp ee5c504 = > plasma/desktop/applets/pager/pager.cpp 588a49d = > plasma/desktop/applets/pager/pagerConfig.ui 18fb7c8 = > = > Diff: http://git.reviewboard.kde.org/r/102073/diff > = > = > Testing > ------- > = > * KWin loads with the last saved desktop layout > * Changes in KCM are reflected in KWin and pager > = > Not tested: > * multiple pagers > * vertical pagers > = > = > Thanks, > = > Martin > = > --===============3228320619039203231== 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/102073/

This revie=
w has been submitted with commit 4bf7761307a2453a63d918c60e13c26980727830 b=
y Martin Gr=C3=A4=C3=9Flin to branch master.

- Commit

On July 24th, 2011, 11:01 a.m., Martin Gr=C3=A4=C3=9Flin wrote:

Review request for kwin and Plasma.

By Martin Gr=C3=A4=C3=9Flin.

Updated July 24, 2011, 11:01 a.m.

Descripti= on

Even if NETWM spec says that the Desktop Layout is owned and=
 maintained by a pager and not by the window manager, I disagree. It has be=
en causing problems for years and is just wrong. Therefore this patch:

* moves the configuration of the number of rows from Pager Config UI to the=
 Virtual Desktops KCM
* Number of rows is stored in the desktop section of KWin config file
* Virtual Desktops KCM updates the layout on change
* KWin sets the desktop layout on startup (as it sets the number of desktop=
s and names)
* Pager gets the number of rows from the NetRootInfo instead of its own con=
fig
* Pager connects to KWin's reconfigure dbus signal to update the layout=
 (KWindowSystem does not emit a signal)

This renders a major break with the NetWM spec:
* There is no process claiming the pager management selection any more
* If another pager is used together with KWin, KWin will ignore the setting=
s of the pager
* If another window manager is used together with Plasma, the layout used b=
y the window manager may not reflect the layout shown in the pager

As I think the concept of having Pager setting the desktop layout is broken=
, I will inform the NetWM spec mailing list when this patch is merged into =
master and recommend a change in the spec. Given that both GNOME Shell and =
Unity do have a combined Window Manager/Desktop Shell combination I expect =
them to ignore this part of the spec anyway.

Unrelated question: what is the use case of pager in a non-X11 environment =
and why are there ifdefs?

Testing <= /h1>

* KWin loads with the last saved desktop layout
* Changes in KCM are reflected in KWin and pager

Not tested:
* multiple pagers
* vertical pagers

Diffs=

plasma/desktop/applets/pager/pager.h (c482= b21)
kwin/workspace.cpp (1a48194)
kwin/kcmkwin/kwindesktop/main.ui (e431d64)=
kwin/kcmkwin/kwindesktop/main.cpp (ee5c504= )
plasma/desktop/applets/pager/pager.cpp (58= 8a49d)
plasma/desktop/applets/pager/pagerConfig.ui View Diff

--===============3228320619039203231==-- --===============0039349744== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel --===============0039349744==--