From kwin Fri Mar 15 12:27:18 2013 From: =?utf-8?q?Martin_Gr=C3=A4=C3=9Flin?= Date: Fri, 15 Mar 2013 12:27:18 +0000 To: kwin Subject: Re: Review Request 103948: Remove "Display borders on maximized windows" Message-Id: <20130315122718.24663.56491 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kwin&m=136335045403616 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============0298403139242913114==" --===============0298403139242913114== Content-Type: multipart/alternative; boundary="===============4578985075321136704==" --===============4578985075321136704== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit > On Feb. 25, 2013, 1:53 p.m., Martin Gräßlin wrote: > > kwin/libkdecorations/kdecorationbridge.h, line 51 > > > > > > 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 > > --===============4578985075321136704== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103948/

On February 25th, 2013, 1:53 p.m. CET, Martin Gräßlin wrote:

kwin/libkdecorations/kdecorationbridge.h (Diff revision 4)
class KDecorationBridge : public KDecorationDefines
51
    virtual QuickTileMode quickTileMode() const = 0;
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?

On February 25th, 2013, 3:43 p.m. CET, 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)

On February 25th, 2013, 4:03 p.m. CET, 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)

On February 25th, 2013, 4:19 p.m. CET, 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 ;-)

On February 25th, 2013, 8:47 p.m. CET, 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.

On February 25th, 2013, 9:58 p.m. CET, Martin Gräßlin wrote:

only till Qt 5. Then I would clearly vote for bridge does not need to be ABI stable.

On March 15th, 2013, 1:11 p.m. CET, 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


On February 24th, 2013, 11:36 p.m. CET, Thomas Lübking wrote:

Review request for kwin, Martin Gräßlin and Hugo Pereira Da Costa.
By Thomas Lübking.

Updated Feb. 24, 2013, 11:36 p.m.

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)

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 =)
Bugs: 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)

View Diff

--===============4578985075321136704==-- --===============0298403139242913114== 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 --===============0298403139242913114==--