'Re: Review Request: new KWin effect: Dashboard'

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

List:       kde-panel-devel
Subject:    Re: Review Request: new KWin effect: Dashboard
From:       Martin_Gräßlin <kde () martin-graesslin ! com>
Date:       2010-06-16 18:19:28
Message-ID: 20100616181928.13615.406 () localhost
[Download RAW message or body]

> On 2010-06-15 20:55:30, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.cpp, lines 100-113
> > <http://reviewboard.kde.org/r/4332/diff/1/?file=28777#file28777line100>
> > 
> > that is quite the hack; this should be replaced with another method in \
> > Plasma::WindowEffects .. markAsDashboard? internally it can set an appropriate \
> > atom or window class (whatever is best from kwin's POV) and that can be called in \
> > DashboardView's ctor in plasma-desktop.
> 
> Andreas Demmer wrote:
> Yeah, I fully agree that the dashboard should be propagate itself with a window \
> class of its own. Should this be solely implemented for the Dashboard or do you \
> propose a Plasma-generic solution?

I think it should be a window type (as discussed in Nuremberg). E.g. \
_KDE_NET_WM_WINDOW_TYPE_DASHBOARD. That would make it easier to do the layering \
correctly. And if we are good citizens we have to get rid of the _KDE ;-)

> On 2010-06-15 20:55:30, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.cpp, line 36
> > <http://reviewboard.kde.org/r/4332/diff/1/?file=28777#file28777line36>
> > 
> > _KDE_DASHBOARD is a bit generic; i wonder if we should be namespacing these a bit \
> > better, e.g. _WM_EFFECT_KDE_DASHBOARD? something the kwin folk can comment on \
> > better than i, probably.
> 
> Andreas Demmer wrote:
> _KDE_PLUGIN-NAME is kind of a naming-convention I found in \
> kdelibs/plasma/windoweffects.cpp. I named it more specifically first and discussed \
> it with mgraesslin, who proposed the current name.

Yes better namespacing is required. I thought about contacting Compiz devs to \
harmonize the atoms for 4.6 (kwin)/0.9 (Compiz). At least with blur I think we have \
overlapping atoms and that sucks. And yes it might be an idea to start a new effects \
spec.

- Martin

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4332/#review6130
-----------------------------------------------------------

On 2010-06-15 20:31:15, Andreas Demmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4332/
> -----------------------------------------------------------
> 
> (Updated 2010-06-15 20:31:15)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Summary
> -------
> 
> This patch adds a new KWin effect that allows to modify the appearance of the \
> Plasma dashboard. It has a KCM configuration dialog where you can adjust \
> brightness, saturation and blur of the dashboard background. Blur depends on the \
> loaded blur plugin. 
> I also patched the Plasma dashboard to recognize the loaded effect: If the effect \
> is loaded, the dashboard draws its background fully translucent. In order for the \
> Dashboard to recognize wether the effect is loaded, I added support for the effect \
> in Plasma::WindowEffects from kdelibs. 
> The dashboard detection in the effect itself is hackish right now. As soon as \
> Plasma adds a proper class to the dashboard window, I will replace the hack with a \
> class-check. 
> 
> This addresses bugs dashboard, detection and hackish.
> https://bugs.kde.org/show_bug.cgi?id=dashboard
> https://bugs.kde.org/show_bug.cgi?id=detection
> https://bugs.kde.org/show_bug.cgi?id=hackish
> 
> 
> Diffs
> -----
> 
> trunk/KDE/kdebase/workspace/kwin/effects/CMakeLists.txt 1138357 
> trunk/KDE/kdebase/workspace/kwin/effects/configs_builtins.cpp 1138357 
> trunk/KDE/kdebase/workspace/kwin/effects/dashboard/CMakeLists.txt PRE-CREATION 
> trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.h PRE-CREATION 
> trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.cpp PRE-CREATION 
> trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.desktop PRE-CREATION 
> trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard_config.desktop \
> PRE-CREATION  trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard_config.h \
> PRE-CREATION  trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard_config.cpp \
> PRE-CREATION  trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard_config.ui \
> PRE-CREATION  trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboardeffectconfig.ui \
> PRE-CREATION  trunk/KDE/kdebase/workspace/plasma/desktop/shell/dashboardview.cpp \
> 1138357  trunk/KDE/kdelibs/plasma/windoweffects.h 1138355 
> trunk/KDE/kdelibs/plasma/windoweffects.cpp 1138355 
> 
> Diff: http://reviewboard.kde.org/r/4332/diff
> 
> 
> Testing
> -------
> 
> Code compiles, plugin loads, plugin configuration dialog is registered in KCM \
> Workspace module under "all effects". If the plugin is enabled, its settings apply \
> to the dashboard. 
> 
> Screenshots
> -----------
> 
> configuration dialog
> http://reviewboard.kde.org/r/4332/s/434/
> dashboard with modified background
> http://reviewboard.kde.org/r/4332/s/435/
> 
> 
> Thanks,
> 
> Andreas
> 
> 

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

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