From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: joakim@verona.se Newsgroups: gmane.emacs.devel Subject: Re: imagemmagick patch 5 Date: Fri, 05 Mar 2010 23:06:18 +0100 Message-ID: References: NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: dough.gmane.org 1267826821 593 80.91.229.12 (5 Mar 2010 22:07:01 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 5 Mar 2010 22:07:01 +0000 (UTC) Cc: Emacs development discussions To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Mar 05 23:06:57 2010 connect(): Connection refused Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1NnffI-0006mO-SK for ged-emacs-devel@m.gmane.org; Fri, 05 Mar 2010 23:06:37 +0100 Original-Received: from localhost ([127.0.0.1]:36890 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NnffI-0002Ck-CY for ged-emacs-devel@m.gmane.org; Fri, 05 Mar 2010 17:06:36 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NnffC-0002CZ-Ej for emacs-devel@gnu.org; Fri, 05 Mar 2010 17:06:30 -0500 Original-Received: from [140.186.70.92] (port=50433 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NnffB-0002CL-IO for emacs-devel@gnu.org; Fri, 05 Mar 2010 17:06:30 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Nnff9-0005N9-QT for emacs-devel@gnu.org; Fri, 05 Mar 2010 17:06:29 -0500 Original-Received: from iwfs.imcode.com ([82.115.149.64]:60926 helo=gate.verona.se) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Nnff9-0005Kx-H2 for emacs-devel@gnu.org; Fri, 05 Mar 2010 17:06:27 -0500 Original-Received: from localhost.localdomain (IDENT:1005@localhost [127.0.0.1]) by gate.verona.se (8.13.4/8.11.4) with ESMTP id o25M6Ixu007587; Fri, 5 Mar 2010 23:06:19 +0100 In-Reply-To: (Stefan Monnier's message of "Wed, 03 Mar 2010 21:41:03 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.90 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4-2.6 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:121661 Archived-At: Stefan Monnier 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. 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