Re: [evolution-patches] Patch to resize images

[Not Zed <notzed novell com>]

On Mon, 2005-06-27 at 15:27 +0530, Srinivasa Ragavan wrote:
> Hah, I hope i have worked on most of the comments...
> 
> - I have reused em-icon-stream

"em_icon_stream_find_pixbuf"??  Its just an exact copy of
em_icon_stream_get_image, only it doesn't ref things properly?  Proper
refcounting here is extremely critical because of the size of the
objects, and adding redundant extra api's is pointless.

Instead of both set_pixbuf and replace_pixbuf, you only need one, they
do the same thing - actually I think you dont need any of them.

The whole replace/set stuff just complicates all the logic too much.
You always need to keep the original around, you just need to be able to
get it scaled sometimes.

I think it would be easier just either expand em_icon_stream_get_image()
to add a 'scale' argument.  It can then produce a scaled image if it
doesn't have one in the cache, etc.  Then all you need to do is store a
scale factor on the puri and remove ALL the scaling code in
format-html-display.

It would be nice if get_image returned a pixmap too, more efficient
(internally it would still have to store the original as a pixbuf, for
scaling).

> - Using my own puri

But you're still using the gobject_set_data stuff.  As a pretty shitty
means to communicate across abstraction boundaries at that.

> - Fixed Object hierarchy issue
> - reduced the complexity of the resize function

It looks pretty inefficient.  Copying stuff around, etc.  Its not really
that hard.  You have the original, if it doesn't fit then make a scaled
one.  Anyway the outline above removes the need for all of this.

> - Closed most of the memory leaks

Most isn't even close to good enough.  There are several obvious ones
still left.

The 'key' one is particularly baffling, you're just copying something
you have access to anyway.