[Sugar-devel] [PATCH] implements http://bugs.sugarlabs.org/ticket/1106 - Browse: No preview in Journal for downloaded image

[Sascha Silbe]

Excerpts from godiard's message of Thu Sep 23 15:42:15 +0200 2010:

> this patch adressed comments from silbe about segregate the check of a image mime type and use of sugar.style constant colors
> now opens the image in the original size because we don't want scale up small images
> also i did changes sugested by simon

Please adjust summary and description (and add a changelog) as explained
by Tomeu and Simon.

>  
> +

Methods should only be separated by one blank line, not two. Normally I
would recommend running pep8, but I haven't had time to clean up Browse
yet so it's hard to find new complaints among all the previously
existing ones, sorry. You might try redirecting the output to a file
and using comm:

$ git stash save
$ ~/sugar-jhbuild/sugar-jhbuild run pep8 --repeat *.py > pep8-old.log
$ git stash pop
$ ~/sugar-jhbuild/sugar-jhbuild run pep8 --repeat *.py > pep8-new.log
$ comm -3 pep8-old.log pep8-new.log |less

This will report a lot of line number changes, but the first few lines
of the output might be interesting.


> +    def _get_preview_image(self):
> +        preview_width, preview_height = style.zoom(300), style.zoom(225)

I still think using zoom(300) here is a bad idea, but will stop arguing
about it for now as sugar-toolkit does the same. We'll fix it up later.

> +        pixbuf = gtk.gdk.pixbuf_new_from_file(self._target_file.path)
> +        width, height = pixbuf.get_width(), pixbuf.get_height()

> +
> +

Blocks inside methods should be separated by a single blank line only.

> +        if (width > preview_width) or (height > preview_height):
> +            scale_x = float(width) / preview_width
> +            scale_y = float(height) / preview_height
> +            scale = max(scale_x, scale_y)
> +
> +            pixbuf = pixbuf.scale_simple(float(width) / scale, height / scale,
> +                                     gtk.gdk.INTERP_BILINEAR)

I guess you meant "width / scale"? scale_simple() expects integers.
If you use scale(), you can get rid of one pixbuf (currently you use up
to three).

> +        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)

This creates a border around an image that's smaller than the preview
size, thus wasting disk space. Please use width and height instead.

Thanks again for working on this!

Sascha

--
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
Url : http://lists.sugarlabs.org/archive/sugar-devel/attachments/20100923/7b9db87e/attachment.pgp