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


I must agree with Aaron: I'm really glad we finally get a Dashboard effect (I 
remember that I wrote one about 4.1, but at that time Dashboard was so broken 
that I stopped working on it :-P)

I'm not sure about the config options. Having them in effect config makes it a 
poweruser feature nobody will find. It's fine for development but for the 
release it's kind of useless. Such option might make sense in the dashboard 
config, but in general it looks quite a bit KDE 3-ish to change the level of 
gray ;-)

The dashboard effect has the potential to be astonishing and I think it needs 
some Nuno love. Something special like we did for the logout effect. A kind of 
decent blur, so that we could enable it without harming the users who want to 
read something in the window. I need to ask Nuno...

And if Plasma devs split Plasmoids from the wallpaper (needs KWin help) we can 
even do more awesome stuff like fading the Plasmoids to the top through all 
windows for same widget sets and a nice animation for dashboard widget set.


trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.cpp
<http://reviewboard.kde.org/r/4332/#comment5762>

    Where does this magic number come from? And why is it required?



trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.cpp
<http://reviewboard.kde.org/r/4332/#comment5764>

    we have reconfigure for reading the configuration values :-)



trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.cpp
<http://reviewboard.kde.org/r/4332/#comment5763>

    In the dtor it's required to remove the property again. Please have a look 
at other effects which use xatoms.



trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.cpp
<http://reviewboard.kde.org/r/4332/#comment5766>

    I would use percentage as brightness/saturation is such a nice [0.0, 1.0] 
interval



trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.cpp
<http://reviewboard.kde.org/r/4332/#comment5765>

    Don't set this in each painting step. Set it in window activated. And as 
region you should use w->geometry(). The region is the area which is painted in 
the current rendering step. If another effect increases the region only parts 
of the dashboard would be blurred.



trunk/KDE/kdebase/workspace/kwin/effects/dashboard/dashboard.desktop
<http://reviewboard.kde.org/r/4332/#comment5761>

    No need to translate, scripty will automatically overwrite it.


- Martin


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

Reply via email to