[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request 103948: Remove "Display borders on maximized windows"
From: Martin_Gräßlin <mgraesslin () kde ! org>
Date: 2013-03-15 12:27:18
Message-ID: 20130315122718.24663.56491 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Feb. 25, 2013, 1:53 p.m., Martin Gräßlin wrote:
> > kwin/libkdecorations/kdecorationbridge.h, line 51
> > <http://git.reviewboard.kde.org/r/103948/diff/4/?file=115266#file115266line51>
> >
> > do we need to create a KDecorationBridge2 or is it safe to do non-ABI changes in \
> > the bridge since Compiz is no issue any more?
>
> Thomas Lübking wrote:
> Towards the decoration plugins this is safe.
>
> Whether this is gonna introduce an ABI break for the kde4 decorator for compiz \
> depends on whether it actually includes this header. If yes, it has to implement \
> the new virtual and previously compiled versions linking libkdecorations will get \
> an ABI crash for this.
> If the decorator brings its own deco interface implementation, there's no problem.
>
> I guess i'll have to lookup the decorator sources, but that would be a quite crap \
> situation because the decobridge would exist for no more any reason (being the glue \
> between the core and the deco API)
> Thomas Lübking wrote:
> No, that's gonna break the decorator at least by ABI :-(
>
> =>
> - "unstable" class (grrr)
> - moc
> - well, "SEP"... (ie. inform compiz developers)
>
>
> Martin Gräßlin wrote:
> given:
>
> // This class is supposed to keep binary compatibility, just like KDecoration.
> // Extending should be done the same way, i.e. inheriting KDecorationBridge2 from \
> it // and adding new functionality there.
>
> the right approach would be to add another bridge class or add it to unstable, but \
> adding to unstable causes the same issues. I guess the best approach would be to \
> use moc and add nice // FIXME: Qt 5 to it ;-)
> Thomas Lübking wrote:
> git show 4933f08a
>
> "Make KDecorationBridge also public and with kept binary compatibility,
> so that Compiz can use it for the KDE decorator. Extending will be
> done the same way like with KDecoration."
>
> -> meeh!
> This means we cannot effectively break ABI compatibility (i actually wanted to \
> remove kdecorationunstable for 4.11) and rely on the ABI tag for this unless the \
> compiz decorator adds the ABI check and even then we can't because the decorator \
> itself will loose ABI compatibility.
> We'll have to rely on moc for *every* further change since we cannot operate on \
> some version check in this direction.
> Martin Gräßlin wrote:
> only till Qt 5. Then I would clearly vote for bridge does not need to be ABI \
> stable.
> Martin Gräßlin wrote:
> I have been thinking about it. Given that Compiz is more or less an Ubuntu-only \
> thing, I don't think it's worth the effort to keep compatibility. I'll recheck \
> which distros still provide it and do an RFC whether we should remove support.
fun stuff: not even Ubuntu is shipping support for kde-window-decorator. So no reason \
to keep support.
- Martin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103948/#review28010
-----------------------------------------------------------
On Feb. 24, 2013, 11:36 p.m., Thomas Lübking wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103948/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2013, 11:36 p.m.)
>
>
> Review request for kwin, Martin Gräßlin and Hugo Pereira Da Costa.
>
>
> Description
> -------
>
> Actually the only thing the patch does not is what's stated in the summary ;-)
>
> - The setting is ignored, the decoration always gets a "true" for it
> - moving a maximized window requires breaking a "strong" snap (1/16 of screen \
> height - unless you use quick maximization)
> - all snapping is done towards the client, not the frame
> - QuickTileMode is exported to the decoration (just as the maximizeMode) so that it \
> can fix the bordersize alongside that. (I've a sample implementation in local \
> Bespin as inspiration for other decos)
> Ratio:
> It's simply not possible to perform the specialstate snapping in the core.
> Either the decoration gets clipped, but that also clips titlebar buttons -> fail
> Or the decoration gets resized "under the hood" (ie. we override border_* after \
> obtaining it) but that has bad visual impact on decorations like plastik (where the \
> client visually overflows the border, it clearly looks like the client is patched \
> onto the deco) -> fail
> -> the deco is informed about those special states and can drop some of its borders \
> or not. Should be no major problem for oxygen (since it blends into clients anyway) \
> but i've not checked how to make use of that in aurorae (or even its client code)
>
> This addresses bug 299245.
> http://bugs.kde.org/show_bug.cgi?id=299245
>
>
> Diffs
> -----
>
> kwin/workspace.h e033ac9
> kwin/libkdecorations/kdecoration_p.cpp 5b54369
> kwin/libkdecorations/kdecorationbridge.h 2cb36c9
> kwin/libkwineffects/kwinglobals.h dba4324
> kwin/kcmkwin/kwindecoration/preview.cpp 94aacba
> kwin/libkdecorations/kdecoration.h 2c20767
> kwin/libkdecorations/kdecoration.cpp 7ccbdb6
> kwin/libkdecorations/kdecoration_p.h 71833cd
> kwin/bridge.h 35efc90
> kwin/bridge.cpp 31d285e
> kwin/client.h 0c9d1d2
> kwin/client.cpp f2338d4
> kwin/geometry.cpp 372a4c8
> kwin/kcmkwin/kwindecoration/preview.h 420e302
>
> Diff: http://git.reviewboard.kde.org/r/103948/diff/
>
>
> Testing
> -------
>
> With the (still local) changes to bespin I get nice partial clipping for quicktiled \
> (or partially maximized) windows. Helped me to clean up some bespin deco code as \
> well =)
>
> Thanks,
>
> Thomas Lübking
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/103948/">http://git.reviewboard.kde.org/r/103948/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On February 25th, 2013, 1:53 p.m. CET, <b>Martin \
Gräßlin</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/103948/diff/4/?file=115266#file115266line51" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/libkdecorations/kdecorationbridge.h</a> <span style="font-weight: \
normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class \
KDecorationBridge : public KDecorationDefines</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">51</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">virtual</span> <span class="n">QuickTileMode</span> <span \
class="n">quickTileMode</span><span class="p">()</span> <span class="k">const</span> \
<span class="o">=</span> <span class="mi">0</span><span class="p">;</span></pre></td> \
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">do we need to create a \
KDecorationBridge2 or is it safe to do non-ABI changes in the bridge since Compiz is \
no issue any more?</pre> </blockquote>
<p>On February 25th, 2013, 3:43 p.m. CET, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Towards the decoration \
plugins this is safe.
Whether this is gonna introduce an ABI break for the kde4 decorator for compiz \
depends on whether it actually includes this header. If yes, it has to implement the \
new virtual and previously compiled versions linking libkdecorations will get an ABI \
crash for this.
If the decorator brings its own deco interface implementation, there's no \
problem.
I guess i'll have to lookup the decorator sources, but that would be a quite crap \
situation because the decobridge would exist for no more any reason (being the glue \
between the core and the deco API)</pre> </blockquote>
<p>On February 25th, 2013, 4:03 p.m. CET, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No, that's gonna \
break the decorator at least by ABI :-(
=>
- "unstable" class (grrr)
- moc
- well, "SEP"... (ie. inform compiz developers)
</pre>
</blockquote>
<p>On February 25th, 2013, 4:19 p.m. CET, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">given:
// This class is supposed to keep binary compatibility, just like KDecoration.
// Extending should be done the same way, i.e. inheriting KDecorationBridge2 from it
// and adding new functionality there.
the right approach would be to add another bridge class or add it to unstable, but \
adding to unstable causes the same issues. I guess the best approach would be to use \
moc and add nice // FIXME: Qt 5 to it ;-)</pre> </blockquote>
<p>On February 25th, 2013, 8:47 p.m. CET, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">git show 4933f08a
"Make KDecorationBridge also public and with kept binary compatibility,
so that Compiz can use it for the KDE decorator. Extending will be
done the same way like with KDecoration."
-> meeh!
This means we cannot effectively break ABI compatibility (i actually wanted to remove \
kdecorationunstable for 4.11) and rely on the ABI tag for this unless the compiz \
decorator adds the ABI check and even then we can't because the decorator itself \
will loose ABI compatibility.
We'll have to rely on moc for *every* further change since we cannot operate on \
some version check in this direction.</pre> </blockquote>
<p>On February 25th, 2013, 9:58 p.m. CET, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">only till Qt 5. Then I \
would clearly vote for bridge does not need to be ABI stable.</pre> </blockquote>
<p>On March 15th, 2013, 1:11 p.m. CET, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have been thinking \
about it. Given that Compiz is more or less an Ubuntu-only thing, I don't think \
it's worth the effort to keep compatibility. I'll recheck which distros still \
provide it and do an RFC whether we should remove support.</pre> </blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">fun stuff: \
not even Ubuntu is shipping support for kde-window-decorator. So no reason to keep \
support.</pre> <br />
<p>- Martin</p>
<br />
<p>On February 24th, 2013, 11:36 p.m. CET, Thomas Lübking wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for kwin, Martin Gräßlin and Hugo Pereira Da Costa.</div>
<div>By Thomas Lübking.</div>
<p style="color: grey;"><i>Updated Feb. 24, 2013, 11:36 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Actually the only thing the patch does not is what's stated in the \
summary ;-)
- The setting is ignored, the decoration always gets a "true" for it
- moving a maximized window requires breaking a "strong" snap (1/16 of \
screen height - unless you use quick maximization)
- all snapping is done towards the client, not the frame
- QuickTileMode is exported to the decoration (just as the maximizeMode) so that it \
can fix the bordersize alongside that. (I've a sample implementation in local \
Bespin as inspiration for other decos)
Ratio:
It's simply not possible to perform the specialstate snapping in the core.
Either the decoration gets clipped, but that also clips titlebar buttons -> fail
Or the decoration gets resized "under the hood" (ie. we override border_* \
after obtaining it) but that has bad visual impact on decorations like plastik (where \
the client visually overflows the border, it clearly looks like the client is patched \
onto the deco) -> fail
-> the deco is informed about those special states and can drop some of its \
borders or not. Should be no major problem for oxygen (since it blends into clients \
anyway) but i've not checked how to make use of that in aurorae (or even its \
client code)</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">With the (still local) changes to bespin I get nice partial clipping for \
quicktiled (or partially maximized) windows. Helped me to clean up some bespin deco \
code as well =)</pre> </td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=299245">299245</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kwin/workspace.h <span style="color: grey">(e033ac9)</span></li>
<li>kwin/libkdecorations/kdecoration_p.cpp <span style="color: \
grey">(5b54369)</span></li>
<li>kwin/libkdecorations/kdecorationbridge.h <span style="color: \
grey">(2cb36c9)</span></li>
<li>kwin/libkwineffects/kwinglobals.h <span style="color: \
grey">(dba4324)</span></li>
<li>kwin/kcmkwin/kwindecoration/preview.cpp <span style="color: \
grey">(94aacba)</span></li>
<li>kwin/libkdecorations/kdecoration.h <span style="color: \
grey">(2c20767)</span></li>
<li>kwin/libkdecorations/kdecoration.cpp <span style="color: \
grey">(7ccbdb6)</span></li>
<li>kwin/libkdecorations/kdecoration_p.h <span style="color: \
grey">(71833cd)</span></li>
<li>kwin/bridge.h <span style="color: grey">(35efc90)</span></li>
<li>kwin/bridge.cpp <span style="color: grey">(31d285e)</span></li>
<li>kwin/client.h <span style="color: grey">(0c9d1d2)</span></li>
<li>kwin/client.cpp <span style="color: grey">(f2338d4)</span></li>
<li>kwin/geometry.cpp <span style="color: grey">(372a4c8)</span></li>
<li>kwin/kcmkwin/kwindecoration/preview.h <span style="color: \
grey">(420e302)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103948/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic