Re: plasma and new shadow mess
On Monday 07 January 2013, Martin Gräßlin wrote: ... What I have measured is the rendering of window decorations which is basically the pain point of our complete rendering stack and follows a similar approach to rendering (each side rendered to a pixmap, TFP done What is TFP ? Alex
Re: plasma and new shadow mess
On Dienstag, 8. Januar 2013 18:40:58 CEST, Alexander Neundorf wrote: What is TFP ? gl_texture_from_pixmap() ;-)
Re: plasma and new shadow mess
On Sunday, January 6, 2013 17:40:42 Thomas Lübking wrote: 1. it will make kwin link generic-shell what is sematically the gnome/unity shell approach. it is a library. that does not rely on the desktop (or other) shell. it provides shared functionality for applications in kde-workspace, but without a guaranteed API for others (ergo no headers). so, no, it is not at all the same approach as gnome/unity. at all. 2. it will visually break non-plasma QMLs (so virtually they're no longer supported, see Martins concern in [1]) ??? which QML are you thinking of that is non-plasma? what are they doing using Plasma's SVG theming? Notice that this is a dead stupid and cumbersome way to do things [2], but ... [2] Instead of dumb copying the shadow implementation of plasma we could maybe rather just load the Plasma::Svg and hand over the pixmap(id) internally to the Shadow system - less tested, though, but could be then also easily extended to proper QML/Component shadow support, thus not pointless either. handing over the pixmap id intenrally to the shadow system is exactly what we do in plasma. granted, it is outside of kwin and we're doing this with an xatom ... but this is precisely how plasma is doing it now: we rely on kwin (or another window manager) to paint the shadows using pixmaps we hand over by id. -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Re: plasma and new shadow mess
On Sun, Jan 6, 2013 at 10:37 AM, Aaron J. Seigo ase...@kde.org wrote: On Sunday, January 6, 2013 13:35:16 Martin Graesslin wrote: btw, these changes were made in mid-November of 2012. i'm a little surprised people are only noticing now. I'm sure this change is not included the first beta, people might see things goes alright and then we have Christmas which worse the case. I'm working on some of fix.. those changes go into 4.10 or 4.11 I don't really care.. but I don't want have visual regression for 4.10. [1] https://git.reviewboard.kde.org/r/108222/ [2] https://git.reviewboard.kde.org/r/108223/ If code is not reverted, we may discuss the solution? As for kwin tabbox shadow, though I'm not expert for kwin, there is two problem 1. how to do the shadow for tabbox? use X property or some kwin custom way because it's in kwin. Well.. I guess later way it not easy and will duplicate the shadow drawing code path, so I still propose to use X property for passing shadow. 2. who provide the shadow, qml or the current declarativeview? I think qml is much more easy, since the Svg object in qml is the Plasma::Svg. If qml tabbox want to use shadow, it can pass the property to rootObject and let declarativeview easily get and render it.
Re: plasma and new shadow mess
On Sunday, January 6, 2013 17:01:06 Martin Gräßlin wrote: On Sunday 06 January 2013 16:37:47 Aaron J. Seigo wrote: btw, these changes were made in mid-November of 2012. i'm a little surprised people are only noticing now. maybe because the hard feature freeze was on November 8th and nobody expects that they need to adjust their applications after the hard feature freeze? my point was that apparently people have neither tested or noticed for nearly 2 months. it's a surprising result. And now it's really, really late to fix these issues. perhaps. i personally don't think it is too late given the trivial nature of the changes needed to make it happen due to having code that has been tested for a couple releases now that can be repurposed (the code in use is NOT new to 4.10) btw, we would have patches done already if we took the time writing these emails and instead spent it writing those patches. it's not rocket science or invasive type changes. shall be fixed based on experience what last minute changes which cannot be properly tested can cause issues in a compositor. So no, in KWin that won't be fixed for 4.10. i respect that this is your call to make as kwin maintainer. Overall I'm rather disappointed on how the situation came up and how it is handled now. me too. i'll discuss this on the plasma-devel list as i believe that is where this discussion belongs. It's not too late to revert the change and do it for 4.11 in a coordinated way that doesn't require rushed in changes. a reversion is not going to happen at this point. it is the proper way to do the shadows (one that relies on window manager functionality which kwin has thankfully provided for quite some time now; i'm a bit surprised that of all applications, kwin was doing shadows incorrectly for its own windows) and it allows us to use shadows on more elements without breaking the UI in the desktop. a reversion would likely mean also rolling back some changes in the Air theme and a bunch of re-testing of the desktop. -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Re: plasma and new shadow mess
On Sunday, January 6, 2013 20:47:55 Weng Xuetian wrote: On Sun, Jan 6, 2013 at 10:37 AM, Aaron J. Seigo ase...@kde.org wrote: On Sunday, January 6, 2013 13:35:16 Martin Graesslin wrote: btw, these changes were made in mid-November of 2012. i'm a little surprised people are only noticing now. I'm sure this change is not included the first beta, assuming it didn't make the release (which happened 1 week after the changes were made), this implies we rely on beta testers and the developers aren't paying attention to their own applications. this surprises me. I'm working on some of fix.. those changes go into 4.10 or 4.11 I don't really care.. but I don't want have visual regression for 4.10. [1] https://git.reviewboard.kde.org/r/108222/ [2] https://git.reviewboard.kde.org/r/108223/ both look good, and as you noted in the reviews they actually simplify the code in the process. i've processed both reviews and they should go into the 4.10 and master branches imo. As for kwin tabbox shadow, though I'm not expert for kwin, there is two problem 1. how to do the shadow for tabbox? use X property or some kwin custom way because it's in kwin. obviously up to the kwin devs, but the x property approach should work and then there's no special code path to worry about: all shadows will be handled the same way. it would also allow the use of code in the plasmagenericshell library. 2. who provide the shadow, qml or the current declarativeview? I think qml is much more easy, since the Svg object in qml is the Plasma::Svg. If qml tabbox want to use shadow, it can pass the property to rootObject and let declarativeview easily get and render it. i don't know about this one, as i'm not familiar enough with the internals of kwin's tab box .. i'm sure Martin or Thomas can offer direction here though. -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Re: Re: plasma and new shadow mess
On Monday 07 January 2013 10:51:11 Aaron J. Seigo wrote: On Sunday, January 6, 2013 17:40:42 Thomas Lübking wrote: 1. it will make kwin link generic-shell what is sematically the gnome/unity shell approach. it is a library. that does not rely on the desktop (or other) shell. it provides shared functionality for applications in kde-workspace, but without a guaranteed API for others (ergo no headers). why then not in libkworkspace? If it's meant for more then just the desktop shells the name sounds confusing. so, no, it is not at all the same approach as gnome/unity. at all. 2. it will visually break non-plasma QMLs (so virtually they're no longer supported, see Martins concern in [1]) ??? which QML are you thinking of that is non-plasma? what are they doing using Plasma's SVG theming? I think it was bad worded by Thomas as it's actually a Plasma which would get broken: the window strip of Plasma Active. But: we make it possible to load any Qml styled window switcher theme. There is no requirement to use Plasma Components, it's just a Qml file that gets loaded. If a user wants to have a KDE 3 style all grey that's possible. If we enforce Plasma that's no longer possible and if there is someone who uses that already it gets broken if we require the shadows. That's exactly the concern mentioned in the review request. Notice that this is a dead stupid and cumbersome way to do things [2], but ... [2] Instead of dumb copying the shadow implementation of plasma we could maybe rather just load the Plasma::Svg and hand over the pixmap(id) internally to the Shadow system - less tested, though, but could be then also easily extended to proper QML/Component shadow support, thus not pointless either. handing over the pixmap id intenrally to the shadow system is exactly what we do in plasma. granted, it is outside of kwin and we're doing this with an xatom ... but this is precisely how plasma is doing it now: we rely on kwin (or another window manager) to paint the shadows using pixmaps we hand over by id. Yeah well, I don't want a roundtrip to X just to set an information which we have internally already available. For KWin we need a better approach. Btw. for the use case of the window switcher the approach of setting the shadow is a performance regression. It's actually for everything inside KWin (and Plasma) a performance regression. The shadow system has been designed to work around the problem which occurs if we try to have the shadow in the panel. That is for any window where the shadow should not be part of the window geometry. It makes sense for the panel, for KRunner and the Oxygen menus. This approach comes with an additional rendering cost inside KWin. We need to create textures from multiple pixmaps, we don't have the window as a consistent texture any more (causes rendering glitches with various effects) and we need several rendering passes - one for the shadow, one for the window. The cost for rendering is higher from throughput to graphics card and from memory consumption (both GPU and X). In the case of TabBox or anything else in KWin (and also for most things in Plasma like tooltips, extenders) we don't have the requirement that the shadows may not be part of the window geometry as it just does not matter. So we get the disadvantage without any advantage. Oh and the fact that the window is one texture instead of multiple is the only valid argument for Client Side Window Decorations and has been the only argument presented by Kristian Hogsberg in his Wayland presentation on XDC. And you know what? I have to agree in that point. Cheers Martin signature.asc Description: This is a digitally signed message part.
Re: Re: plasma and new shadow mess
On Monday 07 January 2013 11:03:24 Aaron J. Seigo wrote: a reversion is not going to happen at this point. it is the proper way to do the shadows (one that relies on window manager functionality which kwin has thankfully provided for quite some time now; i'm a bit surprised that of all applications, kwin was doing shadows incorrectly for its own windows) see my other mail to the thread why in the case of KWin it's a disadvantage to use its own shadow system and it allows us to use shadows on more elements without breaking the UI in the desktop. a reversion would likely mean also rolling back some changes in the Air theme and a bunch of re-testing of the desktop. -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Re: plasma and new shadow mess
On Monday, January 7, 2013 11:14:49 Martin Gräßlin wrote: The shadow system has been designed to work around the problem which occurs if we try to have the shadow in the panel. That is for any window where the shadow should not be part of the window geometry. It makes sense for the panel, for KRunner and the Oxygen menus. it also matters for tooltips where we would like them to align with other windows without including shadows in those calculations (shadows are conceptually immaterial things, so should not cause a visual gap). given that our popup applets also have semantics similar to menus, it matters there as well. i agree there are some cases where it doesn't matter. This approach comes with an additional rendering cost inside KWin. has the cost been measured? this is the most important thing imho. if it costs us 10fps maybe it matters. if it costs us 1fps maybe it doesn't. We need to create textures from multiple pixmaps, we don't have the window as a consistent texture any more (causes rendering glitches with various effects) and we need several rendering passes - one for the shadow, one for the window. the shadow+windowframe is not cached somewhere? -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Re: plasma and new shadow mess
On Montag, 7. Januar 2013 14:18:04 CEST, Aaron J. Seigo wrote: the shadow+windowframe is not cached somewhere? It's not shadow+windowframe but shadow+window - doubling memory usage for that window (once more) since the window texture/picture is provided as pixmap by the redirection, you cannot extend that and ofset the redirection in your shadow-prepared window. (no, wayland should rather not change that) *that* can make things *really* slow if you run out of VRAM (or explicitly leave it by caching in some QImage) - not to speak of the nvidia driver being quite unhappy everytime you allocate memory at runtime.
Re: plasma and new shadow mess
On Montag, 7. Januar 2013 10:51:11 CEST, Aaron J. Seigo wrote: it is a library. that does not rely on the desktop (or other) shell. it provides shared functionality for applications in kde-workspace, but without a guaranteed API for others (ergo no headers). = $ for name in /usr/bin/*; do if [ ! -z `ldd $name | grep libplasmagenericshell` ]; then echo $name; fi; done /usr/bin/krunner /usr/bin/plasma-desktop /usr/bin/plasma-netbook /usr/bin/plasma-overlay --- $ ls libs/plasmagenericshell BackgroundDialog.ui backgrounddialog.cpp mouseplugins.h plasma-layout-template.desktop toolbutton.h CMakeLists.txt backgrounddialog.hmousepluginwidget.cpp plasmagenericshell_export.h wallpaperpreview.cpp Messages.sh* mouseinputbutton.cpp mousepluginwidget.hscripting/ wallpaperpreview.h MousePlugins.ui mouseinputbutton.hpanelshadows.cpp tests/ widgetsexplorer/ TODO mouseplugins.cpp panelshadows.h toolbutton.cpp = So it provides panelshadows, a widgetexplorer, a Click to change how an action is triggered mouseinputbutton, a wallpaperpreview, a backgrounddialog (seems wallpaper picker) ... plasma-* are thin wrappers on the libs that effectively form the desktop shell. If it wasn't, KWin and other stuff was by now linking the desktop process (usually in order to use a common theme) anyway, and if the WM links libplasmagenericshell.so it is even *semantically* the gnome/unity approach - or i need a new dictionary. Thomas
Re: plasma and new shadow mess
On Monday 07 January 2013, Martin Gräßlin wrote: handing over the pixmap id intenrally to the shadow system is exactly what we do in plasma. granted, it is outside of kwin and we're doing this with an xatom ... but this is precisely how plasma is doing it now: we rely on kwin (or another window manager) to paint the shadows using pixmaps we hand over by id. Yeah well, I don't want a roundtrip to X just to set an information which we have internally already available. For KWin we need a better approach. It actually adds 25 roundtrips per window, including 8 image transfers from the X server. Fredrik
Re: plasma and new shadow mess
Am 07.01.2013 14:18, schrieb Aaron J. Seigo: On Monday, January 7, 2013 11:14:49 Martin Gräßlin wrote: The shadow system has been designed to work around the problem which occurs if we try to have the shadow in the panel. That is for any window where the shadow should not be part of the window geometry. It makes sense for the panel, for KRunner and the Oxygen menus. it also matters for tooltips where we would like them to align with other windows without including shadows in those calculations (shadows are conceptually immaterial things, so should not cause a visual gap). given that our popup applets also have semantics similar to menus, it matters there as well. ok, that makes sense there i agree there are some cases where it doesn't matter. This approach comes with an additional rendering cost inside KWin. has the cost been measured? very difficult as it depends on: a) driver b) hardware c) graphics system What I have measured is the rendering of window decorations which is basically the pain point of our complete rendering stack and follows a similar approach to rendering (each side rendered to a pixmap, TFP done from it, rendered in addition to the window) as the shadows. All lags during animations are caused by that architecture - though mostly due to rendering the decorations in the same thread as the compositor. It's not exactly comparable to the shadow situation as the shadows are rendered out of KWin process and are not supposed to be change and stored in one texture. But still if the combination of the three points above is bad, it can cause a lack during first TFP operation. Overall it has an impact due to the OpenGL state changes, but how much: depends, though for most cases not badly. this is the most important thing imho. if it costs us 10fps maybe it matters. if it costs us 1fps maybe it doesn't. We need to create textures from multiple pixmaps, we don't have the window as a consistent texture any more (causes rendering glitches with various effects) and we need several rendering passes - one for the shadow, one for the window. the shadow+windowframe is not cached somewhere? no that doesn't make sense as the window content changes. Cheers Martin
Re: plasma and new shadow mess
On Monday, January 7, 2013 15:15:21 Thomas Lübking wrote: On Montag, 7. Januar 2013 10:51:11 CEST, Aaron J. Seigo wrote: it is a library. that does not rely on the desktop (or other) shell. it provides shared functionality for applications in kde-workspace, but without a guaranteed API for others (ergo no headers). $ for name in /usr/bin/*; do if [ ! -z `ldd $name | grep libplasmagenericshell` ]; then echo $name; fi; done /usr/bin/krunner /usr/bin/plasma-desktop /usr/bin/plasma-netbook /usr/bin/plasma-overlay so far so good ... these 4 processes use the library. the library has no dependencies on them. So it provides panelshadows, a widgetexplorer, a Click to change how an action is triggered mouseinputbutton, a wallpaperpreview, a backgrounddialog (seems wallpaper picker) ... it also includes the desktop js scripting used for layouts, activity templates, etc. so yes, it's really a grab bag of things used in common by the workspace apps. as an internal library, such design-less factor-out-common-stuff pragmatism is fine. as an aside: during the move to libplasma2 + qml2, at least some of these things (perhaps the majority, even) will likely become individual QML components. plasma-* are thin wrappers on the libs that effectively form the desktop plasma-desktop is over 2x the lines of code of libplasmagenericshell. it extends classes from the libraries, even. so it is not a thin wrapper by any reasonable definition. links libplasmagenericshell.so it is even *semantically* the gnome/unity approach - or i need a new dictionary. i don't think you need a new dictionary. but digging a bit deeper into the facts and actually understanding the design of the software you are writing about would be useful. in this case, the library is not where the core code, or even the semantic definition, of the shell processes resides. in fact, the bits that make them shells (the panels, the desktop views, screen handling, configuration, etc..) are predominantly in the shell processes. using libplasmagenericshell does not somehow integrate it more (or less) with Plasma Desktop or any other plasma shell. this is all a moot discussion, however, and i engage only to help you understand something better which you evidently have misconceptions about. the actual solution, however, is not dependent on getting this particular aspect right. if we focus on solution instead of pointless discussion we probably come to this: if libplasmagenericshell is too big of a library or contains functionality kwin is allergic to (for whatever reason one finds/invents), the obvious answer is to move the shadow classes to one of the smaller kde-workspace libraries kwin already links to such as libkworkspace, which the shells also already link to. as they are internal libraries shipped with the binaries, such a shuffle is easy to do and completely allowed, even between minor releases. in fact, the *best* answer would probably be to copy the DialogShadows class from kdelibs/plasma/private/ and put it in kworkspace (or wherever we figure is appropriate so as to meet the needs of kwin and the other binaries in kde- workspace). it is an improvement over the PanelShadows, which assumes the window is edge docked and shadows can therefore be applied all the way around without regard for the reality of the window's borders. porting the use in plasma-desktop to this replacement class is trivial: it's the same API except addWindow has one more parameter, the default argument of which would be equivalent to the existing PanelShadows behaviour. so .. no porting other than perhaps changing the name of the class used. if this is a fitting / desired solution, i can easily make it happen in a very short time span. cheers. -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Re: plasma and new shadow mess
On Sunday 06 January 2013 01:44:18 Aaron J. Seigo wrote: On Monday, December 24, 2012 17:12:22 Weng Xuetian wrote: snip they simply need to be fixed to have the window manager handle that shadows. one way to do this easily is to use Plasma::Dialog. the OSD in kmix (and elsewhere) really shouldn't be using its own QWidget based thing at all. it ought to be a bit of QML in a Plasma::Dialog. Would that also work when running outside of Plasma then? Cheers -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.
Re: plasma and new shadow mess
On Sunday, January 6, 2013 12:22:47 Milian Wolff wrote: On Sunday 06 January 2013 01:44:18 Aaron J. Seigo wrote: On Monday, December 24, 2012 17:12:22 Weng Xuetian wrote: snip they simply need to be fixed to have the window manager handle that shadows. one way to do this easily is to use Plasma::Dialog. the OSD in kmix (and elsewhere) really shouldn't be using its own QWidget based thing at all. it ought to be a bit of QML in a Plasma::Dialog. Would that also work when running outside of Plasma then? yes. it has no runtime dependencies on anything other than a supported window manager (one that provides shadow rendering for windows); if the wm doesn't provide that, then you just get a window without shadow, but the rest continues to work as expected. -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Re: plasma and new shadow mess
On Sunday, January 6, 2013 13:35:16 Martin Graesslin wrote: On Monday 24 December 2012 17:12:22 Weng Xuetian wrote: I think some action need to be taken before the release, some possible solutions. 1. Revert the changes of new plasma air theme, so old shadow can be used. and try to fix all the things in KDE 4.11 Personal opinion: the change should be reverted, as: a) it was basically a break of public API (yes even if it is not considered public API, the fact that everybody used it, makes it public API) it is not a break of API since it isn't API at all. hyperbole helps nothing, so let's not go down that road. b) there is no need for a break now with KF 5 in front of us, we could have used that to do the break most changes are indeed being held for libplasma2 et al. however, as this affected presentation of existing UI in 4.x, i decided to fix it rather than continue with the existing shadow hacks. c) it causes unnecessary work for everybody the change was not done randomly. it puts the shadow drawing where it properly belongs and thereby lets the window be handled correctly with the visual edges (e.g. minus the shadows) being the actual edges of the window. as the change was done for a reason, the work it caused was not without reason either. ergo, not unnecessary. or 2. get some header exposed to avoid duplication of the code, and fixed every custom widget, at least including: kwin, kmix, powerdevil, icontasks. at least for KWin I will *not* accept a fix for 4.10. It's too late in the cycle, it cannot be exposed to user testing any more. I rather have a visual regression than the risk of breakage. so instead of using tested code, you'd prefer a regression visible to the user. this doesn't make much sense to me. truth is that it requires a small amount of work applied to a few areas in the code. if we want to hold those changes until 4.10.1, that's fine with me. btw, these changes were made in mid-November of 2012. i'm a little surprised people are only noticing now. -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
Re: Re: plasma and new shadow mess
On Sunday 06 January 2013 16:37:47 Aaron J. Seigo wrote: btw, these changes were made in mid-November of 2012. i'm a little surprised people are only noticing now. maybe because the hard feature freeze was on November 8th and nobody expects that they need to adjust their applications after the hard feature freeze? Also at least I had not known about it (which really surprises me given that it's about usage of a KWin feature). Might be that I missed a thread on plasma-devel, but mid/end November I was totally stressed and did not have much time for KDE. Dezember I was on vacations, then Christmas and New Year, so yeah in my case it's no surprise I did not notice it. And now it's really, really late to fix these issues. If there would not have been the delay due to another RC we would not have a chance to get out fixes which are actually tested. And to me RC means only showstoppers shall be fixed based on experience what last minute changes which cannot be properly tested can cause issues in a compositor. So no, in KWin that won't be fixed for 4.10. Also the thread on the release team mailing list nicely illustrates why we should stick to such schedules. Overall I'm rather disappointed on how the situation came up and how it is handled now. It's not too late to revert the change and do it for 4.11 in a coordinated way that doesn't require rushed in changes. It's the don't break userspace thing Linus is always talking about, isn't it? Cheers Martin signature.asc Description: This is a digitally signed message part.
Re: plasma and new shadow mess
On Sonntag, 6. Januar 2013 16:37:47 CEST, Aaron J. Seigo wrote: it is not a break of API since it isn't API at all. hyperbole helps nothing, so let's not go down that road. Unilateral change broke things, so technically it's I - whether API is a different matter. the change was not done randomly. it puts the shadow drawing where it properly belongs and thereby lets the window be handled correctly with the visual edges (e.g. minus the shadows) being the actual edges of the window. All true, but i doubt that Martin meant pointless but rather without pressure. Given the flamewars on blocked until frameworks it's a pretty fair question whether it was *necessary* to do it *now*. Doesn't matter either. Happened. Question is how to deal with the situation. so instead of using tested code, you'd prefer a regression visible to the user. I doubt we can just use the Plasma::Shadow code in KWin elements like the tabbox since it operates on QML and not on Plasma Frame (Dialog) directly. 1. it will make kwin link generic-shell what is sematically the gnome/unity shell approach. 2. it will visually break non-plasma QMLs (so virtually they're no longer supported, see Martins concern in [1]) If a clean solution for shadows in QML (as described in the RR [1]) cannot be provided for 4.10 my personal suggestion for 4.10 (only!) would be to temporarily copy over the Plasma::Shadow sources (so we also don't have to break dependency freeze ;-P) and use them controlled by a property in the QML. Ideally the property would be sth. tagging that plasma compmenents are loaded resp. even better PlasmaCore.FrameSvgItem is used. In doubt it would have to be an explicit marker. Notice that this is a dead stupid and cumbersome way to do things [2], but should be safe enough (oxygen popups and bespin decoration long time use the shadow property for in-process Unmanaged and clients) Cheers, Thomas [1] https://git.reviewboard.kde.org/r/108224/ [2] Instead of dumb copying the shadow implementation of plasma we could maybe rather just load the Plasma::Svg and hand over the pixmap(id) internally to the Shadow system - less tested, though, but could be then also easily extended to proper QML/Component shadow support, thus not pointless either.
Re: plasma and new shadow mess
On Monday, December 24, 2012 17:12:22 Weng Xuetian wrote: Hi Plasma world, As new shadow lands in KDE 4.10 RC1, some unintentional mess is introduced. https://bugs.kde.org/show_bug.cgi?id=311502 https://bugs.kde.org/show_bug.cgi?id=311995 as pointed out by others, these bug reports are about other things. (thanks, btw, for fixing one of them! :) The problem is, custom widget using plasma svg doesn't get new shadow support automatically, some code must be written. that shadows for windows were ever done by the application is a bug. this should always have been done by the window manager. they were done by the application for historical reasons (plasma-desktop started doing those shadows before kwin was able to accomodate its needs; once kwin was ready, we then had to adapt plasma code ..) so i understand how other uses (e.g. the OSD) could get it wrong by simply copying what we did in, e.g., plasma-desktop (or by simply being written a long time ago), but they are still *wrong*. they simply need to be fixed to have the window manager handle that shadows. one way to do this easily is to use Plasma::Dialog. the OSD in kmix (and elsewhere) really shouldn't be using its own QWidget based thing at all. it ought to be a bit of QML in a Plasma::Dialog. What make this problem worse is, the necessary class for shadow is not public. And AFAIK there are already 3 copies of same code across different kde projects (kdelibs/plasma/private/dialogshadows , kde-workspace/libs/plasmagenericshell/panelshadows, this is due to historical events. it debuted in kde-workspace, and once it was shown working there, we later added this to libplasma. as we are in the middle of moving to qml, which will cause changes to the shells in kde-workspace anyways, i decided to live with the small duplication of code for now and address it when the shells are reworked to be purely qml based. otherwise, we'd essentially be doing work twice. there is, however, no particular design based reason for the duplication of code between kdelibs and kde-workspace. and one of my own in kdeplasma-addons), it would really be better if your copy was removed by moving to Plasma::Dialog. if there are barriers to this, sharing them with us on plasma- devel would be a good first step. plasmagenericshell is a shared library but it doesn't install header.. yes, it is shared between applications in kde-workspace. it is not a public library. -- Aaron J. Seigo signature.asc Description: This is a digitally signed message part.
plasma and new shadow mess
Hi Plasma world, As new shadow lands in KDE 4.10 RC1, some unintentional mess is introduced. https://bugs.kde.org/show_bug.cgi?id=311502 https://bugs.kde.org/show_bug.cgi?id=311995 And it affects some more components, at least including kmix osd, brightness osd, icontasks. The problem is, custom widget using plasma svg doesn't get new shadow support automatically, some code must be written. I observe that some components get necessary modifications, for example krunner, while also quite a few of them still not. What make this problem worse is, the necessary class for shadow is not public. And AFAIK there are already 3 copies of same code across different kde projects (kdelibs/plasma/private/dialogshadows , kde-workspace/libs/plasmagenericshell/panelshadows, and one of my own in kdeplasma-addons), plasmagenericshell is a shared library but it doesn't install header.. I think some action need to be taken before the release, some possible solutions. 1. Revert the changes of new plasma air theme, so old shadow can be used. and try to fix all the things in KDE 4.11 or 2. get some header exposed to avoid duplication of the code, and fixed every custom widget, at least including: kwin, kmix, powerdevil, icontasks. I can give out my help for fix those, but some decision still need to be made (add new header to kdelibs or kde-workspace). Regards
Re: plasma and new shadow mess
On Montag, 24. Dezember 2012 23:12:22 CEST, Weng Xuetian wrote: Hi Plasma world, As new shadow lands in KDE 4.10 RC1, some unintentional mess is introduced. https://bugs.kde.org/show_bug.cgi?id=311502 https://bugs.kde.org/show_bug.cgi?id=311995 I doubt those have much relation, notably #311995 is not about shadows at all, but invocation of the non-ARGB theme under certain conditions. The problem is, custom widget using plasma svg doesn't get new shadow support automatically, some code must be written. The problem of #311502 rather seems to be an insufficient repaint area - more related to bug #312168, i'd say. Wild guess from your patch: don't remove the shadows before, but *after* the hiding (eventually later, ie. in the deconstructor or on the destroyed signal) to avoid confusations between the deleted window size and the dirty area. Cheers, Thomas
