Re: [Banshee-List] [banshee] [NotificationArea] Send icon name instead of pixbuf

[Gabriel Burt <gabriel burt gmail com>]

A few issues/questions with this commit:

1) ideally mention the bgo# in the commit msg
2) use string instead of String for variable type defs
3) The Pixbufs possibly returned by LookupPixbuf etc are not disposed
4) Is it really appropriate to put a full path in the IconName?

Gabriel

On Sun, Apr 11, 2010 at 12:54 PM, Bertrand Lorentz
<blorentz src gnome org> wrote:
> commit 28e5533e2efc362f363fc4a84910ab2e610ca8b4
> Author: Chow Loong Jin <hyperair ubuntu com>
> Date:   Sun Apr 11 18:21:06 2010 +0800
>
>    [NotificationArea] Send icon name instead of pixbuf
>
>    For large artwork, sending a Gdk.Pixbuf across D-Bus can result in a lot
>    of data being spammed across the bus for large artwork. While this does
>    not usually cause performance issues, certain notification daemons like
>    notify-osd spend a considerable amount of memory storing this data prior
>    to displaying it.
>
>    Signed-off-by: Bertrand Lorentz <bertrand lorentz gmail com>
>
>  .../NotificationAreaService.cs                     |   28 +++++++++++--------
>  1 files changed, 16 insertions(+), 12 deletions(-)
> ---
> diff --git a/src/Extensions/Banshee.NotificationArea/Banshee.NotificationArea/NotificationAreaService.cs b/src/Extensions/Banshee.NotificationArea/Banshee.NotificationArea/NotificationAreaService.cs
> index 352f774..011e596 100644
> --- a/src/Extensions/Banshee.NotificationArea/Banshee.NotificationArea/NotificationAreaService.cs
> +++ b/src/Extensions/Banshee.NotificationArea/Banshee.NotificationArea/NotificationAreaService.cs
> @@ -43,6 +43,9 @@ using Banshee.Gui;
>  using Banshee.Collection.Gui;
>  using Banshee.MediaEngine;
>
> +using Banshee.IO;
> +using Banshee.Base;
> +
>  using Hyena.Gui;
>  using Hyena.Widgets;
>
> @@ -439,19 +442,20 @@ namespace Banshee.NotificationArea
>                 current_track.ArtistName, current_track.DisplayArtistName,
>                 current_track.AlbumTitle, current_track.DisplayAlbumTitle);
>
> -            Gdk.Pixbuf image = null;
> +            String image = null;
>
> -            if (artwork_manager_service != null) {
> -                image = is_notification_daemon
> -                    ? artwork_manager_service.LookupScalePixbuf (current_track.ArtworkId, icon_size)
> -                    : artwork_manager_service.LookupPixbuf (current_track.ArtworkId);
> -            }
> +            image = is_notification_daemon
> +                ? CoverArtSpec.GetPathForSize (current_track.ArtworkId, icon_size)
> +                : CoverArtSpec.GetPath (current_track.ArtworkId);
>
> -            if (image == null) {
> -                image = IconThemeUtils.LoadIcon (48, "audio-x-generic");
> -                if (image != null) {
> -                    image.ScaleSimple (icon_size, icon_size, Gdk.InterpType.Bilinear);
> -                }
> +            if (!File.Exists (new SafeUri(image))) {
> +                // artwork does not exist, try looking up the pixbuf to trigger scaling or conversion
> +                if (artwork_manager_service == null ||
> +                    (is_notification_daemon &&
> +                     artwork_manager_service.LookupScalePixbuf (current_track.ArtworkId, icon_size) == null) ||
> +                    (!is_notification_daemon &&
> +                     artwork_manager_service.LookupPixbuf (current_track.ArtworkId) == null))
> +                    image = "audio-x-generic";
>             }
>
>             try {
> @@ -461,7 +465,7 @@ namespace Banshee.NotificationArea
>                 } else {
>                     current_nf.Summary = current_track.DisplayTrackTitle;
>                     current_nf.Body = message;
> -                    current_nf.Icon = image;
> +                    current_nf.IconName = image;
>                     current_nf.AttachToWidget (notif_area.Widget);
>                 }
>                 current_nf.Urgency = Urgency.Low;
> _______________________________________________
> commits-list mailing list (read only)
> http://mail.gnome.org/mailman/listinfo/commits-list
>
> Want to limit the commits to a few modules? Go to above URL, log in to edit your options and select the modules ('topics') you want.
>