Re: [evolution-patches] Patch to resize images

[Srinivasa Ragavan <sragavan novell com>]

okie, ill try rework on it. and get back.

On Fri, 2005-07-01 at 13:21 +0800, Not Zed wrote:
> 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.
> 
> 
> _______________________________________________
> evolution-patches mailing list
> evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches