From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: imagemmagick patch 5 Date: Fri, 05 Mar 2010 17:30:24 -0500 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 1267828359 5695 80.91.229.12 (5 Mar 2010 22:32:39 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 5 Mar 2010 22:32:39 +0000 (UTC) Cc: Emacs development discussions To: joakim@verona.se Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Mar 05 23:32:35 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 1Nng4Q-00011f-Kx for ged-emacs-devel@m.gmane.org; Fri, 05 Mar 2010 23:32:34 +0100 Original-Received: from localhost ([127.0.0.1]:60307 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nng4Q-0001JL-0x for ged-emacs-devel@m.gmane.org; Fri, 05 Mar 2010 17:32:34 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nng2g-0000it-Fz for emacs-devel@gnu.org; Fri, 05 Mar 2010 17:30:46 -0500 Original-Received: from [140.186.70.92] (port=50265 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nng2f-0000hG-7P for emacs-devel@gnu.org; Fri, 05 Mar 2010 17:30:46 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Nng2b-00079b-TO for emacs-devel@gnu.org; Fri, 05 Mar 2010 17:30:42 -0500 Original-Received: from tomts43.bellnexxia.net ([209.226.175.110]:52588 helo=tomts43-srv.bellnexxia.net) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Nng2b-00079X-IC for emacs-devel@gnu.org; Fri, 05 Mar 2010 17:30:41 -0500 Original-Received: from toip5.srvr.bell.ca ([209.226.175.88]) by tomts43-srv.bellnexxia.net (InterMail vM.5.01.06.13 201-253-122-130-113-20050324) with ESMTP id <20100305223030.UGCG1786.tomts43-srv.bellnexxia.net@toip5.srvr.bell.ca> for ; Fri, 5 Mar 2010 17:30:30 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AvsEAJkPkUtGN5E3/2dsb2JhbACbSHS2eIR3BIMX Original-Received: from bas1-montreal42-1178046775.dsl.bell.ca (HELO ceviche.home) ([70.55.145.55]) by toip5.srvr.bell.ca with ESMTP; 05 Mar 2010 17:29:43 -0500 Original-Received: by ceviche.home (Postfix, from userid 20848) id 3677AB4257; Fri, 5 Mar 2010 17:30:24 -0500 (EST) In-Reply-To: (joakim@verona.se's message of "Fri, 05 Mar 2010 23:06:18 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.91 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Solaris 8 (1) 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:121664 Archived-At: > 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 don't have much experience with such features, so I can't really usefully comment on them (in mpc.el I do scaling (externally, obviously) for album-covers, and indeed, I do it by specifying the target size, so it looks acceptable). But even if it's not 100% resolved, it can be installed, since it's the branch for Emacs-24, so there'll be time to adjust the API. > 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. There's nothing completely X specific in the patch I think, just > the same type of bitmap manipulations that goes on for the other > image libraries. So is the test in configure.in necessary? >> Please don't use the P_ macro in new code: just use prototypes. > Ok. Like this? > static int imagemagick_image_p (Lisp_Object object); Yes. Similarly the function definition shouldn't use K&R style any more (we didn't change all the existing code, but we try and do that for new code). >> 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. It's no big problem. Now that we accept and use prototypes, there probably aren't any compiler that can compile Emacs but does not accept // comments. Stefan PS: A few more things: + /* furthermore :rotation. we need background color and angle for rotation */ Please capitalize your comments, and use double-spaces at the end of sentences (including at the end of a comment, where we also want a full-stop): /* Furthermore :rotation. We need background color and angle for rotation. */ + if (FLOATP (value)){ Please put all "{" on their own line. + PixelWand* background = NewPixelWand(); Add a space before every open paren. + PixelSetColor (background, "#ffffff");/*TODO remove hardcode*/ Please use something like PixelSetColor (background, "#ffffff"); /* TODO remove hardcode. */ I.e. notice the "full stop plus 2 spaces". + status=MagickRotateImage (image_wand, background,rotation); Please add spaces around operators like "=". Also, please drop the "add empty lines" part of the hunk below as well. @@ -8521,12 +8991,20 @@ When an image has not been displayed this many seconds, remove it from the image cache. Value must be an integer or nil with nil meaning don't clear the cache. */); Vimage_cache_eviction_delay = make_number (30 * 60); + +#ifdef HAVE_IMAGEMAGICK + DEFVAR_LISP ("imagemagick-render-type", &Vimagemagick_render_type, + doc: /* */); +#endif } void init_image () { + } + + /* arch-tag: 123c2a5e-14a8-4c53-ab95-af47d7db49b9 (do not change this comment) */