[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo
From:       "Aaron J. Seigo" <aseigo () kde ! org>
Date:       2009-01-08 23:42:40
Message-ID: 200901081741.34460.aseigo () kde ! org
[Download RAW message or body]


On Thursday 08 January 2009, Fredrik Höglund wrote:
> On Thursday 08 January 2009 18:59, Aaron J. Seigo wrote:
> > SVN commit 907753 by aseigo:
> >
> > don't assume 32 bit depth, makes trays work on 16 bit depth with the new
> > hint. patch by ddenis @ Qt; thanks! =)
> >
> >
> >  M  +16 -12    fdoselectionmanager.cpp
> >  M  +0 -4      x11embedcontainer.cpp
> >
> >
> > ---
> > trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/fdose
> >lectionmanager.cpp #907752:907753 @@ -267,20 +267,24 @@
> >      int nvi;
> >      VisualID visual =
> > XVisualIDFromVisual((Visual*)QX11Info::appVisual()); XVisualInfo templ;
> > -    templ.screen  = DefaultScreen(d->display);
> > -    templ.depth   = 32;
> > -    templ.c_class = TrueColor;
> > -    XVisualInfo *xvi = XGetVisualInfo(d->display, VisualScreenMask |
> > VisualDepthMask | VisualClassMask, -                                     
> > &templ, &nvi);
> > -    for (int i = 0; i < nvi; i++) {
> > -        XRenderPictFormat *format = XRenderFindVisualFormat(d->display,
> > xvi[i].visual); -        if (format->type == PictTypeDirect &&
> > format->direct.alphaMask) { -            visual = xvi[i].visualid;
> > -            break;
> > +    templ.visualid = visual;
> > +    XVisualInfo *xvi = XGetVisualInfo(d->display, VisualIDMask, &templ,
> > &nvi); +    if (xvi) {
> > +        templ.screen  = xvi[0].screen;
> > +        templ.depth   = xvi[0].depth;
> > +        templ.c_class = xvi[0].c_class;
> > +        XFree(xvi);
> > +        xvi = XGetVisualInfo(d->display, VisualScreenMask |
> > VisualDepthMask | VisualClassMask, +                             &templ,
> > &nvi);
> > +        for (int i = 0; i < nvi; i++) {
> > +            XRenderPictFormat *format =
> > XRenderFindVisualFormat(d->display, xvi[i].visual); +            if
> > (format->type == PictTypeDirect && format->direct.alphaMask) { +         
> >       visual = xvi[i].visualid;
> > +                break;
> > +            }
> >          }
> > +        XFree(xvi);
> >      }
>
> You just turned this whole block of code into an elaborate no-op that
> always ends up right back where you started; with the visual Plasma was
> using in the first place.
>
> The whole point of this code is to tell applications that you want systray
> icons to always use the 32 bit ARGB visual regardless of the display depth,
> so they will always have an alpha channel.
>
> The new code still accomplishes that by accident, but only because Plasma
> itself always uses the ARGB visual. But the minute you remove that hack
> and start using the new Qt::WA_TranslucentBackground attribute, you
> break the systray.
>
> So could you explain what you were trying to accomplish with this patch
> so we can fix it properly?

according to ddenis who worked on the NET_SYSTEM_TRAY_VISUAL for Qt 4.5, the 
old code was failing on 16bpp displays. well, i'll let him speak for himself 
here:

[Thu Jan 8 2009] [03:55:29] <ddenis> Hi. I wanted to talk to about about the 
NET_SYSTEM_TRAY_VISUAL atom - I've implemented support for it in Qt 4.5, 
however I suspect that its implementation in the systemtray applet in kde is 
not correct - in the fdo/fdoselectionmanager.cpp in the initSelection() 
function the code that searches for the visual explicitely sets the color 
depth to be 32 and the problem occurs when I try to create a systray icon 
window with that visual on a 16bit display. As far as
[Thu Jan 8 2009] [03:55:29] <ddenis> I can see the proper way is to get the 
depth from the system and then try to find the ARGB-enabled visual with the 
same depth on the same screen

he provided a patch that he claimed worked for him in those situations, i 
applied and tested it here as well ... but perhaps you can propose a better 
solution that isn't an elaborate no-op? ;)

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Software


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]