[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: imagemmagick patch 5
From: |
joakim |
Subject: |
Re: imagemmagick patch 5 |
Date: |
Thu, 15 Apr 2010 12:55:03 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1.90 (gnu/linux) |
address@hidden writes:
> address@hidden writes:
>
>> Stefan Monnier <address@hidden> writes:
>>
>>>> The Imagemagick patch allows Emacs to use the Imagemagick libraries to
>>>> load images. This support can be used in parallell with the existing
>>>> image loading support. There is support for asking your imagemagick
>>>> installation which image types it supports, and registering them in
>>>> Emacs selectively.
>>>
>>> Thanks. Looks pretty good. Feel free to install it in the `pending'
>>> branch, but please see the comments below first, and don't forget to add
>>> a good NEWS entry when you install this change.
>>
>> Ok. I will need to read up on bzr before committing.
>
> I've done a local bzr branch now, called "imagemagick". I would now like
> to publish this branch.
>
> http://www.emacswiki.org/emacs/BzrForEmacsDevsmentions ways of publishing a
> branch to launchpad etc, but I cant find
> how to publish it on savannah. Could some kind soul enlighten me?
I tried this, which seemingly works:
bzr push sftp://address@hidden/srv/bzr/emacs/imagemagick
>
>
>
>> Could you possibly also comment on the interface I chose for scaling and
>> rotation? If they are ok, I will document them in the NEWS entry. I
>> assume the "index" spec is ok since that is also used for gifs already.
>>
>> Also please comment if the interface to select which image types
>> imagemagick gets to handle is ok.
>>
>>>
>>>> +HAVE_IMAGEMAGICK=no
>>>> +if test "${HAVE_X11}" = "yes" ; then
>>>
>>> Do I understand it right that this X11-only restriction could be lifted
>>> at some point in the future?
>>
>> Yes. Theres nothing completely X specific in the patch I think, just the
>> same type of bitmap manipulations that goes on for the other image
>> libraries.
>>
>>>> diff --git a/src/dbusbind.c b/src/dbusbind.c
>>>> index 7c0be49..7a47730 100644
>>>> --- a/src/dbusbind.c
>>>> +++ b/src/dbusbind.c
>>>> @@ -773,6 +773,7 @@ xd_add_watch (watch, data)
>>>> if (fd == -1)
>>>> return FALSE;
>>>>
>>>> +
>>>> /* Add the file descriptor to input_wait_mask. */
>>>> add_keyboard_wait_descriptor (fd);
>>>> }
>>>
>>> Please drop this gratuitous change.
>>
>> Hmm, I have no idea how this crept in. Odd.
>>
>>
>>>> +/***********************************************************************
>>>> + imagemagick
>>>> + ***********************************************************************/
>>>> +#if defined (HAVE_IMAGEMAGICK)
>>>> +Lisp_Object Vimagemagick_render_type;
>>>> +/* Function prototypes. */
>>>> +
>>>> +static int imagemagick_image_p P_ ((Lisp_Object object));
>>>> +static int imagemagick_load P_ ((struct frame *f, struct image *img));
>>>> +
>>>> +static int imagemagick_load_image P_ ((struct frame *, struct image *,
>>>> + unsigned char *, unsigned int,
>>>> unsigned char *));
>>>
>>> Please don't use the P_ macro in new code: just use prototypes.
>>
>> Ok. Like this?
>>
>> static int imagemagick_image_p (Lisp_Object object);
>> static int imagemagick_load (struct frame *f, struct image *img);
>>
>> static int imagemagick_load_image (struct frame *, struct image *,
>> unsigned char *, unsigned int,
>> unsigned char *);
>>
>>
>>>> +//#include
>>>> "/home/joakim/current/unison/data/ImageMagick-6.5.4-7/magick/xwindow-private.h"
>>>
>>> This should be removed as well.
>>
>> Ok.
>>
>>>> + //try if magicexportimage is any faster than pixelpushing
>>> [...]
>>>> + //oddly, the below code doesnt seem to work:
>>>
>>> We currently only use /*...*/ comments, so I'd rather we don't introduce
>>> // comments for now.
>>>
>>
>> Sorry, I keep forgetting this, it seems to be in my muscle memory.
>>
>>> Stefan
>>>
--
Joakim Verona