* bug#33587: [PROPOSED] Default to disabling ImageMagick @ 2018-12-02 18:09 Paul Eggert 2018-12-02 18:15 ` Eli Zaretskii ` (3 more replies) 0 siblings, 4 replies; 14+ messages in thread From: Paul Eggert @ 2018-12-02 18:09 UTC (permalink / raw) To: 33587; +Cc: Paul Eggert From: Paul Eggert <eggert@Penguin.CS.UCLA.EDU> ImageMagick has continuing stability and security problems, suggesting that 'configure' should disable it by default. See Glenn Morris's notes at: https://lists.gnu.org/r/emacs-devel/2018-12/msg00036.html * INSTALL, etc/NEWS: Mention this. * configure.ac (imagemagick): Default to off. --- INSTALL | 4 +++- configure.ac | 2 +- etc/NEWS | 4 ++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/INSTALL b/INSTALL index 0c56fff6d4..9696904dce 100644 --- a/INSTALL +++ b/INSTALL @@ -294,7 +294,9 @@ or more of these options: --without-gif for GIF image support --without-png for PNG image support --without-rsvg for SVG image support - --without-imagemagick for Imagemagick support + +Although ImageMagick support is disabled by default due to security +and stability concerns, you can enable it with --with-imagemagick. Use --without-toolkit-scroll-bars to disable Motif or Xaw3d scroll bars. diff --git a/configure.ac b/configure.ac index 8b34c3b658..b70393925a 100644 --- a/configure.ac +++ b/configure.ac @@ -354,7 +354,7 @@ AC_DEFUN OPTION_DEFAULT_ON([libsystemd],[don't compile with libsystemd support]) OPTION_DEFAULT_OFF([cairo],[compile with Cairo drawing (experimental)]) OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support]) -OPTION_DEFAULT_ON([imagemagick],[don't compile with ImageMagick image support]) +OPTION_DEFAULT_OFF([imagemagick],[compile with ImageMagick image support]) OPTION_DEFAULT_ON([json], [don't compile with native JSON support]) OPTION_DEFAULT_ON([xft],[don't use XFT for anti aliased fonts]) diff --git a/etc/NEWS b/etc/NEWS index 6297d07879..07c6f74c44 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -37,6 +37,10 @@ functions 'json-serialize', 'json-insert', 'json-parse-string', and 'json-parse-buffer' are typically much faster than their Lisp counterparts from json.el. +** Emacs no longer defaults to using ImageMagick to display images, +due to security and stability concerns. To override the default, use +'configure --with-imagemagick'. + ** The etags program now uses the C library's regular expression matcher when possible, and a compatible regex substitute otherwise. This will let developers maintain Emacs's own regex code without having to also -- 2.19.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-02 18:09 bug#33587: [PROPOSED] Default to disabling ImageMagick Paul Eggert @ 2018-12-02 18:15 ` Eli Zaretskii 2018-12-02 19:13 ` Andreas Schwab ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Eli Zaretskii @ 2018-12-02 18:15 UTC (permalink / raw) To: Paul Eggert; +Cc: 33587, eggert > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Sun, 2 Dec 2018 10:09:19 -0800 > Cc: Paul Eggert <eggert@Penguin.CS.UCLA.EDU> > > From: Paul Eggert <eggert@Penguin.CS.UCLA.EDU> > > ImageMagick has continuing stability and security problems, suggesting > that 'configure' should disable it by default. See Glenn Morris's notes > at: https://lists.gnu.org/r/emacs-devel/2018-12/msg00036.html > * INSTALL, etc/NEWS: Mention this. > * configure.ac (imagemagick): Default to off. No objections from me, but let's please wait for a week, to let people chance to voice objections. Thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-02 18:09 bug#33587: [PROPOSED] Default to disabling ImageMagick Paul Eggert 2018-12-02 18:15 ` Eli Zaretskii @ 2018-12-02 19:13 ` Andreas Schwab 2018-12-02 23:51 ` Paul Eggert 2018-12-03 19:08 ` Glenn Morris 2018-12-10 17:49 ` Paul Eggert 3 siblings, 1 reply; 14+ messages in thread From: Andreas Schwab @ 2018-12-02 19:13 UTC (permalink / raw) To: Paul Eggert; +Cc: 33587, Paul Eggert On Dez 02 2018, Paul Eggert <eggert@cs.ucla.edu> wrote: > +** Emacs no longer defaults to using ImageMagick to display images, > +due to security and stability concerns. To override the default, use > +'configure --with-imagemagick'. ImageMagick is the only backend that supports scaling. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-02 19:13 ` Andreas Schwab @ 2018-12-02 23:51 ` Paul Eggert 2018-12-03 21:09 ` Alan Third 0 siblings, 1 reply; 14+ messages in thread From: Paul Eggert @ 2018-12-02 23:51 UTC (permalink / raw) To: Andreas Schwab; +Cc: 33587, Paul Eggert Andreas Schwab wrote: > ImageMagick is the only backend that supports scaling. Good point, and if we make the change, the scaling issue should be mentioned in INSTALL. Perhaps something like the following wording: "Although ImageMagick support is disabled by default due to security and stability concerns, you can enable it by configuring with --with-imagemagick. ImageMagick is the only backend that supports image scaling." ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-02 23:51 ` Paul Eggert @ 2018-12-03 21:09 ` Alan Third 0 siblings, 0 replies; 14+ messages in thread From: Alan Third @ 2018-12-03 21:09 UTC (permalink / raw) To: Paul Eggert; +Cc: 33587, Andreas Schwab, Paul Eggert On Sun, Dec 02, 2018 at 03:51:57PM -0800, Paul Eggert wrote: > Andreas Schwab wrote: > > ImageMagick is the only backend that supports scaling. > > Good point, and if we make the change, the scaling issue should be mentioned > in INSTALL. Perhaps something like the following wording: > > "Although ImageMagick support is disabled by default due to security > and stability concerns, you can enable it by configuring with > --with-imagemagick. ImageMagick is the only backend that supports > image scaling." FWIW the NS port on master supports scaling through the NS toolkit, although there is the problem that most lisp code that wants to scale checks exclusively for imagemagick support. -- Alan Third ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-02 18:09 bug#33587: [PROPOSED] Default to disabling ImageMagick Paul Eggert 2018-12-02 18:15 ` Eli Zaretskii 2018-12-02 19:13 ` Andreas Schwab @ 2018-12-03 19:08 ` Glenn Morris 2018-12-03 19:35 ` Paul Eggert 2018-12-04 16:51 ` David Engster 2018-12-10 17:49 ` Paul Eggert 3 siblings, 2 replies; 14+ messages in thread From: Glenn Morris @ 2018-12-03 19:08 UTC (permalink / raw) To: Paul Eggert; +Cc: 33587 I'm a bit surprised by the lack of objections so far, though it's early days yet of course. Maybe it's an experiment that needs to be tried out for the implications to be seen. A related alternative would be to lower the priority of the ImageMagick backend. At the moment, visiting eg a png image uses ImageMagick rather than libpng if both are linked in. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-03 19:08 ` Glenn Morris @ 2018-12-03 19:35 ` Paul Eggert 2018-12-03 19:40 ` Glenn Morris 2018-12-04 16:51 ` David Engster 1 sibling, 1 reply; 14+ messages in thread From: Paul Eggert @ 2018-12-03 19:35 UTC (permalink / raw) To: Glenn Morris; +Cc: 33587 On 12/3/18 11:08 AM, Glenn Morris wrote: > A related alternative would be to lower the priority of the ImageMagick > backend. At the moment, visiting eg a png image uses ImageMagick rather > than libpng if both are linked in. If this alternative is taken and the user requests scaling, presumably the ImageMagick library would need to be used anyway since it can scale and libpng can't. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-03 19:35 ` Paul Eggert @ 2018-12-03 19:40 ` Glenn Morris 0 siblings, 0 replies; 14+ messages in thread From: Glenn Morris @ 2018-12-03 19:40 UTC (permalink / raw) To: Paul Eggert; +Cc: 33587 Paul Eggert wrote: > On 12/3/18 11:08 AM, Glenn Morris wrote: >> A related alternative would be to lower the priority of the ImageMagick >> backend. At the moment, visiting eg a png image uses ImageMagick rather >> than libpng if both are linked in. > > If this alternative is taken and the user requests scaling, presumably > the ImageMagick library would need to be used anyway since it can > scale and libpng can't. Sure. I mean, make use of ImageMagick require an explicit request, for uses that might need those features (eww?), rather than just happening by default like it does now. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-03 19:08 ` Glenn Morris 2018-12-03 19:35 ` Paul Eggert @ 2018-12-04 16:51 ` David Engster 2018-12-04 17:00 ` Glenn Morris 1 sibling, 1 reply; 14+ messages in thread From: David Engster @ 2018-12-04 16:51 UTC (permalink / raw) To: Glenn Morris; +Cc: Paul Eggert, 33587 Glenn Morris writes: > I'm a bit surprised by the lack of objections so far, though it's early > days yet of course. Maybe it's an experiment that needs to be tried out > for the implications to be seen. Well, I do depend on image scaling, but I (like many others here, I guess) build Emacs myself, so defaults don't matter much to me. Question is: will disabling Imagemagick by default also have an impact on how Emacs is shipped in distributions? I don't think so, at least as long as they don't drop Imagemagick completely. If for instance Debian has to take care of Imagemagick security issues anyway, why shouldn't Emacs link to it? But that's just my guess... -David ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-04 16:51 ` David Engster @ 2018-12-04 17:00 ` Glenn Morris 2018-12-04 17:38 ` David Engster 2018-12-04 18:16 ` Glenn Morris 0 siblings, 2 replies; 14+ messages in thread From: Glenn Morris @ 2018-12-04 17:00 UTC (permalink / raw) To: David Engster; +Cc: Paul Eggert, 33587 David Engster wrote: > Question is: will disabling Imagemagick by default also have an impact > on how Emacs is shipped in distributions? I don't know. It depends whether they go with the default configure options or not. > I don't think so, at least as long as they don't drop Imagemagick > completely. Note that Red Hat Enterprise Linux 8 _will_ drop ImageMagick completely (though it will probably be available from an add-on repository), presumably because they don't feel able to keep up with the security issues. That's what prompted me to first raise this in http://lists.gnu.org/r/emacs-devel/2018-12/msg00036.html > If for instance Debian has to take care of Imagemagick security issues > anyway, why shouldn't Emacs link to it? (For reference: https://security-tracker.debian.org/tracker/source-package/imagemagick ) Because one can never guarantee all security issues are fixed, and if a project has a history of having a lot of them, it may be considered likely to be insecure. Also there are the various Emacs crash reports due to ImageMagick. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-04 17:00 ` Glenn Morris @ 2018-12-04 17:38 ` David Engster 2018-12-04 18:16 ` Glenn Morris 1 sibling, 0 replies; 14+ messages in thread From: David Engster @ 2018-12-04 17:38 UTC (permalink / raw) To: Glenn Morris; +Cc: Paul Eggert, 33587 Glenn Morris writes: > Note that Red Hat Enterprise Linux 8 _will_ drop ImageMagick completely > (though it will probably be available from an add-on repository), > presumably because they don't feel able to keep up with the security > issues. That's what prompted me to first raise this in > > http://lists.gnu.org/r/emacs-devel/2018-12/msg00036.html RHEL can do this because they're supporting way less packages than other distributions. As you know, enterprise customers have other priorities than home desktop users. Debian cannot remove Imagemagick because many other packages depend on it, at least currently. >> If for instance Debian has to take care of Imagemagick security issues >> anyway, why shouldn't Emacs link to it? > > (For reference: > https://security-tracker.debian.org/tracker/source-package/imagemagick ) > > Because one can never guarantee all security issues are fixed, and if a > project has a history of having a lot of them, it may be considered > likely to be insecure. Also there are the various Emacs crash reports > due to ImageMagick. I understand the reasoning. To me, image scaling is essential for what I'm doing with Emacs, so I'm willing to take that risk. But that's just one data point. Don't get me wrong: I don't object to disable it by default. Let's see what happens. Maybe distributions will then disable it as well, but they have their own ways to see how changes like these affect users (by having an 'unstable' tree or whatever). -David ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-04 17:00 ` Glenn Morris 2018-12-04 17:38 ` David Engster @ 2018-12-04 18:16 ` Glenn Morris 1 sibling, 0 replies; 14+ messages in thread From: Glenn Morris @ 2018-12-04 18:16 UTC (permalink / raw) To: David Engster; +Cc: Paul Eggert, 33587 PS GraphicsMagick allegedly has fewer security issues than ImageMagick, but https://debbugs.gnu.org/14358 saw no interest. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2018-12-02 18:09 bug#33587: [PROPOSED] Default to disabling ImageMagick Paul Eggert ` (2 preceding siblings ...) 2018-12-03 19:08 ` Glenn Morris @ 2018-12-10 17:49 ` Paul Eggert 3 siblings, 0 replies; 14+ messages in thread From: Paul Eggert @ 2018-12-10 17:49 UTC (permalink / raw) To: 33587; +Cc: Alan Third, David Engster Elias Mårtenson wrote in <http://lists.gnu.org/r/emacs-devel/2018-12/msg00186.html> that image scaling via Xrender is surprisingly simple. So perhaps an X11 expert could investigate doing that for the X Window System, when ImageMagick scaling is not available or not used. My impression is that the Xrender extension (introduced in 2000) is reasonably popular among X11 servers these days. Scaling on the server could also be faster (e.g., with hardware acceleration) and/or more reliable, so quite possibly it'd be better to use Xrender to scale even if ImageMagick is available. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Add native image scaling @ 2019-01-02 16:11 Eli Zaretskii 2019-01-02 21:12 ` [PATCH v2] Add native image scaling (bug#33587) Alan Third 0 siblings, 1 reply; 14+ messages in thread From: Eli Zaretskii @ 2019-01-02 16:11 UTC (permalink / raw) To: Alan Third; +Cc: emacs-devel > Date: Tue, 1 Jan 2019 21:47:19 +0000 > From: Alan Third <alan@idiocy.org> > Cc: emacs-devel@gnu.org > > This is a completely rewritten version that doesn't do anything > different with the image cache. I think this is pretty complete as it > is, excepting Windows support. I don't know how to add Windows support > to match the way it's done in NS and X, however if we can't resize the > images up-front, it should be possible to add some size info to the > image struct and do the resize on display. Thanks, I have only minor comments, after which this can be pushed. Support for MS-Windows can be added later. > +#ifdef HAVE_XRENDER > +static bool x_create_x_image_and_pixmap (struct frame *, int, int, int, > + XImagePtr *, Pixmap *, Picture *); > +#else > static bool x_create_x_image_and_pixmap (struct frame *, int, int, int, > XImagePtr *, Pixmap *); > +#endif > static void x_destroy_x_image (XImagePtr ximg); Here and elsewhere, I'd prefer not to change the signature of functions with and without the scaling support. Instead, let's use the same signature, where the extra argument(s) are left unused when scaling is not supported. If some data types will be undefined without including headers which are available only when the feature is supported, we can either use a void pointer, or have a dummy data type definition for those arguments. > +DEFUN ("image-scaling-p", Fimage_scaling_p, Simage_scaling_p, 0, 1, 0, > + doc: /* Test whether FRAME supports resizing images. > +Return t if FRAME supports native scaling, nil otherwise. */) > + (Lisp_Object frame) This primitive should be documented both in NEWS and in the ELisp manual. Thanks! ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] Add native image scaling (bug#33587) 2019-01-02 16:11 [PATCH] Add native image scaling Eli Zaretskii @ 2019-01-02 21:12 ` Alan Third 2019-01-04 14:31 ` Eli Zaretskii 0 siblings, 1 reply; 14+ messages in thread From: Alan Third @ 2019-01-02 21:12 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel * configure.ac: Test for XRender outside of xft checks. * src/Makefile.in (XRENDER_LIBS): List XRender libs separately from xft libs. * lisp/image.el (image--get-imagemagick-and-warn): Allow resizing if native scaling is available. * src/dispextern.h: Add XRender and image scaling stuff. (struct image): Add XRender Pictures. * src/image.c (x_create_bitmap_mask): (image_create_x_image_and_pixmap): Handle XRender Picture. (scale_image_size): (compute_image_size): Make available when any form of scaling is enabled. (x_set_image_size): New function. (lookup_image): Set image size. (x_create_x_image_and_pixmap): Create XRender Picture when necessary. (x_put_x_image): Handle the case where desired size != actual size. (free_image): Free XRender Pictures. (Fimage_scaling_p): New function. (syms_of_image): Add image-scaling-p. * src/nsimage.m (ns_load_image): Remove NS specific resizing. ([EmacsImage setSizeFromSpec:]): Remove method. (ns_image_set_size): New function. * src/nsterm.m (ns_draw_fringe_bitmap): Cocoa and GNUstep both have the same compositing functions, so remove unnecessary difference. * src/xterm.c (x_composite_image): New function. (x_draw_image_foreground): Use new x_composite_image function. * doc/lispref/display.texi (Image Descriptors): Document image-scaling-p and add resizing descriptors. (ImageMagick Images): Remove resizing descriptors. --- I think this is the final version. I would appreciate if someone who knows their way around image handling would be able to test it. I’m particularly concerned that I’ve probably broken masks. I can’t find any examples of how to use them online, and they don’t work at all in NS, so I don’t know if they get any use. configure.ac | 14 +- doc/lispref/display.texi | 88 ++++++----- etc/NEWS | 6 + lisp/image.el | 4 +- src/Makefile.in | 3 +- src/dispextern.h | 13 ++ src/image.c | 312 ++++++++++++++++++++++++++------------- src/nsimage.m | 68 +-------- src/nsterm.h | 2 +- src/nsterm.m | 8 - src/xterm.c | 58 ++++++-- 11 files changed, 343 insertions(+), 233 deletions(-) diff --git a/configure.ac b/configure.ac index 91fa417308..0c59fe0619 100644 --- a/configure.ac +++ b/configure.ac @@ -3241,6 +3241,17 @@ AC_DEFUN CFLAGS=$late_CFLAGS fi +# Check for XRender +HAVE_XRENDER=no +if test "${HAVE_X11}" = "yes"; then + AC_CHECK_LIB(Xrender, XRenderQueryExtension, HAVE_XRENDER=yes) + if test $HAVE_XRENDER = yes; then + XRENDER_LIBS="-lXrender" + AC_SUBST(XRENDER_LIBS) + AC_DEFINE([HAVE_XRENDER], 1, [Define to 1 if XRender is available.]) + fi +fi + ### Start of font-backend (under any platform) section. # (nothing here yet -- this is a placeholder) ### End of font-backend (under any platform) section. @@ -3263,15 +3274,12 @@ AC_DEFUN EMACS_CHECK_MODULES([XFT], [xft >= 0.13.0], [], [HAVE_XFT=no]) ## Because xterm.c uses XRenderQueryExtension when XFT is ## enabled, we also need to link to -lXrender. - HAVE_XRENDER=no - AC_CHECK_LIB(Xrender, XRenderQueryExtension, HAVE_XRENDER=yes) if test "$HAVE_XFT" != no && test "$HAVE_XRENDER" != no; then OLD_CPPFLAGS="$CPPFLAGS" OLD_CFLAGS="$CFLAGS" OLD_LIBS="$LIBS" CPPFLAGS="$CPPFLAGS $XFT_CFLAGS" CFLAGS="$CFLAGS $XFT_CFLAGS" - XFT_LIBS="-lXrender $XFT_LIBS" LIBS="$XFT_LIBS $LIBS" AC_CHECK_HEADER(X11/Xft/Xft.h, AC_CHECK_LIB(Xft, XftFontOpen, HAVE_XFT=yes, , $XFT_LIBS) , , diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi index 19424ecc7e..f311cd73df 100644 --- a/doc/lispref/display.texi +++ b/doc/lispref/display.texi @@ -5112,6 +5112,45 @@ Image Descriptors @var{relief} is negative, shadows are drawn so that the image appears as a pressed button; otherwise, it appears as an unpressed button. +@item :width @var{width}, :height @var{height} +The @code{:width} and @code{:height} keywords are used for scaling the +image. If only one of them is specified, the other one will be +calculated so as to preserve the aspect ratio. If both are specified, +aspect ratio may not be preserved. + +@item :max-width @var{max-width}, :max-height @var{max-height} +The @code{:max-width} and @code{:max-height} keywords are used for +scaling if the size of the image of the image exceeds these values. +If @code{:width} is set it will have precedence over @code{max-width}, +and if @code{:height} is set it will have precedence over +@code{max-height}, but you can otherwise mix these keywords as you +wish. @code{:max-width} and @code{:max-height} will always preserve +the aspect ratio. + +If both @code{:width} and @code{:max-height} has been set (but +@code{:height} has not been set), then @code{:max-height} will have +precedence. The same is the case for the opposite combination: The +``max'' keyword has precedence. That is, if you have a 200x100 image +and specify that @code{:width} should be 400 and @code{:max-height} +should be 150, you'll end up with an image that is 300x150: Preserving +the aspect ratio and not exceeding the ``max'' setting. This +combination of parameters is a useful way of saying ``display this +image as large as possible, but no larger than the available display +area''. + +@item :scale @var{scale} +This should be a number, where values higher than 1 means to increase +the size, and lower means to decrease the size. For instance, a value +of 0.25 will make the image a quarter size of what it originally was. +If the scaling makes the image larger than specified by +@code{:max-width} or @code{:max-height}, the resulting size will not +exceed those two values. If both @code{:scale} and +@code{:height}/@code{:width} are specified, the height/width will be +adjusted by the specified scaling factor. + +@item :index @var{frame} +@xref{Multi-Frame Images}. + @item :conversion @var{algorithm} This specifies a conversion algorithm that should be applied to the image before it is displayed; the value, @var{algorithm}, specifies @@ -5251,6 +5290,16 @@ Image Descriptors (@pxref{Input Focus}). @end defun +@defun image-scaling-p &optional frame +This function returns @code{t} if @var{frame} supports image scaling. +@var{frame} @code{nil} or omitted means to use the selected frame +(@pxref{Input Focus}). + +If image scaling is not supported, @code{:width}, @code{:height}, +@code{:scale}, @code{:max-width} and @code{:max-height} will only be +usable through ImageMagick, if available (@pxref{ImageMagick Images}). +@end defun + @node XBM Images @subsection XBM Images @cindex XBM @@ -5387,42 +5436,6 @@ ImageMagick Images supports transparency. If the value is @code{nil}, it defaults to the frame's background color. -@item :width @var{width}, :height @var{height} -The @code{:width} and @code{:height} keywords are used for scaling the -image. If only one of them is specified, the other one will be -calculated so as to preserve the aspect ratio. If both are specified, -aspect ratio may not be preserved. - -@item :max-width @var{max-width}, :max-height @var{max-height} -The @code{:max-width} and @code{:max-height} keywords are used for -scaling if the size of the image of the image exceeds these values. -If @code{:width} is set it will have precedence over @code{max-width}, -and if @code{:height} is set it will have precedence over -@code{max-height}, but you can otherwise mix these keywords as you -wish. @code{:max-width} and @code{:max-height} will always preserve -the aspect ratio. - -If both @code{:width} and @code{:max-height} has been set (but -@code{:height} has not been set), then @code{:max-height} will have -precedence. The same is the case for the opposite combination: The -``max'' keyword has precedence. That is, if you have a 200x100 image -and specify that @code{:width} should be 400 and @code{:max-height} -should be 150, you'll end up with an image that is 300x150: Preserving -the aspect ratio and not exceeding the ``max'' setting. This -combination of parameters is a useful way of saying ``display this -image as large as possible, but no larger than the available display -area''. - -@item :scale @var{scale} -This should be a number, where values higher than 1 means to increase -the size, and lower means to decrease the size. For instance, a value -of 0.25 will make the image a quarter size of what it originally was. -If the scaling makes the image larger than specified by -@code{:max-width} or @code{:max-height}, the resulting size will not -exceed those two values. If both @code{:scale} and -@code{:height}/@code{:width} are specified, the height/width will be -adjusted by the specified scaling factor. - @item :format @var{type} The value, @var{type}, should be a symbol specifying the type of the image data, as found in @code{image-format-suffixes}. This is used @@ -5431,9 +5444,6 @@ ImageMagick Images @item :rotation @var{angle} Specifies a rotation angle in degrees. - -@item :index @var{frame} -@xref{Multi-Frame Images}. @end table @node SVG Images diff --git a/etc/NEWS b/etc/NEWS index 75e2c1bf98..69e14773a7 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1442,6 +1442,12 @@ that is non-nil, it will look for a file name handler for the current buffer's 'default-directory' and invoke that file name handler to make the process. That way 'make-process' can start remote processes. ++++ +** Emacs now supports resizing images without ImageMagick on X window +systems where the XRender extension is available, and on the NS port. +The new function 'image-scaling-p' can be used to test whether any +given frame supports resizing. + \f * Changes in Emacs 27.1 on Non-Free Operating Systems diff --git a/lisp/image.el b/lisp/image.el index 5727d8fbce..2e84e47b5c 100644 --- a/lisp/image.el +++ b/lisp/image.el @@ -982,8 +982,8 @@ image--get-image image)) (defun image--get-imagemagick-and-warn () - (unless (or (fboundp 'imagemagick-types) (featurep 'ns)) - (error "Cannot rescale images without ImageMagick support")) + (unless (or (fboundp 'imagemagick-types) (image-scaling-p)) + (error "Cannot rescale images on this terminal")) (let ((image (image--get-image))) (image-flush image) (when (fboundp 'imagemagick-types) diff --git a/src/Makefile.in b/src/Makefile.in index e9831e9299..f409ed4db2 100644 --- a/src/Makefile.in +++ b/src/Makefile.in @@ -127,7 +127,8 @@ LIBIMAGE= XCB_LIBS=@XCB_LIBS@ XFT_LIBS=@XFT_LIBS@ -LIBX_EXTRA=-lX11 $(XCB_LIBS) $(XFT_LIBS) +XRENDER_LIBS=@XRENDER_LIBS@ +LIBX_EXTRA=-lX11 $(XCB_LIBS) $(XFT_LIBS) $(XRENDER_LIBS) FONTCONFIG_CFLAGS = @FONTCONFIG_CFLAGS@ FONTCONFIG_LIBS = @FONTCONFIG_LIBS@ diff --git a/src/dispextern.h b/src/dispextern.h index 5774e3e951..b064875ac4 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -31,6 +31,9 @@ along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */ #include <X11/Intrinsic.h> #endif /* USE_X_TOOLKIT */ +#ifdef HAVE_XRENDER +#include <X11/extensions/Xrender.h> +#endif #else /* !HAVE_X_WINDOWS */ /* X-related stuff used by non-X gui code. */ @@ -2935,6 +2938,11 @@ struct redisplay_interface #ifdef HAVE_WINDOW_SYSTEM +#if defined (HAVE_X_WINDOWS) && defined (HAVE_XRENDER) \ + || defined (HAVE_NS) +#define HAVE_NATIVE_SCALING +#endif + /* Structure describing an image. Specific image formats like XBM are converted into this form, so that display only has to deal with this type of image. */ @@ -2958,6 +2966,11 @@ struct image and the latter is outdated. NULL means the X image has been synchronized to Pixmap. */ XImagePtr ximg, mask_img; + +#ifdef HAVE_NATIVE_SCALING + /* Picture versions of pixmap and mask for compositing. */ + Picture picture, mask_picture; +#endif #endif /* Colors allocated for this image, if any. Allocated via xmalloc. */ diff --git a/src/image.c b/src/image.c index 87e0c071ee..677689cae2 100644 --- a/src/image.c +++ b/src/image.c @@ -408,8 +408,13 @@ x_destroy_all_bitmaps (Display_Info *dpyinfo) dpyinfo->bitmaps_last = 0; } +#ifndef HAVE_XRENDER +/* Required for the definition of x_create_x_image_and_pixmap below. */ +typedef void Picture; +#endif + static bool x_create_x_image_and_pixmap (struct frame *, int, int, int, - XImagePtr *, Pixmap *); + XImagePtr *, Pixmap *, Picture *); static void x_destroy_x_image (XImagePtr ximg); #ifdef HAVE_NTGUI @@ -472,7 +477,8 @@ x_create_bitmap_mask (struct frame *f, ptrdiff_t id) return; } - result = x_create_x_image_and_pixmap (f, width, height, 1, &mask_img, &mask); + result = x_create_x_image_and_pixmap (f, width, height, 1, + &mask_img, &mask, NULL); unblock_input (); if (!result) @@ -1011,6 +1017,13 @@ free_image (struct frame *f, struct image *img) c->images[img->id] = NULL; +#ifdef HAVE_XRENDER + if (img->picture) + XRenderFreePicture (FRAME_X_DISPLAY (f), img->picture); + if (img->mask_picture) + XRenderFreePicture (FRAME_X_DISPLAY (f), img->mask_picture); +#endif + /* Windows NT redefines 'free', but in this file, we need to avoid the redefinition. */ #ifdef WINDOWSNT @@ -1747,6 +1760,147 @@ postprocess_image (struct frame *f, struct image *img) } } +#if defined (HAVE_IMAGEMAGICK) || defined (HAVE_NATIVE_SCALING) +/* Scale an image size by returning SIZE / DIVISOR * MULTIPLIER, + safely rounded and clipped to int range. */ + +static int +scale_image_size (int size, size_t divisor, size_t multiplier) +{ + if (divisor != 0) + { + double s = size; + double scaled = s * multiplier / divisor + 0.5; + if (scaled < INT_MAX) + return scaled; + } + return INT_MAX; +} + +/* Compute the desired size of an image with native size WIDTH x HEIGHT. + Use SPEC to deduce the size. Store the desired size into + *D_WIDTH x *D_HEIGHT. Store -1 x -1 if the native size is OK. */ +static void +compute_image_size (size_t width, size_t height, + Lisp_Object spec, + int *d_width, int *d_height) +{ + Lisp_Object value; + int desired_width = -1, desired_height = -1, max_width = -1, max_height = -1; + double scale = 1; + + value = image_spec_value (spec, QCscale, NULL); + if (NUMBERP (value)) + scale = XFLOATINT (value); + + value = image_spec_value (spec, QCmax_width, NULL); + if (FIXNATP (value)) + max_width = min (XFIXNAT (value), INT_MAX); + + value = image_spec_value (spec, QCmax_height, NULL); + if (FIXNATP (value)) + max_height = min (XFIXNAT (value), INT_MAX); + + /* If width and/or height is set in the display spec assume we want + to scale to those values. If either h or w is unspecified, the + unspecified should be calculated from the specified to preserve + aspect ratio. */ + value = image_spec_value (spec, QCwidth, NULL); + if (FIXNATP (value)) + { + desired_width = min (XFIXNAT (value) * scale, INT_MAX); + /* :width overrides :max-width. */ + max_width = -1; + } + + value = image_spec_value (spec, QCheight, NULL); + if (FIXNATP (value)) + { + desired_height = min (XFIXNAT (value) * scale, INT_MAX); + /* :height overrides :max-height. */ + max_height = -1; + } + + /* If we have both width/height set explicitly, we skip past all the + aspect ratio-preserving computations below. */ + if (desired_width != -1 && desired_height != -1) + goto out; + + width = width * scale; + height = height * scale; + + if (desired_width != -1) + /* Width known, calculate height. */ + desired_height = scale_image_size (desired_width, width, height); + else if (desired_height != -1) + /* Height known, calculate width. */ + desired_width = scale_image_size (desired_height, height, width); + else + { + desired_width = width; + desired_height = height; + } + + if (max_width != -1 && desired_width > max_width) + { + /* The image is wider than :max-width. */ + desired_width = max_width; + desired_height = scale_image_size (desired_width, width, height); + } + + if (max_height != -1 && desired_height > max_height) + { + /* The image is higher than :max-height. */ + desired_height = max_height; + desired_width = scale_image_size (desired_height, height, width); + } + + out: + *d_width = desired_width; + *d_height = desired_height; +} + +#ifdef HAVE_NATIVE_SCALING +static void +x_set_image_size (struct frame *f, struct image *img) +{ +#ifdef HAVE_IMAGEMAGICK + /* ImageMagick images are already the correct size. */ + if (!imagemagick_image_p (img->spec)) +#endif + { + int width, height; + + compute_image_size (img->width, img->height, img->spec, &width, &height); + +#ifdef HAVE_NS + ns_image_set_size (img->pixmap, width, height); + img->width = width; + img->height = height; +#endif + +#ifdef HAVE_XRENDER + if (img->picture) + { + double xscale = (double) img->width/width; + double yscale = (double) img->height/height; + + XTransform tmat = {{{XDoubleToFixed (xscale), XDoubleToFixed (0), XDoubleToFixed (0)}, + {XDoubleToFixed (0), XDoubleToFixed (yscale), XDoubleToFixed (0)}, + {XDoubleToFixed (0), XDoubleToFixed (0), XDoubleToFixed (1)}}}; + + XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest, 0, 0); + XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat); + + img->width = width; + img->height = height; + } +#endif + } +} +#endif +#endif /* HAVE_IMAGEMAGICK || HAVE_XRENDER || HAVE_NS */ + /* Return the id of image with Lisp specification SPEC on frame F. SPEC must be a valid Lisp image specification (see valid_image_p). */ @@ -1802,6 +1956,9 @@ lookup_image (struct frame *f, Lisp_Object spec) `:background COLOR'. */ Lisp_Object ascent, margin, relief, bg; int relief_bound; +#ifdef HAVE_NATIVE_SCALING + x_set_image_size (f, img); +#endif ascent = image_spec_value (spec, QCascent, NULL); if (FIXNUMP (ascent)) @@ -1976,12 +2133,15 @@ x_check_image_size (XImagePtr ximg, int width, int height) static bool x_create_x_image_and_pixmap (struct frame *f, int width, int height, int depth, - XImagePtr *ximg, Pixmap *pixmap) + XImagePtr *ximg, Pixmap *pixmap, Picture *picture) { #ifdef HAVE_X_WINDOWS Display *display = FRAME_X_DISPLAY (f); Drawable drawable = FRAME_X_DRAWABLE (f); Screen *screen = FRAME_X_SCREEN (f); +#ifdef HAVE_XRENDER + int event_basep, error_basep; +#endif eassert (input_blocked_p ()); @@ -2018,6 +2178,21 @@ x_create_x_image_and_pixmap (struct frame *f, int width, int height, int depth, return 0; } +#ifdef HAVE_XRENDER + if (picture && XRenderQueryExtension (display, &event_basep, &error_basep)) + { + XRenderPictFormat *format; + XRenderPictureAttributes attr; + + /* FIXME: Do we need to handle all possible bit depths? */ + format = XRenderFindStandardFormat (display, + depth > 24 ? PictStandardARGB32 + : depth > 8 ? PictStandardRGB24 + : PictStandardA8); + *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr); + } +#endif + return 1; #endif /* HAVE_X_WINDOWS */ @@ -2163,7 +2338,8 @@ x_put_x_image (struct frame *f, XImagePtr ximg, Pixmap pixmap, int width, int he eassert (input_blocked_p ()); gc = XCreateGC (FRAME_X_DISPLAY (f), pixmap, 0, NULL); - XPutImage (FRAME_X_DISPLAY (f), pixmap, gc, ximg, 0, 0, 0, 0, width, height); + XPutImage (FRAME_X_DISPLAY (f), pixmap, gc, ximg, 0, 0, 0, 0, + ximg->width, ximg->height); XFreeGC (FRAME_X_DISPLAY (f), gc); #endif /* HAVE_X_WINDOWS */ @@ -2192,7 +2368,13 @@ image_create_x_image_and_pixmap (struct frame *f, struct image *img, eassert ((!mask_p ? img->pixmap : img->mask) == NO_PIXMAP); return x_create_x_image_and_pixmap (f, width, height, depth, ximg, - !mask_p ? &img->pixmap : &img->mask); + !mask_p ? &img->pixmap : &img->mask, +#ifdef HAVE_XRENDER + !mask_p ? &img->picture : &img->mask_picture +#else + NULL +#endif + ); } /* Put X image XIMG into image IMG on frame F, as a mask if and only @@ -8101,105 +8283,6 @@ gif_load (struct frame *f, struct image *img) ImageMagick ***********************************************************************/ -/* Scale an image size by returning SIZE / DIVISOR * MULTIPLIER, - safely rounded and clipped to int range. */ - -static int -scale_image_size (int size, size_t divisor, size_t multiplier) -{ - if (divisor != 0) - { - double s = size; - double scaled = s * multiplier / divisor + 0.5; - if (scaled < INT_MAX) - return scaled; - } - return INT_MAX; -} - -/* Compute the desired size of an image with native size WIDTH x HEIGHT. - Use SPEC to deduce the size. Store the desired size into - *D_WIDTH x *D_HEIGHT. Store -1 x -1 if the native size is OK. */ -static void -compute_image_size (size_t width, size_t height, - Lisp_Object spec, - int *d_width, int *d_height) -{ - Lisp_Object value; - int desired_width = -1, desired_height = -1, max_width = -1, max_height = -1; - double scale = 1; - - value = image_spec_value (spec, QCscale, NULL); - if (NUMBERP (value)) - scale = XFLOATINT (value); - - value = image_spec_value (spec, QCmax_width, NULL); - if (FIXNATP (value)) - max_width = min (XFIXNAT (value), INT_MAX); - - value = image_spec_value (spec, QCmax_height, NULL); - if (FIXNATP (value)) - max_height = min (XFIXNAT (value), INT_MAX); - - /* If width and/or height is set in the display spec assume we want - to scale to those values. If either h or w is unspecified, the - unspecified should be calculated from the specified to preserve - aspect ratio. */ - value = image_spec_value (spec, QCwidth, NULL); - if (FIXNATP (value)) - { - desired_width = min (XFIXNAT (value) * scale, INT_MAX); - /* :width overrides :max-width. */ - max_width = -1; - } - - value = image_spec_value (spec, QCheight, NULL); - if (FIXNATP (value)) - { - desired_height = min (XFIXNAT (value) * scale, INT_MAX); - /* :height overrides :max-height. */ - max_height = -1; - } - - /* If we have both width/height set explicitly, we skip past all the - aspect ratio-preserving computations below. */ - if (desired_width != -1 && desired_height != -1) - goto out; - - width = width * scale; - height = height * scale; - - if (desired_width != -1) - /* Width known, calculate height. */ - desired_height = scale_image_size (desired_width, width, height); - else if (desired_height != -1) - /* Height known, calculate width. */ - desired_width = scale_image_size (desired_height, height, width); - else - { - desired_width = width; - desired_height = height; - } - - if (max_width != -1 && desired_width > max_width) - { - /* The image is wider than :max-width. */ - desired_width = max_width; - desired_height = scale_image_size (desired_width, width, height); - } - - if (max_height != -1 && desired_height > max_height) - { - /* The image is higher than :max-height. */ - desired_height = max_height; - desired_width = scale_image_size (desired_height, height, width); - } - - out: - *d_width = desired_width; - *d_height = desired_height; -} - static bool imagemagick_image_p (Lisp_Object); static bool imagemagick_load (struct frame *, struct image *); static void imagemagick_clear_image (struct frame *, struct image *); @@ -9816,6 +9899,25 @@ DEFUN ("lookup-image", Flookup_image, Slookup_image, 1, 1, 0, Initialization ***********************************************************************/ +DEFUN ("image-scaling-p", Fimage_scaling_p, Simage_scaling_p, 0, 1, 0, + doc: /* Test whether FRAME supports resizing images. +Return t if FRAME supports native scaling, nil otherwise. */) + (Lisp_Object frame) +{ +#ifdef HAVE_NS + return Qt; +#elif defined (HAVE_X_WINDOWS) && defined (HAVE_XRENDER) + int event_basep, error_basep; + + if (XRenderQueryExtension + (FRAME_X_DISPLAY (decode_window_system_frame (frame)), + &event_basep, &error_basep)) + return Qt; +#endif + + return Qnil; +} + DEFUN ("init-image-library", Finit_image_library, Sinit_image_library, 1, 1, 0, doc: /* Initialize image library implementing image type TYPE. Return non-nil if TYPE is a supported image type. @@ -10058,6 +10160,8 @@ non-numeric, there is no explicit limit on the size of images. */); defsubr (&Slookup_image); #endif + defsubr (&Simage_scaling_p); + DEFVAR_BOOL ("cross-disabled-images", cross_disabled_images, doc: /* Non-nil means always draw a cross over disabled images. Disabled images are those having a `:conversion disabled' property. diff --git a/src/nsimage.m b/src/nsimage.m index 7879c5891d..f16910de08 100644 --- a/src/nsimage.m +++ b/src/nsimage.m @@ -126,8 +126,6 @@ Updated by Christian Limpach (chris@nice.ch) eImg = temp; } - [eImg setSizeFromSpec:XCDR (img->spec)]; - size = [eImg size]; img->width = size.width; img->height = size.height; @@ -151,6 +149,12 @@ Updated by Christian Limpach (chris@nice.ch) return [(id)img size].height; } +void +ns_image_set_size (void *img, int width, int height) +{ + [(EmacsImage *)img setSize:NSMakeSize (width, height)]; +} + unsigned long ns_get_pixel (void *img, int x, int y) { @@ -524,66 +528,6 @@ - (BOOL)setFrame: (unsigned int) index return YES; } -- (void)setSizeFromSpec: (Lisp_Object) spec -{ - NSSize size = [self size]; - Lisp_Object value; - double scale = 1, aspect = size.width / size.height; - double width = -1, height = -1, max_width = -1, max_height = -1; - - value = Fplist_get (spec, QCscale); - if (NUMBERP (value)) - scale = XFLOATINT (value) ; - - value = Fplist_get (spec, QCmax_width); - if (NUMBERP (value)) - max_width = XFLOATINT (value); - - value = Fplist_get (spec, QCmax_height); - if (NUMBERP (value)) - max_height = XFLOATINT (value); - - value = Fplist_get (spec, QCwidth); - if (NUMBERP (value)) - { - width = XFLOATINT (value) * scale; - /* :width overrides :max-width. */ - max_width = -1; - } - - value = Fplist_get (spec, QCheight); - if (NUMBERP (value)) - { - height = XFLOATINT (value) * scale; - /* :height overrides :max-height. */ - max_height = -1; - } - - if (width <= 0 && height <= 0) - { - width = size.width * scale; - height = size.height * scale; - } - else if (width > 0 && height <= 0) - height = width / aspect; - else if (height > 0 && width <= 0) - width = height * aspect; - - if (max_width > 0 && width > max_width) - { - width = max_width; - height = max_width / aspect; - } - - if (max_height > 0 && height > max_height) - { - height = max_height; - width = max_height * aspect; - } - - [self setSize:NSMakeSize(width, height)]; -} - - (instancetype)rotate: (double)rotation { EmacsImage *new_image; diff --git a/src/nsterm.h b/src/nsterm.h index 089cbccbf0..78ce608554 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -648,7 +648,6 @@ typedef id instancetype; - (NSColor *)stippleMask; - (Lisp_Object)getMetadata; - (BOOL)setFrame: (unsigned int) index; -- (void)setSizeFromSpec: (Lisp_Object) spec; - (instancetype)rotate: (double)rotation; @end @@ -1197,6 +1196,7 @@ extern bool ns_load_image (struct frame *f, struct image *img, Lisp_Object spec_file, Lisp_Object spec_data); extern int ns_image_width (void *img); extern int ns_image_height (void *img); +extern void ns_image_set_size (void *img, int width, int height); extern unsigned long ns_get_pixel (void *img, int x, int y); extern void ns_put_pixel (void *img, int x, int y, unsigned long argb); extern void ns_set_alpha (void *img, int x, int y, unsigned char a); diff --git a/src/nsterm.m b/src/nsterm.m index 016c044760..cbb2d2a5ce 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -3121,7 +3121,6 @@ so some key presses (TAB) are swallowed by the system. */ [img setXBMColor: bm_color]; } -#ifdef NS_IMPL_COCOA // Note: For periodic images, the full image height is "h + hd". // By using the height h, a suitable part of the image is used. NSRect fromRect = NSMakeRect(0, 0, p->wd, p->h); @@ -3134,13 +3133,6 @@ so some key presses (TAB) are swallowed by the system. */ fraction: 1.0 respectFlipped: YES hints: nil]; -#else - { - NSPoint pt = imageRect.origin; - pt.y += p->h; - [img compositeToPoint: pt operation: NSCompositingOperationSourceOver]; - } -#endif } ns_reset_clipping (f); } diff --git a/src/xterm.c b/src/xterm.c index e9cebcebba..fbbf61d320 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -38,11 +38,6 @@ along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. */ #include <X11/extensions/Xfixes.h> #endif -/* Using Xft implies that XRender is available. */ -#ifdef HAVE_XFT -#include <X11/extensions/Xrender.h> -#endif - #ifdef HAVE_XDBE #include <X11/extensions/Xdbe.h> #endif @@ -2976,6 +2971,46 @@ x_draw_glyph_string_box (struct glyph_string *s) } +static void +x_composite_image (struct glyph_string *s, Pixmap dest, + int srcX, int srcY, int dstX, int dstY, + int width, int height) +{ +#ifdef HAVE_XRENDER + if (s->img->picture) + { + Picture destination; + XRenderPictFormat *default_format; + XRenderPictureAttributes attr; + + /* FIXME: Should we do this each time or would it make sense to + store destination in the frame struct? */ + default_format = XRenderFindVisualFormat (s->display, + DefaultVisual (s->display, 0)); + destination = XRenderCreatePicture (s->display, dest, + default_format, 0, &attr); + + /* FIXME: It may make sense to use PictOpSrc instead of + PictOpOver, as I don't know if we care about alpha values too + much here. */ + XRenderComposite (s->display, PictOpOver, + s->img->picture, s->img->mask_picture, destination, + srcX, srcY, + srcX, srcY, + dstX, dstY, + width, height); + + XRenderFreePicture (s->display, destination); + } + else +#endif + XCopyArea (s->display, s->img->pixmap, + dest, s->gc, + srcX, srcY, + width, height, dstX, dstY); +} + + /* Draw foreground of image glyph string S. */ static void @@ -3007,6 +3042,7 @@ x_draw_image_foreground (struct glyph_string *s) trust on the shape extension to be available (XShapeCombineRegion). So, compute the rectangle to draw manually. */ + /* FIXME: Do we need to do this when using XRender compositing? */ unsigned long mask = (GCClipMask | GCClipXOrigin | GCClipYOrigin | GCFunction); XGCValues xgcv; @@ -3024,10 +3060,8 @@ x_draw_image_foreground (struct glyph_string *s) image_rect.width = s->slice.width; image_rect.height = s->slice.height; if (x_intersect_rectangles (&clip_rect, &image_rect, &r)) - XCopyArea (s->display, s->img->pixmap, - FRAME_X_DRAWABLE (s->f), s->gc, - s->slice.x + r.x - x, s->slice.y + r.y - y, - r.width, r.height, r.x, r.y); + x_composite_image (s, FRAME_X_DRAWABLE (s->f), s->slice.x + r.x - x, s->slice.y + r.y - y, + r.x, r.y, r.width, r.height); } else { @@ -3039,10 +3073,8 @@ x_draw_image_foreground (struct glyph_string *s) image_rect.width = s->slice.width; image_rect.height = s->slice.height; if (x_intersect_rectangles (&clip_rect, &image_rect, &r)) - XCopyArea (s->display, s->img->pixmap, - FRAME_X_DRAWABLE (s->f), s->gc, - s->slice.x + r.x - x, s->slice.y + r.y - y, - r.width, r.height, r.x, r.y); + x_composite_image (s, FRAME_X_DRAWABLE (s->f), s->slice.x + r.x - x, s->slice.y + r.y - y, + r.x, r.y, r.width, r.height); /* When the image has a mask, we can expect that at least part of a mouse highlight or a block cursor will -- 2.19.1 -- Alan Third ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] Add native image scaling (bug#33587) 2019-01-02 21:12 ` [PATCH v2] Add native image scaling (bug#33587) Alan Third @ 2019-01-04 14:31 ` Eli Zaretskii 2019-01-04 19:09 ` Alan Third 0 siblings, 1 reply; 14+ messages in thread From: Eli Zaretskii @ 2019-01-04 14:31 UTC (permalink / raw) To: Alan Third; +Cc: emacs-devel > Date: Wed, 2 Jan 2019 21:12:41 +0000 > From: Alan Third <alan@idiocy.org> > Cc: emacs-devel@gnu.org > > I think this is the final version. Thanks. A few minor gotchas. > I would appreciate if someone who knows their way around image > handling would be able to test it. I’m particularly concerned that > I’ve probably broken masks. I can’t find any examples of how to use > them online, and they don’t work at all in NS, so I don’t know if they > get any use. Seconded. > +@item :max-width @var{max-width}, :max-height @var{max-height} > +The @code{:max-width} and @code{:max-height} keywords are used for > +scaling if the size of the image of the image exceeds these values. ^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant repetition. > +If @code{:width} is set it will have precedence over @code{max-width}, ^ A comma is missing there. > +and if @code{:height} is set it will have precedence over ^ Likewise here. > + @code{:max-width} and @code{:max-height} will always preserve > +the aspect ratio. I don't think I understand what that means in practice. Does this allude to the issue described below with using :max-width and :max-height in preference to :with and :height? If so, I suggest to describe that only once. > +If both @code{:width} and @code{:max-height} has been set (but ^^^ "have" > +@code{:height} has not been set), then @code{:max-height} will have > +precedence. The same is the case for the opposite combination: The > +``max'' keyword has precedence. This confused me until I've read the example. having read the example, I suggest to describe this differently: If both @code{:max-width} and @code{:height} are specified, but @code{:width} is not, preserving the aspect ratio might require that width exceeds @code{:max-width}. If this happens, scaling will use a smaller value for the height so as to preserve the aspect ratio while not exceeding @code{:max-width}. Similarly when both @code{:max-height} and @code{:width} are specified, but @code{:height} is not. > +@item :scale @var{scale} > +This should be a number, where values higher than 1 means to increase > +the size, and lower means to decrease the size. For instance, a value > +of 0.25 will make the image a quarter size of what it originally was. I think we should say here explicitly that scale multiplies both height and width, because people might otherwise think the scaling applies to the area of the image. > +@defun image-scaling-p &optional frame > +This function returns @code{t} if @var{frame} supports image scaling. > +@var{frame} @code{nil} or omitted means to use the selected frame > +(@pxref{Input Focus}). > + > +If image scaling is not supported, @code{:width}, @code{:height}, > +@code{:scale}, @code{:max-width} and @code{:max-height} will only be > +usable through ImageMagick, if available (@pxref{ImageMagick Images}). > +@end defun Shouldn't image-scaling-p return non-nil if ImageMagick is available? I think that would allow a simpler Lisp code. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] Add native image scaling (bug#33587) 2019-01-04 14:31 ` Eli Zaretskii @ 2019-01-04 19:09 ` Alan Third 2019-01-04 20:21 ` Eli Zaretskii 0 siblings, 1 reply; 14+ messages in thread From: Alan Third @ 2019-01-04 19:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel On Fri, Jan 04, 2019 at 04:31:41PM +0200, Eli Zaretskii wrote: > > Date: Wed, 2 Jan 2019 21:12:41 +0000 > > From: Alan Third <alan@idiocy.org> > > Cc: emacs-devel@gnu.org > > > > I think this is the final version. > > Thanks. A few minor gotchas. I regret touching this now. ;) Do these look OK? modified doc/lispref/display.texi @@ -5120,33 +5120,35 @@ Image Descriptors @item :max-width @var{max-width}, :max-height @var{max-height} The @code{:max-width} and @code{:max-height} keywords are used for -scaling if the size of the image of the image exceeds these values. -If @code{:width} is set it will have precedence over @code{max-width}, -and if @code{:height} is set it will have precedence over +scaling if the size of the image exceeds these values. If +@code{:width} is set, it will have precedence over @code{max-width}, +and if @code{:height} is set, it will have precedence over @code{max-height}, but you can otherwise mix these keywords as you -wish. @code{:max-width} and @code{:max-height} will always preserve -the aspect ratio. - -If both @code{:width} and @code{:max-height} has been set (but -@code{:height} has not been set), then @code{:max-height} will have -precedence. The same is the case for the opposite combination: The -``max'' keyword has precedence. That is, if you have a 200x100 image -and specify that @code{:width} should be 400 and @code{:max-height} -should be 150, you'll end up with an image that is 300x150: Preserving -the aspect ratio and not exceeding the ``max'' setting. This -combination of parameters is a useful way of saying ``display this -image as large as possible, but no larger than the available display -area''. +wish. + +If both @code{:max-width} and @code{:height} are specified, but +@code{:width} is not, preserving the aspect ratio might require that +width exceeds @code{:max-width}. If this happens, scaling will use a +smaller value for the height so as to preserve the aspect ratio while +not exceeding @code{:max-width}. Similarly when both +@code{:max-height} and @code{:width} are specified, but @code{:height} +is not. For example, if you have a 200x100 image and specify that +@code{:width} should be 400 and @code{:max-height} should be 150, +you'll end up with an image that is 300x150: Preserving the aspect +ratio and not exceeding the ``max'' setting. This combination of +parameters is a useful way of saying ``display this image as large as +possible, but no larger than the available display area''. @item :scale @var{scale} This should be a number, where values higher than 1 means to increase -the size, and lower means to decrease the size. For instance, a value -of 0.25 will make the image a quarter size of what it originally was. -If the scaling makes the image larger than specified by -@code{:max-width} or @code{:max-height}, the resulting size will not -exceed those two values. If both @code{:scale} and -@code{:height}/@code{:width} are specified, the height/width will be -adjusted by the specified scaling factor. +the size, and lower means to decrease the size, by multiplying both +the width and height. For instance, a value of 0.25 will make the +image a quarter size of what it originally was. If the scaling makes +the image larger than specified by @code{:max-width} or +@code{:max-height}, the resulting size will not exceed those two +values. If both @code{:scale} and @code{:height}/@code{:width} are +specified, the height/width will be adjusted by the specified scaling +factor. @item :index @var{frame} @xref{Multi-Frame Images}. > > +@code{:height} has not been set), then @code{:max-height} will have > > +precedence. The same is the case for the opposite combination: The > > +``max'' keyword has precedence. > > This confused me until I've read the example. having read the > example, I suggest to describe this differently: > > If both @code{:max-width} and @code{:height} are specified, but > @code{:width} is not, preserving the aspect ratio might require that > width exceeds @code{:max-width}. If this happens, scaling will use a > smaller value for the height so as to preserve the aspect ratio while > not exceeding @code{:max-width}. Similarly when both > @code{:max-height} and @code{:width} are specified, but @code{:height} > is not. I’ve put that in. FWIW I don’t understand why this behaviour was chosen, it seems to me that max-width and max-height should be exactly that, the max width and height. I don’t understand why they can be over‐ridden. Too late to do anything about it now, I suppose. > > +@defun image-scaling-p &optional frame > > +This function returns @code{t} if @var{frame} supports image scaling. > > +@var{frame} @code{nil} or omitted means to use the selected frame > > +(@pxref{Input Focus}). > > + > > +If image scaling is not supported, @code{:width}, @code{:height}, > > +@code{:scale}, @code{:max-width} and @code{:max-height} will only be > > +usable through ImageMagick, if available (@pxref{ImageMagick Images}). > > +@end defun > > Shouldn't image-scaling-p return non-nil if ImageMagick is available? > I think that would allow a simpler Lisp code. The difficulty is in knowing whether you need to set :type to ImageMagick or not. What we really don’t want to do is just return t for both native and ImageMagick, as you then have no way of deciding to go with native over ImageMagick, because you can’t tell the difference between native‐and‐ImageMagick, or just ImageMagick. I did think that I could return t if native is available, and if it’s not, but ImageMagick is, return 'imagemagick. Or perhaps return a list of scaling capable backends '(native imagemagick) There are other ways of detecting ImageMagick, but no other way of detecting native scaling, other than just trying it I suppose. (Another option would be to automatically fall‐back to ImageMagick if required, but given that we’re doing this because people are concerned about ImageMagick’s security record, I doubt making it’s use automatic would appeal.) -- Alan Third ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] Add native image scaling (bug#33587) 2019-01-04 19:09 ` Alan Third @ 2019-01-04 20:21 ` Eli Zaretskii 2019-01-04 22:45 ` Alan Third 0 siblings, 1 reply; 14+ messages in thread From: Eli Zaretskii @ 2019-01-04 20:21 UTC (permalink / raw) To: Alan Third; +Cc: emacs-devel > Date: Fri, 4 Jan 2019 19:09:14 +0000 > From: Alan Third <alan@idiocy.org> > Cc: emacs-devel@gnu.org > > (Another option would be to automatically fall‐back to ImageMagick if > required, but given that we’re doing this because people are concerned > about ImageMagick’s security record, I doubt making it’s use automatic > would appeal.) I guess I've misread the code, because I thought we did fall back automatically. FWIW, I think if the user builds Emacs with ImageMagick, they already considered the security issues and decided they are okay with them. We shouldn't second-guess them. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] Add native image scaling (bug#33587) 2019-01-04 20:21 ` Eli Zaretskii @ 2019-01-04 22:45 ` Alan Third 2019-01-10 19:42 ` Alan Third 0 siblings, 1 reply; 14+ messages in thread From: Alan Third @ 2019-01-04 22:45 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel On Fri, Jan 04, 2019 at 10:21:17PM +0200, Eli Zaretskii wrote: > > Date: Fri, 4 Jan 2019 19:09:14 +0000 > > From: Alan Third <alan@idiocy.org> > > Cc: emacs-devel@gnu.org > > > > (Another option would be to automatically fall‐back to ImageMagick if > > required, but given that we’re doing this because people are concerned > > about ImageMagick’s security record, I doubt making it’s use automatic > > would appeal.) > > I guess I've misread the code, because I thought we did fall back > automatically. FWIW, I think if the user builds Emacs with > ImageMagick, they already considered the security issues and decided > they are okay with them. We shouldn't second-guess them. If you load an image then use image-mode’s + and - bindings it automatically falls back to ImageMagick, but if you just load and insert an image like this: (insert-image (create-image "image.gif" nil nil :width 25)) it doesn’t. To get :width to do anything you need to specify that you want to use ImageMagick: (insert-image (create-image "image.gif" imagemagick nil :width 25)) I suspect the simplest place to do it would be in create-image, but someone who understands image.el better than me might have a different opinion. (In other words: I hope someone else fancies implementing the fallback, as I’m not very confident digging into lisp.) -- Alan Third ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] Add native image scaling (bug#33587) 2019-01-04 22:45 ` Alan Third @ 2019-01-10 19:42 ` Alan Third 2019-01-10 23:40 ` Paul Eggert 0 siblings, 1 reply; 14+ messages in thread From: Alan Third @ 2019-01-10 19:42 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel I’ve pushed this to master as‐is. If nobody else picks it up I’ll see if I can work out an ImageMagick fall‐back system. -- Alan Third ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] Add native image scaling (bug#33587) 2019-01-10 19:42 ` Alan Third @ 2019-01-10 23:40 ` Paul Eggert 2019-05-14 6:15 ` bug#33587: [PROPOSED] Default to disabling ImageMagick Paul Eggert 0 siblings, 1 reply; 14+ messages in thread From: Paul Eggert @ 2019-01-10 23:40 UTC (permalink / raw) To: Alan Third, Eli Zaretskii; +Cc: 33587, emacs-devel On 1/10/19 11:42 AM, Alan Third wrote: > I’ve pushed this to master as‐is. If nobody else picks it up I’ll see > if I can work out an ImageMagick fall‐back system. Thanks for doing all that. I tweaked the indentation a bit to fit in 80 columns etc. At some point soon I plan to install the patch in Bug#33587#5, which would disable ImageMagick by default when configuring Emacs (you can still enable it if you like). On my Fedora 29 desktop this patch decouples Emacs from libMagickCore, libMagickWand, libfftw3 (the FFT library), libgomp (OpenMP), libtldl (libtool), and libXt (X toolkit), which shaves 5-10 ms off the Emacs startup time in my little experiments. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#33587: [PROPOSED] Default to disabling ImageMagick 2019-01-10 23:40 ` Paul Eggert @ 2019-05-14 6:15 ` Paul Eggert 0 siblings, 0 replies; 14+ messages in thread From: Paul Eggert @ 2019-05-14 6:15 UTC (permalink / raw) To: 33587-done Paul Eggert wrote: > At some point soon I plan to install the patch in Bug#33587#5 It wasn't soon, but I did install the patch just now. Closing the bug report. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-05-14 6:15 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-02 18:09 bug#33587: [PROPOSED] Default to disabling ImageMagick Paul Eggert 2018-12-02 18:15 ` Eli Zaretskii 2018-12-02 19:13 ` Andreas Schwab 2018-12-02 23:51 ` Paul Eggert 2018-12-03 21:09 ` Alan Third 2018-12-03 19:08 ` Glenn Morris 2018-12-03 19:35 ` Paul Eggert 2018-12-03 19:40 ` Glenn Morris 2018-12-04 16:51 ` David Engster 2018-12-04 17:00 ` Glenn Morris 2018-12-04 17:38 ` David Engster 2018-12-04 18:16 ` Glenn Morris 2018-12-10 17:49 ` Paul Eggert -- strict thread matches above, loose matches on Subject: below -- 2019-01-02 16:11 [PATCH] Add native image scaling Eli Zaretskii 2019-01-02 21:12 ` [PATCH v2] Add native image scaling (bug#33587) Alan Third 2019-01-04 14:31 ` Eli Zaretskii 2019-01-04 19:09 ` Alan Third 2019-01-04 20:21 ` Eli Zaretskii 2019-01-04 22:45 ` Alan Third 2019-01-10 19:42 ` Alan Third 2019-01-10 23:40 ` Paul Eggert 2019-05-14 6:15 ` bug#33587: [PROPOSED] Default to disabling ImageMagick Paul Eggert
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.