[Sugar-devel] [PATCH] implements #1106 - Browse: No preview in Journal for downloaded image

[Simon Schampijer]

Hi Gonzalo,

thanks for your patch!

> ---
>   downloadmanager.py |   32 ++++++++++++++++++++++++++++++++
>   1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/downloadmanager.py b/downloadmanager.py
> index 3eec649..8a90964 100644
> --- a/downloadmanager.py
> +++ b/downloadmanager.py
> @@ -36,6 +36,7 @@ from sugar import profile
>   from sugar import mime
>   from sugar.graphics.alert import Alert, TimeoutAlert
>   from sugar.graphics.icon import Icon
> +from sugar.graphics import style
>   from sugar.activity import activity
>
>   # #3903 - this constant can be removed and assumed to be 1 when dbus-python
> @@ -192,12 +193,43 @@ class Download:
>                   sniffed_mime_type = mime.get_for_file(self._target_file.path)
>                   self.dl_jobject.metadata['mime_type'] = sniffed_mime_type
>
> +            pixbuf_mime_types = []
> +            for format in gtk.gdk.pixbuf_get_formats():
> +                pixbuf_mime_types.extend(format['mime_types'])
> +
> +            if self._mime_type in pixbuf_mime_types:
> +                self.dl_jobject.metadata['preview'] = self.__get_preview_image()
> +            else:
> +                self.dl_jobject.metadata['preview'] = ''

Shall we maybe just not write the pixbuf if we can not handle the mime_type?

>               datastore.write(self.dl_jobject,
>                               transfer_ownership=True,
>                               reply_handler=self._internal_save_cb,
>                               error_handler=self._internal_save_error_cb,
>                               timeout=360 * DBUS_PYTHON_TIMEOUT_UNITS_PER_SECOND)
>
> +    def __get_preview_image(self):

Please use one underscore for private methods. Two underscores for 
callbacks.

> +        preview_width, preview_height = style.zoom(300), style.zoom(225)

I like that you use variables here so one knows as well where the 
numbers 300 and 225 comes from. Even better than the activity.py code :)

> +        pixbuf = gtk.gdk.pixbuf_new_from_file_at_size(self._target_file.path, preview_width, preview_height)
> +        width, height = pixbuf.get_width(), pixbuf.get_height()
> +
> +        pixbuf2 = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB, pixbuf.get_has_alpha(),
> +            pixbuf.get_bits_per_sample(), preview_width, preview_height)
> +        pixbuf2.fill(0xffffffff)

Can we use a constant here that explains what 0xffffffff is for?

> +        margin_x, margin_y = (preview_width - width) / 2, (preview_height - height) / 2
> +
> +        pixbuf.copy_area(0, 0, width, height, pixbuf2, margin_x, margin_y)
> +
> +        preview_data = []
> +        def save_func(buf, data):
> +            data.append(buf)
> +
> +        pixbuf2.save_to_callback(save_func, 'png', user_data=preview_data)
> +        preview_data = ''.join(preview_data)
> +        return dbus.ByteArray(preview_data)
> +
>       def __start_response_cb(self, alert, response_id):
>           global _active_downloads
>           if response_id is gtk.RESPONSE_CANCEL:

All the rest looks good to me.
    Simon