Bug 295056 - shadow passed to decoration via _KDE_NET_WM_SHADOW are not drawn in KCM preview
Summary: shadow passed to decoration via _KDE_NET_WM_SHADOW are not drawn in KCM preview
Status: RESOLVED WORKSFORME
Alias: None
Product: kwin
Classification: Plasma
Component: kdecorations (show other bugs)
Version: git master
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-29 10:19 UTC by Hugo Pereira Da Costa
Modified: 2013-01-30 08:14 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hugo Pereira Da Costa 2012-02-29 10:19:09 UTC
Version:           git master (using Devel) 
OS:                Linux

Title says it all. 
Also see screenshot at:



Reproducible: Didn't try

Steps to Reproduce:
1/ checkout the code (and compile)
  cd kde-workspace
  git checkout hpereira/oxygen-shadows

2/ look at previews in window decorations kcm



Actual Results:  
-

Expected Results:  
-
Comment 1 Martin Flöser 2012-02-29 14:11:19 UTC
any suggestion what we can do about it?

My suggestion would be that the decoration has to handle this for itself 
through the knowledge that it is a preview.
Comment 2 Hugo Pereira Da Costa 2012-02-29 14:34:53 UTC
well, that would be pretty much equivalent to the code I use now for the shadows ... Meaning, two code path to maintain.
Also, when using hints, I had to remove the LM_OUTER_PADDING metrics.
These guys where used to draw the shadow *outside* of the window, in the preview.
Redoing the painting now, without the Outer_padding, would result in much smaller windows, which is not very satisfactory either ...

I would personally consider such a solution as quite demotivating for switching to the HINT implementation.
Comment 3 Martin Flöser 2012-02-29 14:40:53 UTC
> I would personally consider such a solution as quite demotivating for
> switching to the HINT implementation.
yes I understand that, but I'm just failing to see what we could do here. 
Given that the deco sets the X11 hint we cannot read it in the deco.

So we need ideas how to fix that... maybe a Q property read by the list?
Comment 4 Hugo Pereira Da Costa 2012-02-29 14:48:44 UTC
just shooting in the dark (cause I'm honestly not sure of what I am talking about):

Any way one can create a dummy invisible X11 window, pass its winID to the client (as done for the real window), get the shadows from there, and do the magic ? 
Otherwise, I guess QProperty also works (and is not much extra code on the decoration side)
Comment 5 Thomas Lübking 2012-02-29 14:50:48 UTC
The decoration can paint into the preview whatever it wants - see the bespin preview. Do you actually believe i paint that symbol behind every client? =)

So just check whether isPreview() and paint the shadow by hand there - the height of the preview should be the least problem (one could even try to abuse the minimum decoration hint, i'll try whether that works)
Comment 6 Hugo Pereira Da Costa 2012-02-29 15:39:17 UTC
> The decoration can paint into the preview whatever it wants - see the bespin
> preview

I know that, that's how I get the background gradient instead of the default flat QLabel background.

As for the margins (which is more of a concern), that all sounds quite hacky.
Also, that still leaves the fact that this would be quite duplicated code (pretty much keeping the current implementation with an "if( isPreview() )" in front, and adding a new implementation for the HINT.
Not very satisfactory, I'd say. (would rather stay with current code, especially since I have not noticed significant performances improvements so far ...)
Comment 7 Thomas Lübking 2012-02-29 15:44:44 UTC
(In reply to comment #6)

> Not very satisfactory, I'd say. (would rather stay with current code,
> especially since I have not noticed significant performances improvements so
> far ...)

The OpenGL path is still piped through a QImage creating a new texture, however if your animation should not only update the shadow but also the entire decoration, that will get you nothing. We need a new deco API avoiding the paint redirector for this.
Comment 8 Thomas Lübking 2012-03-01 07:57:19 UTC
(In reply to comment #4)

> Any way one can create a dummy invisible X11 window, pass its winID to the
> client (as done for the real window), get the shadows from there, and do the
> magic ?
Not w/o breaking deco ABI in a rather unpleasing way - but since we discuss that anyway, the deco can just export the shadow pixmaps or some shadow structure directly.

> Otherwise, I guess QProperty also works (and is not much extra code on the
> decoration side)
*If* we break deco ABI for 4.9 that's not necessary. As an interim solution if you want to pre-ship the deco via kde-look, it's probably most sane to live with the glitch for the time or paint the shadows by hand (there's no real point in rewriting the kcm twice for that purpose, and possibly not for 4.8 anyway)
Comment 9 Martin Flöser 2012-03-01 08:07:40 UTC
> > Any way one can create a dummy invisible X11 window, pass its winID to the
> > client (as done for the real window), get the shadows from there, and do
> > the magic ?
> 
> Not w/o breaking deco ABI in a rather unpleasing way - but since we discuss
> that anyway, the deco can just export the shadow pixmaps or some shadow
> structure directly.
This sounds like a good idea as it would also remove the roundtrip through X.
Comment 10 Hugo Pereira Da Costa 2012-03-01 10:09:06 UTC
Git commit 09b7d944a1b2a11d6e1b2c767c271f4cad2fc804 by Hugo Pereira Da Costa.
Committed on 01/03/2012 at 11:07.
Pushed by hpereiradacosta into branch 'hpereira/oxygen-shadows'.

re-added painting of shadows inside client, in "preview()" case :(

M  +43   -18   kwin/clients/oxygen/oxygenclient.cpp

http://commits.kde.org/kde-workspace/09b7d944a1b2a11d6e1b2c767c271f4cad2fc804
Comment 11 Hugo Pereira Da Costa 2012-03-01 10:15:25 UTC
Git commit 51baa3343947afe652f44e7ff7e1bf2a7b8c933a by Hugo Pereira Da Costa.
Committed on 01/03/2012 at 11:14.
Pushed by hpereiradacosta into branch 'hpereira/oxygen-shadows'.

also fixed sizegrip positionning .

M  +6    -2    kwin/clients/oxygen/oxygensizegrip.cpp

http://commits.kde.org/kde-workspace/51baa3343947afe652f44e7ff7e1bf2a7b8c933a
Comment 12 Martin Flöser 2013-01-30 08:14:18 UTC
seems like a worksforme with the preview case. From the KCM there is just nothing which could be done about it.