bug#38109: 27.0.50; xpm image scaling doesn't work

bug#38109: 27.0.50; xpm image scaling doesn't work

[Unknown]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]


I *think* scaling of xpm images used to work (maybe back when
ImageMagick did the scaling?) - but it doesn't now, e.g. if you go:

  (insert-image (create-image "/usr/src/emacs/etc/images/smilies/medium/wry.xpm" nil nil :scale 20.0))

a tiny 16x16 image is inserted. The expected result was a huge 320x320
image.

If I convert the image to png:

  convert /usr/src/emacs/etc/images/smilies/medium/wry.xpm /tmp/wry.gif

and insert it with:

  (insert-image (create-image "/tmp/wry.gif" nil nil :scale 20.0))

I do get a bug 320x320 pixel image in my buffer.

(Interestingly, if I convert it to PNG and insert it, I get a 320x320
pixel image, with the original small 16x16 graphics inside it, so
scaling seems to not work for PNG either, in a different way.)

This is what it looks like in emacs -Q:


[-- Attachment #2: emacs27-image-scaling.png --]
[-- Type: image/png, Size: 126422 bytes --]

[-- Attachment #3: Type: text/plain, Size: 8743 bytes --]



In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.12)
 of 2019-10-31 built on tullinupnew
Repository revision: 058e293fddeaa7821f13055b451951360a135b0c
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux bullseye/sid

Recent messages:
Reading active file via nnml...done
Reading active file from archive via nnml...
Opening nnml server on archive...done
Reading active file from archive via nnml...done
Reading active file via nnir...done
Reading active file via nndraft...done
Opening nnml server...done
Registering 1 specific articles as ham using backend spam-use-move
Opening nntp server on gm...done
Mark set

Configured using:
 'configure --without-pop'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GSETTINGS GLIB NOTIFY INOTIFY
ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LIBSYSTEMD PDUMPER LCMS2
GMP

Important settings:
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-topic-mode: t
  gnus-undo-mode: t
  pixel-scroll-mode: t
  engine-mode: t
  global-magit-file-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  dumb-jump-mode: t
  which-function-mode: t
  global-auto-complete-mode: t
  shell-dirtrack-mode: t
  save-place-mode: t
  jabber-activity-mode: t
  winner-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t

Load-path shadows:
/usr/share/emacs/site-lisp/elpa-src/ess-18.10.2/debian-autoloads hides /usr/share/emacs/site-lisp/elpa-src/dpkg-dev-el-37.0/debian-autoloads
/usr/share/emacs/site-lisp/elpa-src/boxquote-2.1/boxquote hides ~/elisp/extra/boxquote
~/elisp/let-alist/let-alist hides ~/elisp/extra/let-alist
~/elisp/with-editor/with-editor hides ~/elisp/extra/with-editor
~/elisp/with-editor/with-editor-autoloads hides ~/elisp/extra/with-editor-autoloads
~/elisp/let-alist/let-alist hides /usr/src/emacs/lisp/emacs-lisp/let-alist

Features:
(shadow emacsbug bbdb-gnus-aux misearch multi-isearch shr-color color
canlock ispell bbdb-message sendmail nnir compface gnus-gravatar
gravatar sort smiley gnus-cite mm-archive gnus-async gnus-bcklg gnus-dup
qp gnus-ml gmane gnus-topic paren utf-7 imap rfc2104 epa-file
network-stream nnml bbdb-gnus bbdb-mua nnnil gnus-demon gnus-delay
gnus-draft gnus-agent gnus-srvr gnus-score score-mode nnvirtual nntp
gnus-cache nndraft nnmh mail-extr spam spam-stat bbdb-com gnus-uu yenc
gnus-msg gnus-html url-queue help-fns radix-tree url-cache bbdb-picture
gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum gnus-group
gnus-undo gnus-fun hashcash gnus-start gnus-cloud nnimap nnmail
mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range gnus-win
gopher shr with-url mm-url gnus nnheader svg pixel-scroll litable
engine-mode gitpatch magithub magithub-ci magithub-issue magithub-cache
magithub-core magit-submodule magit-obsolete magit-blame magit-stash
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-collab ghub-graphql treepy
graphql ghub url-http url-gw nsm url-auth let-alist magit-files
magit-refs magit-status magit magit-repos magit-apply magit-wip
magit-log magit-diff smerge-mode diff magit-core magit-autorevert
autorevert filenotify magit-process magit-margin magit-mode git-commit
recentf tree-widget magit-git magit-section magit-utils magit-popup
vc-git diff-mode crm log-edit message rmc rfc822 mml mml-sec epa epg
epg-config gnus-util rmail rmail-loaddefs text-property-search mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader
pcvs-util with-editor term disp-table ehelp eshell esh-cmd esh-ext
esh-opt esh-proc esh-io esh-arg esh-module esh-groups esh-util wgrep-ag
wgrep grep ag vc-svn find-dired dumb-jump f dash s ucs-normalize etags
fileloop generator tex-site auto-loads expand-region
cperl-mode-expansions text-mode-expansions html-mode-expansions
er-basic-expansions expand-region-core expand-region-custom which-func
cperl-mode auto-complete-config auto-complete popup cl-extra help-mode
ess-site ess-toolbar ess-mouse mouseme ess-swv ess-noweb
ess-noweb-font-lock-mode ess-jags-d ess-bugs-l essd-els ess-xls-d
ess-vst-d ess-stata-mode ess-stata-lang cc-vars cc-defs make-regexp
ess-sp6w-d ess-sp5-d ess-sp4-d ess-sas-d ess-sas-l ess-sas-a ess-s4-d
ess-s3-d ess-omg-d ess-omg-l ess-arc-d ess-lsp-l ess-sp6-d ess-dde
ess-sp3-d ess-julia julia-mode ess-r-mode ess-r-flymake rx flymake-proc
flymake warnings thingatpt ess-r-xref xref project ess-trns
ess-r-package ess-r-syntax pcase ess-r-completion ess-roxy ess-rd essddr
noutline outline hideshow ess-s-lang speedbar sb-image ezimage dframe
ess-help info reporter ess-mode ess ess-noweb-mode ess-inf ess-tracebug
easy-mmode ess-generics compile ess-utils ido ess-custom executable
tramp tramp-loaddefs trampver tramp-integration files-x tramp-compat
shell pcomplete parse-time iso8601 ls-lisp debian-changelog-mode imenu
add-log dpkg-dev-el saveplace vc vc-dispatcher bbdb derived bbdb-site
timezone bbdb-loaddefs boxquote rect jabber-http-file-upload url
url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util jabber-print-html jabber-otr jabber
jabber-notifications notifications jabber-libnotify dbus jabber-awesome
jabber-osd jabber-wmii jabber-xmessage jabber-festival jabber-sawfish
jabber-ratpoison jabber-tmux jabber-screen jabber-socks5
jabber-ft-server jabber-si-server jabber-ft-client jabber-ft-common
jabber-si-client jabber-si-common jabber-feature-neg jabber-truncate
jabber-time jabber-autoaway time-date jabber-vcard-avatars
jabber-chatstates jabber-events jabber-vcard jabber-avatar mailcap
jabber-activity jabber-watch jabber-modeline advice jabber-ahc-presence
jabber-ahc jabber-version jabber-ourversion jabber-muc-nick-completion
hippie-exp comint ansi-color jabber-browse jabber-search jabber-register
jabber-roster format-spec jabber-presence jabber-muc jabber-bookmarks
jabber-private jabber-muc-nick-coloring hexrgb jabber-widget
jabber-disco wid-edit jabber-chat jabber-history jabber-chatbuffer
jabber-alert jabber-iq jabber-core jabber-console sgml-mode dom ewoc
jabber-keymap jabber-sasl sasl sasl-anonymous sasl-login sasl-plain fsm
jabber-logon jabber-conn srv dns starttls tls jabber-xml xml jabber-menu
jabber-util cl winner ring gnutls puny find-file-from-selection
find-lisp dired dired-loaddefs cap-words superword subword edmacro
kmacro server finder-inf package easymenu browse-url url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 903266 194934)
 (symbols 48 41175 5)
 (strings 32 264148 17332)
 (string-bytes 1 11187328)
 (vectors 16 82347)
 (vector-slots 8 2589302 201528)
 (floats 8 706 677)
 (intervals 56 1323 779)
 (buffers 1000 93))

-- 
 "Please don't "meh" the panopticon. You are                   Adam Sjøgren
  not making things better by doing that."                asjo@koldfront.dk

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

I just updated my Emacs build to master HEAD now (previously was built
in mid-October), and the results are consitent no scaling for XPM, GIF
and PNG now, as can be seen in this image:


[-- Attachment #2: emacsmasterhead-scaling.png --]
[-- Type: image/png, Size: 71092 bytes --]

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Lars Ingebrigtsen]

Unknown <unknown@unknown.invalid> writes:

> I just updated my Emacs build to master HEAD now (previously was built
> in mid-October), and the results are consitent no scaling for XPM, GIF
> and PNG now, as can be seen in this image:

Hm.  Odd.  When I try

(insert-image (create-image "/tmp/foo.png" nil nil :scale 10))

etc, I get the proper scaling with png and gif, but not with xpm.
(Running from HEAD with -Q.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

Lars writes:

>> I just updated my Emacs build to master HEAD now (previously was built
>> in mid-October), and the results are consitent no scaling for XPM, GIF
>> and PNG now, as can be seen in this image:
>
> Hm.  Odd.

Argh, I tested the wrong Emacs binary. How embarrasing. Ignore my second
email.

> I get the proper scaling with png and gif, but not with xpm. (Running
> from HEAD with -Q.)

I don't, the XPM is unscaled and the PNG is inserted on a large canvas
with the original image unscaled in the corner:

[-- Attachment #2: emacsmasterheadforreal-scaling.png --]
[-- Type: image/png, Size: 139582 bytes --]

bug#38109: 27.0.50; xpm image scaling doesn't work

[Stephen Berman]

On Thu, 07 Nov 2019 22:11:14 +0100 Unknown <unknown@unknown.invalid> wrote:

> I *think* scaling of xpm images used to work (maybe back when
> ImageMagick did the scaling?) - but it doesn't now, e.g. if you go:
>
>   (insert-image (create-image
> "/usr/src/emacs/etc/images/smilies/medium/wry.xpm" nil nil :scale 20.0))
>
> a tiny 16x16 image is inserted. The expected result was a huge 320x320
> image.
[...]
> Configured features:
> XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GSETTINGS GLIB NOTIFY INOTIFY
> ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF XFT ZLIB
> TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LIBSYSTEMD PDUMPER LCMS2
> GMP

On Thu, 07 Nov 2019 22:30:57 +0100 Unknown <unknown@unknown.invalid> wrote:

> I just updated my Emacs build to master HEAD now (previously was built
> in mid-October), and the results are consitent no scaling for XPM, GIF
> and PNG now [...]

Cf. NEWS:

 ** Emacs now supports resizing and rotating images without ImageMagick.
 All modern systems support this feature.  (On GNU and Unix systems,
 Cairo drawing or the XRender extension to X11 is required for this to
 be available; the configure script will test for it and, if found,
 enable scaling.)

I'm not sure about XRender; I think my X has it, but I only get scaling
(also of XPM and PNG images) with the Cairo build.

Steve Berman

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Lars Ingebrigtsen]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

Adam Sjøgren <asjo@koldfront.dk> writes:

> I don't, the XPM is unscaled and the PNG is inserted on a large canvas
> with the original image unscaled in the corner:

I made two PNGs from wry.xpm. 

file /tmp/wry*.png 
/tmp/wry2.png: PNG image data, 13 x 14, 8-bit/color RGBA, non-interlaced
/tmp/wry.png:  PNG image data, 13 x 14, 2-bit colormap, non-interlaced

With the colormap png, I see the same as you -- no scaling, but a large
canvas.  With the non-colormap one, I get the proper scaling, too.

Odder and odder.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

[-- Attachment #2: wry.png --]
[-- Type: image/png, Size: 341 bytes --]

[-- Attachment #3: wry2.png --]
[-- Type: image/png, Size: 7051 bytes --]

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Lars writes:

> I made two PNGs from wry.xpm. 
>
> file /tmp/wry*.png 
> /tmp/wry2.png: PNG image data, 13 x 14, 8-bit/color RGBA, non-interlaced
> /tmp/wry.png:  PNG image data, 13 x 14, 2-bit colormap, non-interlaced
>
> With the colormap png, I see the same as you -- no scaling, but a large
> canvas.  With the non-colormap one, I get the proper scaling, too.

Interesting; the files I tested with:

  $ file wry.*
  wry.gif: GIF image data, version 89a, 16 x 16
  wry.png: PNG image data, 16 x 16, 4-bit colormap, non-interlaced

So that's consistent with what you see. Perhaps testing was done with
RGB(A) images, and not with colormaps/palettes.

-- 
 "Please don't "meh" the panopticon. You are                   Adam Sjøgren
  not making things better by doing that."                asjo@koldfront.dk

bug#38109: 27.0.50; xpm image scaling doesn't work

[Eli Zaretskii]

> Date: Thu, 07 Nov 2019 22:11:14 +0100
> From: Adam Sjøgren via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> I *think* scaling of xpm images used to work (maybe back when
> ImageMagick did the scaling?) - but it doesn't now, e.g. if you go:
> 
>   (insert-image (create-image "/usr/src/emacs/etc/images/smilies/medium/wry.xpm" nil nil :scale 20.0))
> 
> a tiny 16x16 image is inserted. The expected result was a huge 320x320
> image.

You need to build either with XRender support (should be automatic if
you have it installed) or with ImageMagick.  Does config.h define
HAVE_XRENDER to 1?

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 07 Nov 2019 22:49:13 +0100
> Cc: 38109@debbugs.gnu.org
> 
> Hm.  Odd.  When I try
> 
> (insert-image (create-image "/tmp/foo.png" nil nil :scale 10))
> 
> etc, I get the proper scaling with png and gif, but not with xpm.
> (Running from HEAD with -Q.)

Maybe there's some X-specific problem.  I see no problem on
MS-Windows.

bug#38109: 27.0.50; xpm image scaling doesn't work

[Unknown]

Eli writes:

> You need to build either with XRender support (should be automatic if
> you have it installed) or with ImageMagick.  Does config.h define
> HAVE_XRENDER to 1?

It does:

  asjo@tullinup:/usr/src/emacs$ grep HAVE_XRENDER src/config.h
  #define HAVE_XRENDER 1

and ImageMagick isn't defined:

  asjo@tullinup:/usr/src/emacs$ grep HAVE_IMAGEMAGICK src/config.h
  /* #undef HAVE_IMAGEMAGICK */
  /* #undef HAVE_IMAGEMAGICK7 */

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

On Thu, Nov 07, 2019 at 11:12:31PM +0100, Adam Sjøgren via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> Lars writes:
> 
> > I made two PNGs from wry.xpm. 
> >
> > file /tmp/wry*.png 
> > /tmp/wry2.png: PNG image data, 13 x 14, 8-bit/color RGBA, non-interlaced
> > /tmp/wry.png:  PNG image data, 13 x 14, 2-bit colormap, non-interlaced
> >
> > With the colormap png, I see the same as you -- no scaling, but a large
> > canvas.  With the non-colormap one, I get the proper scaling, too.
> 
> Interesting; the files I tested with:
> 
>   $ file wry.*
>   wry.gif: GIF image data, version 89a, 16 x 16
>   wry.png: PNG image data, 16 x 16, 4-bit colormap, non-interlaced
> 
> So that's consistent with what you see. Perhaps testing was done with
> RGB(A) images, and not with colormaps/palettes.

OK, that makes sense. Check out the comment and code at line 2593 of
image.c:

    /* FIXME: Do we need to handle all possible bit depths?
       XRenderFindStandardFormat supports PictStandardARGB32,
       PictStandardRGB24, PictStandardA8, PictStandardA4,
       PictStandardA1, and PictStandardNUM (what is this?!).
    
       XRenderFindFormat may support more, but I don't
       understand the documentation.  */
    format = XRenderFindStandardFormat (display,
                                        depth == 32 ? PictStandardARGB32
                                        : depth == 24 ? PictStandardRGB24
                                        : PictStandardA8);
    *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);

There should be an error message somewhere telling you that Emacs
doesn’t support scaling with that bit depth.

I guess it should be simple enough to add 4 and 1 bit to the above
which I hope would cover some of the above cases...
-- 
Alan Third

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

On Fri, Nov 08, 2019 at 07:34:07PM +0000, Alan Third wrote:
> On Thu, Nov 07, 2019 at 11:12:31PM +0100, Adam Sjøgren via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> 
> I guess it should be simple enough to add 4 and 1 bit to the above
> which I hope would cover some of the above cases...

Although I don’t understand why the png displays unscaled in a big
canvas. We must have missed a check against the Picture member to
prevent it resizing if there is no Picture.

This is at the top of image_set_transform:

    # if !defined USE_CAIRO && defined HAVE_XRENDER
      if (!img->picture)
        return;
    # endif

which should prevent any attempt to scale right at the start... I’m
really not sure what’s going on there.
-- 
Alan Third

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Alan writes:

[...]

> There should be an error message somewhere telling you that Emacs
> doesn’t support scaling with that bit depth.

Indeed there is for the:

  /tmp/wry.png: PNG image data, 16 x 16, 4-bit colormap, non-interlaced

image. But not for the .xpm. Ah, but xpm doesn't go through
image_create_x_image_and_pixmap_1(), so that explains that.

> I guess it should be simple enough to add 4 and 1 bit to the above
> which I hope would cover some of the above cases...

Yes, with this patch:

diff --git a/src/image.c b/src/image.c
index 870f008b14..bb76e9db60 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2585,7 +2585,7 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
     {
       if (depth <= 0)
 	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
-      if (depth == 32 || depth == 24 || depth == 8)
+      if (depth == 32 || depth == 24 || depth == 8 || depth == 4 || depth == 1)
         {
           XRenderPictFormat *format;
           XRenderPictureAttributes attr;
@@ -2600,12 +2600,16 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
           format = XRenderFindStandardFormat (display,
                                               depth == 32 ? PictStandardARGB32
                                               : depth == 24 ? PictStandardRGB24
-                                              : PictStandardA8);
+                                              : depth ==  8 ? PictStandardA8
+                                              : depth ==  4 ? PictStandardA4
+                                              : PictStandardA1);
           *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);
         }
       else
         {
-          image_error ("Specified image bit depth is not supported by XRender");
+          Lisp_Object depth_err;
+          XSETINT(depth_err, depth);
+          image_error ("Specified image bit depth (%d) is not supported by XRender", depth_err);
           *picture = 0;
         }
     }

the error in *Messages* doesn't appear.

However the png-image is still not scaled, and still shown in the larger
canvas.

I apparently haven't got Cairo enabled in my build, maybe that's a
problem?

  asjo@tullinup:/usr/src/emacs$ grep CAIRO src/config.h
  /* #undef USE_CAIRO */

An observation: when displaying the colormapped .png,
image_create_x_image_and_pixmap_1() is called twice, the first time
depth is <= 0, and the second time it isn't.

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Hm.  Odd.  When I try
>> 
>> (insert-image (create-image "/tmp/foo.png" nil nil :scale 10))
>> 
>> etc, I get the proper scaling with png and gif, but not with xpm.
>> (Running from HEAD with -Q.)
>
> Maybe there's some X-specific problem.  I see no problem on
> MS-Windows.

Did you try with the test images I posted?  The problem is that png
images with a colormap aren't scaled, but rgb (etc) ones are.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> OK, that makes sense. Check out the comment and code at line 2593 of
> image.c:
>
>     /* FIXME: Do we need to handle all possible bit depths?
>        XRenderFindStandardFormat supports PictStandardARGB32,
>        PictStandardRGB24, PictStandardA8, PictStandardA4,
>        PictStandardA1, and PictStandardNUM (what is this?!).
>
>        XRenderFindFormat may support more, but I don't
>        understand the documentation.  */
>     format = XRenderFindStandardFormat (display,
>                                         depth == 32 ? PictStandardARGB32
>                                         : depth == 24 ? PictStandardRGB24
>                                         : PictStandardA8);
>     *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);

But isn't that about the depth of the screen, not the number of bits in
the image?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Lars Ingebrigtsen]

Adam Sjøgren <asjo@koldfront.dk> writes:

>> There should be an error message somewhere telling you that Emacs
>> doesn’t support scaling with that bit depth.
>
> Indeed there is for the:
>
>   /tmp/wry.png: PNG image data, 16 x 16, 4-bit colormap, non-interlaced
>
> image. But not for the .xpm. Ah, but xpm doesn't go through
> image_create_x_image_and_pixmap_1(), so that explains that.

Where do you get the error message?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Lars writes:

> Where do you get the error message?

In the *Messages* buffer.

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Alan writes:

> Although I don’t understand why the png displays unscaled in a big
> canvas. We must have missed a check against the Picture member to
> prevent it resizing if there is no Picture.
>
> This is at the top of image_set_transform:
>
>     # if !defined USE_CAIRO && defined HAVE_XRENDER
>       if (!img->picture)
>         return;
>     # endif
>
> which should prevent any attempt to scale right at the start... I’m
> really not sure what’s going on there.

This is what stops the .xpm from being scaled, I think. At least I see
image_set_transform() being called for the .xpm, and that if() is hit
and it returns early.

The colormapped .png, however, passes by that if() and the entire
function is executed.

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Lars Ingebrigtsen]

Adam Sjøgren <asjo@koldfront.dk> writes:

> Lars writes:
>
>> Where do you get the error message?
>
> In the *Messages* buffer.

Hm.  I don't get any on saying

(insert-image (create-image "/tmp/wry.png" nil nil :scale 10))

Do I have to switch error reporting to verbose somehow?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Lars writes:

>>> Where do you get the error message?
>>
>> In the *Messages* buffer.
>
> Hm.  I don't get any on saying
>
> (insert-image (create-image "/tmp/wry.png" nil nil :scale 10))
>
> Do I have to switch error reporting to verbose somehow?

I may have confused myself.

I get the error in *Messages* the _first_ time I display the .gif.

I only get the error in *Messages* on the .xpm, if it is the first image
I try to insert.

I don't get it on either .png's now (so I maybe have been cross-eyed
when I said I did previously).

This is not for kids.

bug#38109: Another data point

[Unknown]

Another data point: if I configure --with-cairo, all images are scaled
as expected.

bug#38109: Another data point

[Lars Ingebrigtsen]

Unknown <unknown@unknown.invalid> writes:

> Another data point: if I configure --with-cairo, all images are scaled
> as expected.

Yup; here too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

On Fri, Nov 08, 2019 at 10:03:17PM +0100, Adam Sjøgren wrote:
> I apparently haven't got Cairo enabled in my build, maybe that's a
> problem?

No, this should all work without Cairo.

>   asjo@tullinup:/usr/src/emacs$ grep CAIRO src/config.h
>   /* #undef USE_CAIRO */
> 
> An observation: when displaying the colormapped .png,
> image_create_x_image_and_pixmap_1() is called twice, the first time
> depth is <= 0, and the second time it isn't.

Does the RGB png call that function twice? The second time through is
for setting the mask, or transparency, and it looks like it sets depth
to ‘1’, which image_create_x_image_and_pixmap_1 doesn’t like with
xrender. Although your patch should have fixed that.

But in addition you might need this:

modified   src/image.c
@@ -2244,6 +2244,13 @@ image_set_transform (struct frame *f, struct image *img)
       XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
 			       0, 0);
       XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+
+      if (img->mask_picture)
+        {
+          XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture, FilterBest,
+                                   0, 0);
+          XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture, &tmat);
+        }
     }
 # elif defined HAVE_NTGUI
   /* Store the transform matrix for application at draw time.  */

As I don’t think we were setting the transform for the mask to match
the image. Although I don’t really know for sure if it’s needed.

It really wouldn’t surprise me too much if this was all related to
masks, I never managed to get a satisfactory test going.

OTOH, if XPMs don’t even use these functions then that would certainly
cause scaling to fail. I’ll have to have a look at the XPM code to
find out what they’re doing instead.
-- 
Alan Third

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

On Fri, Nov 08, 2019 at 10:11:13PM +0100, Lars Ingebrigtsen wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > OK, that makes sense. Check out the comment and code at line 2593 of
> > image.c:
> >
> >     /* FIXME: Do we need to handle all possible bit depths?
> >        XRenderFindStandardFormat supports PictStandardARGB32,
> >        PictStandardRGB24, PictStandardA8, PictStandardA4,
> >        PictStandardA1, and PictStandardNUM (what is this?!).
> >
> >        XRenderFindFormat may support more, but I don't
> >        understand the documentation.  */
> >     format = XRenderFindStandardFormat (display,
> >                                         depth == 32 ? PictStandardARGB32
> >                                         : depth == 24 ? PictStandardRGB24
> >                                         : PictStandardA8);
> >     *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);
> 
> But isn't that about the depth of the screen, not the number of bits in
> the image?

Ah, yes, you’re right. Most of the time. Masks use it set to 1.

-- 
Alan Third

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: asjo@koldfront.dk,  38109@debbugs.gnu.org
> Date: Fri, 08 Nov 2019 22:04:25 +0100
> 
> >> (insert-image (create-image "/tmp/foo.png" nil nil :scale 10))
> >> 
> >> etc, I get the proper scaling with png and gif, but not with xpm.
> >> (Running from HEAD with -Q.)
> >
> > Maybe there's some X-specific problem.  I see no problem on
> > MS-Windows.
> 
> Did you try with the test images I posted?

I did now, and I see them scaled fine.  (This is a build without
ImageMagick, in case this matters.)

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Eli Zaretskii]

> Date: Fri, 08 Nov 2019 22:03:17 +0100
> From: Adam Sjøgren via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> However the png-image is still not scaled, and still shown in the larger
> canvas.

What do you mean by "shown in the larger canvas"?  Can you post a
screenshot of how you see those PNG images with and without scaling,
after applying that patch?

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

Eli writes:

>> However the png-image is still not scaled, and still shown in the larger
>> canvas.

> What do you mean by "shown in the larger canvas"?

I mean that the original 16x16 pixel image is shown in the corner of a
320x320 pixel white box.

As can be seen in the screenshot here:

 · https://debbugs.gnu.org/cgi/bugreport.cgi?att=1;filename=emacs27-image-scaling.png;bug=38109;msg=5

> Can you post a screenshot of how you see those PNG images with and
> without scaling, after applying that patch?

The patch doesn't make any difference on the scaling of the PNG images.

--with-cairo does.

Here is a screenshot of the attached .png with and without scaling,
without the patch and without Cairo (i.e. with Xrender):


[-- Attachment #2: without-patch.png --]
[-- Type: image/png, Size: 47567 bytes --]

[-- Attachment #3: Type: text/plain, Size: 104 bytes --]


Here is a screenshot of the attached .png with and without scaling, with
the patch and without Cairo:


[-- Attachment #4: with-patch.png --]
[-- Type: image/png, Size: 46954 bytes --]

[-- Attachment #5: Type: text/plain, Size: 81 bytes --]


Here is what happens when I also apply Alan's patch for
image_set_transform():


[-- Attachment #6: with-two-patches.png --]
[-- Type: image/png, Size: 50907 bytes --]

[-- Attachment #7: Type: text/plain, Size: 89 bytes --]


And finally what I see with no patches and --with-cairo (this is the
expected result):


[-- Attachment #8: with-cairo.png --]
[-- Type: image/png, Size: 88525 bytes --]

[-- Attachment #9: Type: text/plain, Size: 119 bytes --]


Here is the image I'm using for testing:

  medium-wry.png: PNG image data, 16 x 16, 4-bit colormap, non-interlaced:


[-- Attachment #10: medium-wry.png --]
[-- Type: image/png, Size: 404 bytes --]

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Eli Zaretskii]

> From: Adam Sjøgren <asjo@koldfront.dk>
> Cc: alan@idiocy.org,  larsi@gnus.org,  38109@debbugs.gnu.org
> Date: Sat, 09 Nov 2019 11:28:42 +0100
> 
> >> However the png-image is still not scaled, and still shown in the larger
> >> canvas.
> 
> > What do you mean by "shown in the larger canvas"?
> 
> I mean that the original 16x16 pixel image is shown in the corner of a
> 320x320 pixel white box.
> 
> As can be seen in the screenshot here:
> 
>  · https://debbugs.gnu.org/cgi/bugreport.cgi?att=1;filename=emacs27-image-scaling.png;bug=38109;msg=5
> 
> > Can you post a screenshot of how you see those PNG images with and
> > without scaling, after applying that patch?
> 
> The patch doesn't make any difference on the scaling of the PNG images.
> 
> --with-cairo does.

Thanks for the screenshots.  I see on MS-Windows what you see with
Cairo.  So I guess this means the problem is specific to the
configuration with XRender.

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

> It really wouldn’t surprise me too much if this was all related to
> masks, I never managed to get a satisfactory test going.
> 
> OTOH, if XPMs don’t even use these functions then that would certainly
> cause scaling to fail. I’ll have to have a look at the XPM code to
> find out what they’re doing instead.

OK, so when there’s a mask there’s a different function called in
xterm.c to draw the image. I’m not sure what the deal with that is but
I had to modify it to use x_composite_image. We also need pretty much
every other patch that we’ve posted to this thread, so I’ve attached
something that works for me.

I want to go back and add a couple of comments to it, so it’s not
final, but can you please test it and see if you can break it.

XPMs are still not scaling. I suspect PBMs are in the same boat. It
looks like the code for them creates the pixmaps themselves instead of
using image_create_x_image_and_pixmap, so they bypass the whole
XRender Picture creation. It should be relatively easy to just add in
the creation of a Picture.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-image-scaling-with-masks-bug-38109.patch --]
[-- Type: text/plain, Size: 5006 bytes --]

From 0831791c2c287c0c897ffb0b8bb35fabd890aa36 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 9 Nov 2019 17:04:25 +0000
Subject: [PATCH] Fix image scaling with masks (bug#38109)

* src/image.c (image_clear_image_1): Free the XRender Pictures.
(lookup_image): Move call to image_set_transform after
postprocess_image.
(image_create_x_image_and_pixmap_1): Add 1 bit image type for masks.
* src/xterm.c (x_composite_image): Use PictOpOver when there is a mask
so the transparency is honoured.
(x_draw_image_foreground_1): Use x_composite_image.
---
 src/image.c | 36 ++++++++++++++++++++++++++++++------
 src/xterm.c |  8 ++++----
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/src/image.c b/src/image.c
index 870f008b14..de883faac7 100644
--- a/src/image.c
+++ b/src/image.c
@@ -1465,6 +1465,13 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags)
 	  img->ximg = NULL;
 	  img->background_valid = 0;
 	}
+# ifdef HAVE_XRENDER
+      if (img->picture)
+        {
+          XRenderFreePicture (FRAME_X_DISPLAY (f), img->picture);
+          img->picture = 0;
+        }
+# endif
 #endif
     }
 
@@ -1483,6 +1490,14 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags)
 	  img->mask_img = NULL;
 	  img->background_transparent_valid = 0;
 	}
+# ifdef HAVE_XRENDER
+      if (img->mask_picture)
+        {
+          XRenderFreePicture (FRAME_X_DISPLAY (f), img->mask_picture);
+          img->mask_picture = 0;
+        }
+# endif
+
 #endif
     }
 
@@ -2244,6 +2259,14 @@ image_set_transform (struct frame *f, struct image *img)
       XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
 			       0, 0);
       XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+
+      if (img->mask_picture)
+        {
+          XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture,
+                                   FilterBest, 0, 0);
+          XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture,
+                                      &tmat);
+        }
     }
 # elif defined HAVE_NTGUI
   /* Store the transform matrix for application at draw time.  */
@@ -2313,10 +2336,6 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
 
-#ifdef HAVE_NATIVE_TRANSFORMS
-          image_set_transform (f, img);
-#endif
-
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
 	    img->ascent = XFIXNUM (ascent);
@@ -2357,6 +2376,10 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     don't have the image yet.  */
 	  if (!EQ (builtin_lisp_symbol (img->type->type), Qpostscript))
 	    postprocess_image (f, img);
+
+#ifdef HAVE_NATIVE_TRANSFORMS
+          image_set_transform (f, img);
+#endif
 	}
 
       unblock_input ();
@@ -2585,7 +2608,7 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
     {
       if (depth <= 0)
 	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
-      if (depth == 32 || depth == 24 || depth == 8)
+      if (depth == 32 || depth == 24 || depth == 8 || depth == 1)
         {
           XRenderPictFormat *format;
           XRenderPictureAttributes attr;
@@ -2600,7 +2623,8 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
           format = XRenderFindStandardFormat (display,
                                               depth == 32 ? PictStandardARGB32
                                               : depth == 24 ? PictStandardRGB24
-                                              : PictStandardA8);
+                                              : depth == 8 ? PictStandardA8
+                                              : PictStandardA1);
           *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);
         }
       else
diff --git a/src/xterm.c b/src/xterm.c
index 44fbd27b11..6de1644cb0 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3056,7 +3056,7 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       destination = XRenderCreatePicture (display, dest,
                                           default_format, 0, &attr);
 
-      XRenderComposite (display, PictOpSrc,
+      XRenderComposite (display, s->img->mask_picture ? PictOpOver : PictOpSrc,
                         s->img->picture, s->img->mask_picture, destination,
                         srcX, srcY,
                         srcX, srcY,
@@ -3325,9 +3325,9 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap)
 	  xgcv.function = GXcopy;
 	  XChangeGC (display, s->gc, mask, &xgcv);
 
-	  XCopyArea (display, s->img->pixmap, pixmap, s->gc,
-		     s->slice.x, s->slice.y,
-		     s->slice.width, s->slice.height, x, y);
+	  x_composite_image (s, pixmap,
+                             s->slice.x, s->slice.y,
+                             x, y, s->slice.width, s->slice.height);
 	  XSetClipMask (display, s->gc, None);
 	}
       else
-- 
2.21.0

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Eli Zaretskii]

> Date: Sat, 9 Nov 2019 17:22:41 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 38109@debbugs.gnu.org
> 
> OK, so when there’s a mask there’s a different function called in
> xterm.c to draw the image. I’m not sure what the deal with that is but
> I had to modify it to use x_composite_image. We also need pretty much
> every other patch that we’ve posted to this thread, so I’ve attached
> something that works for me.

Can you explain why you moved the call to image_set_transform?

Thanks.

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

On Sat, Nov 09, 2019 at 07:58:19PM +0200, Eli Zaretskii wrote:
> > Date: Sat, 9 Nov 2019 17:22:41 +0000
> > From: Alan Third <alan@idiocy.org>
> > Cc: Lars Ingebrigtsen <larsi@gnus.org>, 38109@debbugs.gnu.org
> > 
> > OK, so when there’s a mask there’s a different function called in
> > xterm.c to draw the image. I’m not sure what the deal with that is but
> > I had to modify it to use x_composite_image. We also need pretty much
> > every other patch that we’ve posted to this thread, so I’ve attached
> > something that works for me.
> 
> Can you explain why you moved the call to image_set_transform?

postprocess_image can create or modify the mask, which causes problems
if we’ve already changed the width and height of the image. The
simplest solution I could see was to move image_set_transform to after
it, as I don’t think there’s anything in that function that relies on
it having happened already.

-- 
Alan Third

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Eli Zaretskii]

> Date: Sat, 9 Nov 2019 18:11:28 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: asjo@koldfront.dk, larsi@gnus.org, 38109@debbugs.gnu.org
> 
> > Can you explain why you moved the call to image_set_transform?
> 
> postprocess_image can create or modify the mask, which causes problems
> if we’ve already changed the width and height of the image. The
> simplest solution I could see was to move image_set_transform to after
> it, as I don’t think there’s anything in that function that relies on
> it having happened already.

Thanks for explaining.

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> I want to go back and add a couple of comments to it, so it’s not
> final, but can you please test it and see if you can break it.

I did some light testing of the patch, and it fixes the problem with the
non-scaling colormap PNG images, and I didn't see any other adverse
effects the couple of minutes I've used it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

[-- Attachment #1: Type: text/plain, Size: 198 bytes --]

Alan writes:

> I want to go back and add a couple of comments to it, so it’s not
> final, but can you please test it and see if you can break it.

.png is scaling for me with your patch:


[-- Attachment #2: alan9nov.png --]
[-- Type: image/png, Size: 84638 bytes --]

[-- Attachment #3: Type: text/plain, Size: 334 bytes --]


Nice!

> XPMs are still not scaling. I suspect PBMs are in the same boat. It
> looks like the code for them creates the pixmaps themselves instead of
> using image_create_x_image_and_pixmap, so they bypass the whole
> XRender Picture creation. It should be relatively easy to just add in
> the creation of a Picture.

👍

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Hm, since I added the patch, Emacs has crashed a couple of times. The
latest with this output:

  (emacs:11286): GLib-CRITICAL **: 23:08:53.296: Source ID 758 was not found when attempting to remove it
  Fatal error 6: Aborted

  (emacs:11286): GLib-WARNING **: 23:08:53.297: g_main_context_prepare() called recursively from within a source's check() or prepare() member.

  (emacs:11286): GLib-WARNING **: 23:08:53.297: g_main_context_check() called recursively from within a source's check() or prepare() member.
  Aborted

Not sure if it is related, but frankly I can't remember the last time
Emacs crashed on me, so...

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Got another crash after a while of using Gnus:

  Fatal error 6: Aborted

  (emacs:12743): GLib-WARNING **: 23:22:30.538: g_main_context_prepare() called recursively from within a source's check() or prepare() member.

  (emacs:12743): GLib-WARNING **: 23:22:30.538: g_main_context_check() called recursively from within a source's check() or prepare() member.
  Aborted

I will try backing out the patch and see if the crashes disappear.

No crashes during the last 50 minutes with the patch removed.

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

On Sun, Nov 10, 2019 at 12:13:18AM +0100, Adam Sjøgren wrote:
> Got another crash after a while of using Gnus:
> 
>   Fatal error 6: Aborted
> 
>   (emacs:12743): GLib-WARNING **: 23:22:30.538: g_main_context_prepare() called recursively from within a source's check() or prepare() member.
> 
>   (emacs:12743): GLib-WARNING **: 23:22:30.538: g_main_context_check() called recursively from within a source's check() or prepare() member.
>   Aborted
> 
> I will try backing out the patch and see if the crashes disappear.
> 
> No crashes during the last 50 minutes with the patch removed.

Hi Adam, can you try with the patch again, but undoing the changes to
image_clear_image_1 in image.c? There should be two, both wrapped in
‘#ifdef HAVE_XRENDER’.

Those changes weren’t required for the fix, but looked as though they
should be needed generally.

If that doesn’t help can you try running under a debugger and sending
us the backtrace?
-- 
Alan Third

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Alan writes:

> Hi Adam, can you try with the patch again, but undoing the changes to
> image_clear_image_1 in image.c? There should be two, both wrapped in
> ‘#ifdef HAVE_XRENDER’.
>
> Those changes weren’t required for the fix, but looked as though they
> should be needed generally.
>
> If that doesn’t help can you try running under a debugger and sending
> us the backtrace?

I'm trying the patch without those two sections now (finally). No
crashes so far (around an hours worth of using Gnus).


  Best regards,

    Adam

-- 
 "I wish *I* was a tiger!"                                     Adam Sjøgren
 "A common lament."                                       asjo@koldfront.dk
 

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 770 bytes --]

On Sat, Nov 16, 2019 at 05:53:16PM +0100, Adam Sjøgren wrote:
> Alan writes:
> 
> > Hi Adam, can you try with the patch again, but undoing the changes to
> > image_clear_image_1 in image.c? There should be two, both wrapped in
> > ‘#ifdef HAVE_XRENDER’.
> >
> > Those changes weren’t required for the fix, but looked as though they
> > should be needed generally.
> >
> > If that doesn’t help can you try running under a debugger and sending
> > us the backtrace?
> 
> I'm trying the patch without those two sections now (finally). No
> crashes so far (around an hours worth of using Gnus).

I’m pretty sure we need to free the Pictures there, so perhaps we need
to free the Picture before freeing the associated pixmap...

New patch attached.
-- 
Alan Third

[-- Attachment #2: v2-0001-Fix-image-scaling-with-masks-bug-38109.patch --]
[-- Type: text/plain, Size: 5274 bytes --]

From 00d2632c91bc8a930aaeb87018aa9f06dd5dbecd Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 9 Nov 2019 17:04:25 +0000
Subject: [PATCH v2] Fix image scaling with masks (bug#38109)

* src/image.c (image_clear_image_1): Free the XRender Pictures.
(lookup_image): Move call to image_set_transform after
postprocess_image.
(image_create_x_image_and_pixmap_1): Add 1 bit image type for masks.
* src/xterm.c (x_composite_image): Use PictOpOver when there is a mask
so the transparency is honoured.
(x_draw_image_foreground_1): Use x_composite_image.
---
 src/image.c | 35 +++++++++++++++++++++++++++++------
 src/xterm.c |  8 ++++----
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/src/image.c b/src/image.c
index 870f008b14..f872cb31fb 100644
--- a/src/image.c
+++ b/src/image.c
@@ -1453,6 +1453,13 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags)
     {
       if (img->pixmap)
 	{
+#if !defined USE_CAIRO && defined HAVE_XRENDER
+          if (img->picture)
+            {
+              XRenderFreePicture (FRAME_X_DISPLAY (f), img->picture);
+              img->picture = 0;
+            }
+# endif
 	  FRAME_TERMINAL (f)->free_pixmap (f, img->pixmap);
 	  img->pixmap = NO_PIXMAP;
 	  /* NOTE (HAVE_NS): background color is NOT an indexed color! */
@@ -1472,6 +1479,13 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags)
     {
       if (img->mask)
 	{
+#if !defined USE_CAIRO && defined HAVE_XRENDER
+          if (img->mask_picture)
+            {
+              XRenderFreePicture (FRAME_X_DISPLAY (f), img->mask_picture);
+              img->mask_picture = 0;
+            }
+#endif
 	  FRAME_TERMINAL (f)->free_pixmap (f, img->mask);
 	  img->mask = NO_PIXMAP;
 	  img->background_transparent_valid = 0;
@@ -2244,6 +2258,14 @@ image_set_transform (struct frame *f, struct image *img)
       XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
 			       0, 0);
       XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+
+      if (img->mask_picture)
+        {
+          XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture,
+                                   FilterBest, 0, 0);
+          XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture,
+                                      &tmat);
+        }
     }
 # elif defined HAVE_NTGUI
   /* Store the transform matrix for application at draw time.  */
@@ -2313,10 +2335,6 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
 
-#ifdef HAVE_NATIVE_TRANSFORMS
-          image_set_transform (f, img);
-#endif
-
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
 	    img->ascent = XFIXNUM (ascent);
@@ -2357,6 +2375,10 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     don't have the image yet.  */
 	  if (!EQ (builtin_lisp_symbol (img->type->type), Qpostscript))
 	    postprocess_image (f, img);
+
+#ifdef HAVE_NATIVE_TRANSFORMS
+          image_set_transform (f, img);
+#endif
 	}
 
       unblock_input ();
@@ -2585,7 +2607,7 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
     {
       if (depth <= 0)
 	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
-      if (depth == 32 || depth == 24 || depth == 8)
+      if (depth == 32 || depth == 24 || depth == 8 || depth == 1)
         {
           XRenderPictFormat *format;
           XRenderPictureAttributes attr;
@@ -2600,7 +2622,8 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
           format = XRenderFindStandardFormat (display,
                                               depth == 32 ? PictStandardARGB32
                                               : depth == 24 ? PictStandardRGB24
-                                              : PictStandardA8);
+                                              : depth == 8 ? PictStandardA8
+                                              : PictStandardA1);
           *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);
         }
       else
diff --git a/src/xterm.c b/src/xterm.c
index 44fbd27b11..6de1644cb0 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3056,7 +3056,7 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       destination = XRenderCreatePicture (display, dest,
                                           default_format, 0, &attr);
 
-      XRenderComposite (display, PictOpSrc,
+      XRenderComposite (display, s->img->mask_picture ? PictOpOver : PictOpSrc,
                         s->img->picture, s->img->mask_picture, destination,
                         srcX, srcY,
                         srcX, srcY,
@@ -3325,9 +3325,9 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap)
 	  xgcv.function = GXcopy;
 	  XChangeGC (display, s->gc, mask, &xgcv);
 
-	  XCopyArea (display, s->img->pixmap, pixmap, s->gc,
-		     s->slice.x, s->slice.y,
-		     s->slice.width, s->slice.height, x, y);
+	  x_composite_image (s, pixmap,
+                             s->slice.x, s->slice.y,
+                             x, y, s->slice.width, s->slice.height);
 	  XSetClipMask (display, s->gc, None);
 	}
       else
-- 
2.21.0

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Alan writes:

>> I'm trying the patch without those two sections now (finally). No
>> crashes so far (around an hours worth of using Gnus).

> I’m pretty sure we need to free the Pictures there, so perhaps we need
> to free the Picture before freeing the associated pixmap...
>
> New patch attached.

After a while with this patch running Gnus, Emacs crashed like this:

  Fatal error 6: Aborted

  (emacs:56492): GLib-WARNING **: 19:05:37.714: g_main_context_prepare() called recursively from within a source's check() or prepare() member.

  (emacs:56492): GLib-WARNING **: 19:05:37.715: g_main_context_check() called recursively from within a source's check() or prepare() member.
  Aborted


  Best regards,

    Adam

-- 
 "I wish *I* was a tiger!"                                     Adam Sjøgren
 "A common lament."                                       asjo@koldfront.dk
 

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Adam writes:

>> New patch attached.
>
> After a while with this patch running Gnus, Emacs crashed like this:

Here is a backtrace running Emacs with gdb (I don't really know how to
use gdb, just "run" and "bt"):

  (gdb) run
  Starting program: /usr/src/emacs/src/emacs 
  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
  [New Thread 0x7ffff0b32700 (LWP 59023)]
  [New Thread 0x7fffebfff700 (LWP 59024)]
  [New Thread 0x7fffeb7fe700 (LWP 59025)]
  [Detaching after vfork from child process 59026]
  [Detaching after vfork from child process 59027]
  [Detaching after vfork from child process 59028]
  [Detaching after vfork from child process 59030]
  [Detaching after vfork from child process 59031]
  [Detaching after vfork from child process 59032]
  [Detaching after vfork from child process 59033]
  [Detaching after vfork from child process 59034]
  [Detaching after vfork from child process 59035]
  [Detaching after vfork from child process 59036]
  [Detaching after vfork from child process 59037]
  [Detaching after vfork from child process 59039]
  [Detaching after vfork from child process 59040]
  [Detaching after vfork from child process 59041]
  [Detaching after vfork from child process 59042]
  [Detaching after vfork from child process 59043]
  [Detaching after vfork from child process 59044]
  [New Thread 0x7ffff001ee00 (LWP 59046)]
  [Thread 0x7ffff001ee00 (LWP 59046) exited]
  [Detaching after vfork from child process 59073]
  [New Thread 0x7ffff001ee00 (LWP 59098)]
  [New Thread 0x7fffea845700 (LWP 59099)]
  [Thread 0x7fffea845700 (LWP 59099) exited]
  [Thread 0x7ffff001ee00 (LWP 59098) exited]
  [New Thread 0x7ffff001ee00 (LWP 59101)]
  [Thread 0x7ffff001ee00 (LWP 59101) exited]
  [Detaching after vfork from child process 59586]
  [Detaching after vfork from child process 59587]
  Fatal error 6: Aborted

  (emacs:59019): GLib-WARNING **: 19:46:35.390: g_main_context_prepare() called recursively from within a source's check() or prepare() member.

  (emacs:59019): GLib-WARNING **: 19:46:35.390: g_main_context_check() called recursively from within a source's check() or prepare() member.

  Thread 1 "emacs" received signal SIGABRT, Aborted.
  raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
  50	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
  (gdb) bt
  #0  0x00007ffff51873b1 in raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
  #1  0x0000555555596a2b in terminate_due_to_signal (sig=sig@entry=6, backtrace_limit=backtrace_limit@entry=40)
      at emacs.c:401
  #2  0x0000555555596ecc in emacs_abort () at sysdep.c:2450
  #3  0x0000555555594416 in redisplay_internal () at lisp.h:1032
  #4  0x00005555555dcd32 in redisplay_preserve_echo_area (from_where=from_where@entry=13) at xdisp.c:15938
  #5  0x0000555555720450 in Fdelete_process (process=0x5555580d18d5) at process.c:1095
  #6  0x0000555555727b05 in kill_buffer_processes (buffer=buffer@entry=0x0) at process.c:8007
  #7  0x0000555555672579 in shut_down_emacs (sig=sig@entry=6, stuff=stuff@entry=0x0) at lisp.h:1032
  #8  0x00005555555969f8 in terminate_due_to_signal (sig=sig@entry=6, backtrace_limit=backtrace_limit@entry=40)
      at lisp.h:1032
  #9  0x0000555555596ecc in emacs_abort () at sysdep.c:2450
  #10 0x0000555555594416 in redisplay_internal () at lisp.h:1032
  #11 0x00005555555dcd32 in redisplay_preserve_echo_area (from_where=from_where@entry=13) at xdisp.c:15938
  #12 0x0000555555720450 in Fdelete_process (process=0x555556429155) at process.c:1095
  #13 0x0000555555727b05 in kill_buffer_processes (buffer=buffer@entry=0x0) at process.c:8007
  #14 0x00005555556724bd in shut_down_emacs (sig=sig@entry=0, stuff=stuff@entry=0x0) at lisp.h:1032
  #15 0x0000555555595f93 in x_connection_closed (dpy=dpy@entry=0x555555c570c0, error_message=<optimized out>, 
      error_message@entry=0x7fffffff6820 "X protocol error: RenderBadPicture (invalid Picture parameter) on protocol request 139", ioerror=ioerror@entry=false) at lisp.h:1032
  #16 0x000055555564886a in x_error_quitter
      (display=display@entry=0x555555c570c0, event=<optimized out>, event=<optimized out>) at xterm.c:10153
  #17 0x00005555556488f6 in x_error_handler (display=0x555555c570c0, event=0x7fffffff69e0) at xterm.c:10123
  #18 0x00007ffff685014b in _XError () at /lib/x86_64-linux-gnu/libX11.so.6
  #19 0x00007ffff684cf77 in  () at /lib/x86_64-linux-gnu/libX11.so.6
  #20 0x00007ffff684d015 in  () at /lib/x86_64-linux-gnu/libX11.so.6
  #21 0x00007ffff684d95a in _XEventsQueued () at /lib/x86_64-linux-gnu/libX11.so.6
  #22 0x00007ffff683f511 in XPending () at /lib/x86_64-linux-gnu/libX11.so.6
  #23 0x00007ffff70c6a6f in  () at /lib/x86_64-linux-gnu/libgdk-3.so.0
  #24 0x00007ffff6bc361f in g_main_context_prepare () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
  #25 0x00007ffff6bc3fcb in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
  #26 0x00007ffff6bc4168 in g_main_context_pending () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
  #27 0x00007ffff739776e in gtk_events_pending () at /lib/x86_64-linux-gnu/libgtk-3.so.0
  #28 0x00005555556459ad in XTread_socket (terminal=<optimized out>, hold_quit=0x7fffffff6cd0) at xterm.c:9379
  #29 0x0000555555679462 in gobble_input () at keyboard.c:6874
  #30 0x0000555555679a15 in handle_async_input () at keyboard.c:7111
  #31 0x0000555555679a15 in process_pending_signals () at keyboard.c:7125
  #32 0x00005555555dcd1c in redisplay_preserve_echo_area (from_where=from_where@entry=12) at xdisp.c:15931
  #33 0x000055555572548b in wait_reading_process_output
      (time_limit=<optimized out>, nsecs=<optimized out>, read_kbd=read_kbd@entry=-1, do_display=do_display@entry=true, wait_for_cell=wait_for_cell@entry=0x0, wait_proc=wait_proc@entry=0x0, just_wait_proc=0) at process.c:5786
  #34 0x000055555567d491 in kbd_buffer_get_event (end_time=0x7fffffff7760, used_mouse_menu=0x0, kbp=<synthetic pointer>)
      at lisp.h:1032
  #35 0x000055555567d491 in read_event_from_main_queue
      (used_mouse_menu=0x0, local_getcjmp=0x7fffffff7490, end_time=0x7fffffff7760) at keyboard.c:2151
  #36 0x000055555567d491 in read_decoded_event_from_main_queue
      (used_mouse_menu=<optimized out>, prev_event=<optimized out>, local_getcjmp=<optimized out>, end_time=<optimized out>) at keyboard.c:2215
  #37 0x000055555567d491 in read_char
      (commandflag=commandflag@entry=0, map=map@entry=0x0, prev_event=prev_event@entry=0x0, used_mouse_menu=used_mouse_menu@entry=0x0, end_time=0x7fffffff7760) at keyboard.c:2825
  #38 0x000055555570762e in read_filtered_event
      (no_switch_frame=false, ascii_required=false, error_nonascii=false, input_method=<optimized out>, seconds=0x6)
      at lisp.h:1032
  #39 0x00005555556e6e93 in Ffuncall (nargs=4, args=args@entry=0x7fffffff7830) at lisp.h:2109
  #40 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at bytecode.c:633
  #41 0x00005555556e6df7 in Ffuncall (nargs=2, args=args@entry=0x7fffffff7ba8) at eval.c:2808
  #42 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at bytecode.c:633
  #43 0x00005555556e6df7 in Ffuncall (nargs=5, args=args@entry=0x7fffffff7ef0) at eval.c:2808
  --Type <RET> for more, q to quit, c to continue without paging--
  #44 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at bytecode.c:633
  #45 0x00005555556e6df7 in Ffuncall (nargs=2, args=args@entry=0x7fffffff8270) at eval.c:2808
  #46 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at bytecode.c:633
  #47 0x00005555556e6df7 in Ffuncall (nargs=6, args=args@entry=0x7fffffff8678) at eval.c:2808
  #48 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #49 0x00005555556e91ae in funcall_lambda (fun=0x555558037db5, nargs=2, arg_vector=0x7fffffff8c38) at lisp.h:1852
  #50 0x00005555556e6df7 in Ffuncall (nargs=3, args=args@entry=0x7fffffff8c30) at eval.c:2808
  #51 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #52 0x00005555556e91ae in funcall_lambda (fun=0x555558033f85, nargs=2, arg_vector=0x7fffffff9060) at lisp.h:1852
  #53 0x00005555556e6df7 in Ffuncall (nargs=3, args=args@entry=0x7fffffff9058) at eval.c:2808
  #54 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #55 0x00005555556e91ae in funcall_lambda (fun=0x5555580441e5, nargs=6, arg_vector=0x7fffffff9590) at lisp.h:1852
  #56 0x00005555556e6df7 in Ffuncall (nargs=7, args=args@entry=0x7fffffff9588) at eval.c:2808
  #57 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #58 0x00005555556e91ae in funcall_lambda (fun=0x555558043f15, nargs=4, arg_vector=0x7fffffff9910) at lisp.h:1852
  #59 0x00005555556e6df7 in Ffuncall (nargs=5, args=args@entry=0x7fffffff9908) at eval.c:2808
  #60 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #61 0x00005555556e91ae in funcall_lambda (fun=0x555557ee73d5, nargs=2, arg_vector=0x7fffffff9cb0) at lisp.h:1852
  #62 0x00005555556e6df7 in Ffuncall (nargs=3, args=args@entry=0x7fffffff9ca8) at eval.c:2808
  #63 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #64 0x00005555556e91ae in funcall_lambda (fun=0x555557fbb5a5, nargs=2, arg_vector=0x7fffffffa050) at lisp.h:1852
  #65 0x00005555556e6df7 in Ffuncall (nargs=3, args=args@entry=0x7fffffffa048) at eval.c:2808
  #66 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #67 0x00005555556e91ae in funcall_lambda (fun=0x5555577d3785, nargs=2, arg_vector=0x7fffffffaf10) at lisp.h:1852
  #68 0x00005555556e6df7 in Ffuncall (nargs=3, args=args@entry=0x7fffffffaf08) at eval.c:2808
  #69 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #70 0x00005555556e91ae in funcall_lambda (fun=0x555558002b25, nargs=3, arg_vector=0x7fffffffb400) at lisp.h:1852
  #71 0x00005555556e6df7 in Ffuncall (nargs=4, args=args@entry=0x7fffffffb3f8) at eval.c:2808
  #72 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #73 0x00005555556e91ae in funcall_lambda (fun=0x555557f36635, nargs=3, arg_vector=0x7fffffffd270) at lisp.h:1852
  #74 0x00005555556e6df7 in Ffuncall (nargs=4, args=args@entry=0x7fffffffd268) at eval.c:2808
  #75 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=args_template@entry=0x0, nargs=nargs@entry=0, args=<optimized out>, args@entry=0x0) at bytecode.c:633
  #76 0x00005555556e91ae in funcall_lambda (fun=0x555557ea7d15, nargs=0, arg_vector=0x7fffffffd590) at lisp.h:1852
  #77 0x00005555556e8853 in apply_lambda (fun=0x555557ea7d15, args=<optimized out>, count=count@entry=11) at eval.c:2926
  #78 0x00005555556e8b23 in eval_sub (form=<optimized out>) at eval.c:2348
  #79 0x00005555556e902d in Fprogn (body=0x555557e701e3, body@entry=0x555557e70183) at eval.c:462
  #80 0x00005555556daa0b in Fsave_excursion (args=0x555557e70183) at editfns.c:842
  #81 0x00005555556e8c99 in eval_sub (form=<optimized out>) at lisp.h:2109
  #82 0x00005555556e9305 in Fprogn (body=0x555557e70293) at eval.c:462
  --Type <RET> for more, q to quit, c to continue without paging--
  #83 0x00005555556e9305 in funcall_lambda (fun=0x555557e70323, nargs=0, arg_vector=0x7fffffffda10) at eval.c:3060
  #84 0x00005555556e6df7 in Ffuncall (nargs=nargs@entry=1, args=args@entry=0x7fffffffda08) at eval.c:2808
  #85 0x00005555556e38a1 in Ffuncall_interactively (nargs=1, args=0x7fffffffda08) at callint.c:254
  #86 0x00005555556e6e93 in Ffuncall (nargs=2, args=0x7fffffffda00) at lisp.h:2109
  #87 0x00005555556e719c in Fapply (nargs=nargs@entry=3, args=args@entry=0x7fffffffda00) at eval.c:2377
  #88 0x00005555556e4dba in Fcall_interactively (function=0x23f67c0, record_flag=0x0, keys=0x5555580d22d5)
      at lisp.h:1032
  #89 0x00005555556e6e93 in Ffuncall (nargs=4, args=args@entry=0x7fffffffdaf8) at lisp.h:2109
  #90 0x000055555571ac18 in exec_byte_code
      (bytestr=<optimized out>, vector=<optimized out>, maxdepth=<optimized out>, args_template=<optimized out>, nargs=<optimized out>, args=<optimized out>) at bytecode.c:633
  #91 0x00005555556e6df7 in Ffuncall (nargs=2, args=0x7fffffffde80) at eval.c:2808
  #92 0x00005555556e6f3a in call1 (fn=fn@entry=0x41a0, arg1=<optimized out>) at eval.c:2654
  #93 0x0000555555680f78 in command_loop_1 () at lisp.h:1032
  #94 0x00005555556e61a7 in internal_condition_case
      (bfun=bfun@entry=0x555555680ba0 <command_loop_1>, handlers=handlers@entry=0x90, hfun=hfun@entry=0x555555677d20 <cmd_error>) at eval.c:1355
  #95 0x0000555555672a64 in command_loop_2 (ignore=ignore@entry=0x0) at lisp.h:1032
  #96 0x00005555556e6101 in internal_catch
      (tag=tag@entry=0xd0e0, func=func@entry=0x555555672a40 <command_loop_2>, arg=arg@entry=0x0) at eval.c:1116
  #97 0x0000555555672a0b in command_loop () at lisp.h:1032
  #98 0x0000555555677936 in recursive_edit_1 () at keyboard.c:714
  #99 0x0000555555677c62 in Frecursive_edit () at keyboard.c:786
  #100 0x000055555559d50a in main (argc=1, argv=<optimized out>) at emacs.c:2055
  (gdb) 

I hope it's useful.


  Best regards,

    Adam

-- 
 "I wish *I* was a tiger!"                                     Adam Sjøgren
 "A common lament."                                       asjo@koldfront.dk
 

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 808 bytes --]

On Sun, Nov 17, 2019 at 07:49:56PM +0100, Adam Sjøgren wrote:
> Adam writes:
> 
> >> New patch attached.
> >
> > After a while with this patch running Gnus, Emacs crashed like this:
> 
> Here is a backtrace running Emacs with gdb (I don't really know how to
> use gdb, just "run" and "bt"):

Thanks for trying. I’m giving up on freeing the Pictures there,
although I strongly suspect it’s adding a memory leak.

New patch attached which should resize xpms and xbms. I noticed two problems:

  1. sometimes shrinking an image leaves some pixels behind in clear
  areas.
  
  2. In image mode if I shrink an image enough so that it disappears
  from view completely I can’t then scale it up with +. I suspect
  something’s going to zero and subsequent calculations all stay at
  zero.

-- 
Alan Third

[-- Attachment #2: v3-0001-Fix-image-scaling-with-masks-bug-38109.patch --]
[-- Type: text/plain, Size: 9007 bytes --]

From 8aeed43bbbf1931c4b5ef1ca0e61103ca9fcaf6c Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 9 Nov 2019 17:04:25 +0000
Subject: [PATCH v3] Fix image scaling with masks (bug#38109)

* src/image.c (lookup_image): Move call to image_set_transform after
postprocess_image.
(image_create_x_image_and_pixmap_1): Use new function.
(image_set_transform): Apply the transform to the mask too.
(x_create_xrender_picture): New function.
(Create_Pixmap_From_Bitmap_Data):
(xpm_load): Use new function.
* src/xterm.c (x_composite_image): Use PictOpOver when there is a mask
so the transparency is honoured.
(x_draw_image_foreground_1): Use x_composite_image.
---
 src/image.c | 139 +++++++++++++++++++++++++++++++++++-----------------
 src/xterm.c |   8 +--
 2 files changed, 97 insertions(+), 50 deletions(-)

diff --git a/src/image.c b/src/image.c
index 870f008b14..6c1898eae0 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2244,6 +2244,14 @@ image_set_transform (struct frame *f, struct image *img)
       XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
 			       0, 0);
       XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+
+      if (img->mask_picture)
+        {
+          XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture,
+                                   FilterBest, 0, 0);
+          XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture,
+                                      &tmat);
+        }
     }
 # elif defined HAVE_NTGUI
   /* Store the transform matrix for application at draw time.  */
@@ -2313,10 +2321,6 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
 
-#ifdef HAVE_NATIVE_TRANSFORMS
-          image_set_transform (f, img);
-#endif
-
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
 	    img->ascent = XFIXNUM (ascent);
@@ -2357,6 +2361,12 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     don't have the image yet.  */
 	  if (!EQ (builtin_lisp_symbol (img->type->type), Qpostscript))
 	    postprocess_image (f, img);
+
+          /* postprocess_image above may modify the image or the mask,
+             so image_set_transform must be called after it.  */
+#ifdef HAVE_NATIVE_TRANSFORMS
+          image_set_transform (f, img);
+#endif
 	}
 
       unblock_input ();
@@ -2527,6 +2537,54 @@ x_destroy_x_image (XImage *ximg)
     }
 }
 
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+/* Create and return an XRender Picture for XRender transforms.  */
+static Picture
+x_create_xrender_picture (struct frame *f, Emacs_Pixmap pixmap, int depth)
+{
+  Picture p;
+  Display *display = FRAME_X_DISPLAY (f);
+  int event_basep, error_basep;
+
+  if (XRenderQueryExtension (display, &event_basep, &error_basep))
+    {
+      if (depth <= 0)
+	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
+      if (depth == 32 || depth == 24 || depth == 8 || depth == 4 || depth == 1)
+        {
+          XRenderPictFormat *format;
+          XRenderPictureAttributes attr;
+
+          /* FIXME: Do we need to handle all possible bit depths?
+             XRenderFindStandardFormat supports PictStandardARGB32,
+             PictStandardRGB24, PictStandardA8, PictStandardA4,
+             PictStandardA1, and PictStandardNUM (what is this?!).
+
+             XRenderFindFormat may support more, but I don't
+             understand the documentation.  */
+          format = XRenderFindStandardFormat (display,
+                                              depth == 32 ? PictStandardARGB32
+                                              : depth == 24 ? PictStandardRGB24
+                                              : depth == 8 ? PictStandardA8
+                                              : depth == 4 ? PictStandardA4
+                                              : PictStandardA1);
+          p = XRenderCreatePicture (display, pixmap, format, 0, &attr);
+        }
+      else
+        {
+          image_error ("Specified image bit depth is not supported by XRender");
+          return 0;
+        }
+    }
+  else
+    {
+      /* XRender not supported on this display.  */
+      return 0;
+    }
+
+  return p;
+}
+# endif /* !defined USE_CAIRO && defined HAVE_XRENDER */
 #endif	/* HAVE_X_WINDOWS */
 
 /* Return true if XIMG's size WIDTH x HEIGHT doesn't break the
@@ -2579,36 +2637,8 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
   if (!x_create_x_image_and_pixmap (f, width, height, depth, pimg, pixmap))
     return 0;
 # ifdef HAVE_XRENDER
-  Display *display = FRAME_X_DISPLAY (f);
-  int event_basep, error_basep;
-  if (picture && XRenderQueryExtension (display, &event_basep, &error_basep))
-    {
-      if (depth <= 0)
-	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
-      if (depth == 32 || depth == 24 || depth == 8)
-        {
-          XRenderPictFormat *format;
-          XRenderPictureAttributes attr;
-
-          /* FIXME: Do we need to handle all possible bit depths?
-             XRenderFindStandardFormat supports PictStandardARGB32,
-             PictStandardRGB24, PictStandardA8, PictStandardA4,
-             PictStandardA1, and PictStandardNUM (what is this?!).
-
-             XRenderFindFormat may support more, but I don't
-             understand the documentation.  */
-          format = XRenderFindStandardFormat (display,
-                                              depth == 32 ? PictStandardARGB32
-                                              : depth == 24 ? PictStandardRGB24
-                                              : PictStandardA8);
-          *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);
-        }
-      else
-        {
-          image_error ("Specified image bit depth is not supported by XRender");
-          *picture = 0;
-        }
-    }
+  if (picture)
+    *picture = x_create_xrender_picture (f, *pixmap, depth);
 # endif
 
   return 1;
@@ -3387,6 +3417,11 @@ Create_Pixmap_From_Bitmap_Data (struct frame *f, struct image *img, char *data,
 				   img->width, img->height,
 				   fg, bg,
 				   DefaultDepthOfScreen (FRAME_X_SCREEN (f)));
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+  if (img->pixmap)
+    img->picture = x_create_xrender_picture (f, img->pixmap, 0);
+# endif
+
 #elif defined HAVE_NTGUI
   img->pixmap
     = w32_create_pixmap_from_bitmap_data (img->width, img->height, data);
@@ -4359,18 +4394,30 @@ xpm_load (struct frame *f, struct image *img)
 	  image_clear_image (f, img);
 	  rc = XpmNoMemory;
 	}
-      else if (img->mask_img)
-	{
-          img->mask = XCreatePixmap (FRAME_X_DISPLAY (f), FRAME_X_DRAWABLE (f),
-				     img->mask_img->width,
-				     img->mask_img->height,
-				     img->mask_img->depth);
-	  if (img->mask == NO_PIXMAP)
-	    {
-	      image_clear_image (f, img);
-	      rc = XpmNoMemory;
-	    }
-	}
+      else
+        {
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+          img->picture = x_create_xrender_picture (f, img->pixmap,
+                                                   img->ximg->depth);
+# endif
+          if (img->mask_img)
+            {
+              img->mask = XCreatePixmap (FRAME_X_DISPLAY (f), FRAME_X_DRAWABLE (f),
+                                         img->mask_img->width,
+                                         img->mask_img->height,
+                                         img->mask_img->depth);
+              if (img->mask == NO_PIXMAP)
+                {
+                  image_clear_image (f, img);
+                  rc = XpmNoMemory;
+                }
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+              else
+                img->mask_picture = x_create_xrender_picture
+                  (f, img->mask, img->mask_img->depth);
+# endif
+            }
+        }
     }
 #endif
 
diff --git a/src/xterm.c b/src/xterm.c
index 44fbd27b11..6de1644cb0 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3056,7 +3056,7 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       destination = XRenderCreatePicture (display, dest,
                                           default_format, 0, &attr);
 
-      XRenderComposite (display, PictOpSrc,
+      XRenderComposite (display, s->img->mask_picture ? PictOpOver : PictOpSrc,
                         s->img->picture, s->img->mask_picture, destination,
                         srcX, srcY,
                         srcX, srcY,
@@ -3325,9 +3325,9 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap)
 	  xgcv.function = GXcopy;
 	  XChangeGC (display, s->gc, mask, &xgcv);
 
-	  XCopyArea (display, s->img->pixmap, pixmap, s->gc,
-		     s->slice.x, s->slice.y,
-		     s->slice.width, s->slice.height, x, y);
+	  x_composite_image (s, pixmap,
+                             s->slice.x, s->slice.y,
+                             x, y, s->slice.width, s->slice.height);
 	  XSetClipMask (display, s->gc, None);
 	}
       else
-- 
2.21.0

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

[-- Attachment #1: Type: text/plain, Size: 123 bytes --]

Alan writes:

> New patch attached which should resize xpms and xbms. I noticed two problems:

Seems to work well for me:


[-- Attachment #2: atv4.png --]
[-- Type: image/png, Size: 106417 bytes --]

[-- Attachment #3: Type: text/plain, Size: 384 bytes --]


Note, though, how the RGB image looks different than the two others, it
has a border added.

I have run Gnus in Emacs for around an hour now without any crashes,
with this patch.


  Best regards,

    Adam

-- 
 "I wish *I* was a tiger!"                                     Adam Sjøgren
 "A common lament."                                       asjo@koldfront.dk
 

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On Sat, Nov 23, 2019 at 01:02:50AM +0100, Adam Sjøgren wrote:
> 
> Note, though, how the RGB image looks different than the two others, it
> has a border added.

This is caused by the smoothing of the image. After a LOT of searching
around I found out we need to set the repeat value for the Picture to
pad so that when it’s applying the filter and looking outside the
image bounds it will look for the nearest real pixel rather than
inventing a transparent black one.

Patch attached.

> I have run Gnus in Emacs for around an hour now without any crashes,
> with this patch.

Thanks!
-- 
Alan Third

[-- Attachment #2: v4-0001-Fix-image-scaling-with-masks-bug-38109.patch --]
[-- Type: text/plain, Size: 10173 bytes --]

From 4efbc28e264e2045d4836492bd7fe388e42da055 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 9 Nov 2019 17:04:25 +0000
Subject: [PATCH v4] Fix image scaling with masks (bug#38109)

* src/image.c (lookup_image): Move call to image_set_transform after
postprocess_image.
(image_create_x_image_and_pixmap_1): Use new function.
(image_set_transform): Apply the transform to the mask too.
(x_create_xrender_picture): New function.
(Create_Pixmap_From_Bitmap_Data):
(xpm_load): Use new function.
* src/xterm.c (x_composite_image): Use PictOpOver when there is a mask
so the transparency is honoured.
(x_draw_image_foreground_1): Use x_composite_image.
---
 src/image.c | 147 ++++++++++++++++++++++++++++++++++++----------------
 src/xterm.c |  11 ++--
 2 files changed, 106 insertions(+), 52 deletions(-)

diff --git a/src/image.c b/src/image.c
index 870f008b14..70d932f9ed 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2244,6 +2244,14 @@ image_set_transform (struct frame *f, struct image *img)
       XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
 			       0, 0);
       XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
+
+      if (img->mask_picture)
+        {
+          XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture,
+                                   FilterBest, 0, 0);
+          XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture,
+                                      &tmat);
+        }
     }
 # elif defined HAVE_NTGUI
   /* Store the transform matrix for application at draw time.  */
@@ -2313,10 +2321,6 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	  Lisp_Object ascent, margin, relief, bg;
 	  int relief_bound;
 
-#ifdef HAVE_NATIVE_TRANSFORMS
-          image_set_transform (f, img);
-#endif
-
 	  ascent = image_spec_value (spec, QCascent, NULL);
 	  if (FIXNUMP (ascent))
 	    img->ascent = XFIXNUM (ascent);
@@ -2357,6 +2361,13 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	     don't have the image yet.  */
 	  if (!EQ (builtin_lisp_symbol (img->type->type), Qpostscript))
 	    postprocess_image (f, img);
+
+          /* postprocess_image above may modify the image or the mask,
+             relying on the image's real width and height, so
+             image_set_transform must be called after it.  */
+#ifdef HAVE_NATIVE_TRANSFORMS
+          image_set_transform (f, img);
+#endif
 	}
 
       unblock_input ();
@@ -2527,6 +2538,61 @@ x_destroy_x_image (XImage *ximg)
     }
 }
 
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+/* Create and return an XRender Picture for XRender transforms.  */
+static Picture
+x_create_xrender_picture (struct frame *f, Emacs_Pixmap pixmap, int depth)
+{
+  Picture p;
+  Display *display = FRAME_X_DISPLAY (f);
+  int event_basep, error_basep;
+
+  if (XRenderQueryExtension (display, &event_basep, &error_basep))
+    {
+      if (depth <= 0)
+	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
+      if (depth == 32 || depth == 24 || depth == 8 || depth == 4 || depth == 1)
+        {
+          /* FIXME: Do we need to handle all possible bit depths?
+             XRenderFindStandardFormat supports PictStandardARGB32,
+             PictStandardRGB24, PictStandardA8, PictStandardA4,
+             PictStandardA1, and PictStandardNUM (what is this?!).
+
+             XRenderFindFormat may support more, but I don't
+             understand the documentation.  */
+          XRenderPictFormat *format;
+          format = XRenderFindStandardFormat (display,
+                                              depth == 32 ? PictStandardARGB32
+                                              : depth == 24 ? PictStandardRGB24
+                                              : depth == 8 ? PictStandardA8
+                                              : depth == 4 ? PictStandardA4
+                                              : PictStandardA1);
+
+          /* Set the Picture repeat to "pad".  This means when
+             operations look at pixels outside the image area they
+             will use the value of the nearest real pixel instead of
+             using a transparent black pixel.  */
+          XRenderPictureAttributes attr;
+          unsigned long attr_mask = CPRepeat;
+          attr.repeat = RepeatPad;
+
+          p = XRenderCreatePicture (display, pixmap, format, attr_mask, &attr);
+        }
+      else
+        {
+          image_error ("Specified image bit depth is not supported by XRender");
+          return 0;
+        }
+    }
+  else
+    {
+      /* XRender not supported on this display.  */
+      return 0;
+    }
+
+  return p;
+}
+# endif /* !defined USE_CAIRO && defined HAVE_XRENDER */
 #endif	/* HAVE_X_WINDOWS */
 
 /* Return true if XIMG's size WIDTH x HEIGHT doesn't break the
@@ -2579,36 +2645,8 @@ image_create_x_image_and_pixmap_1 (struct frame *f, int width, int height, int d
   if (!x_create_x_image_and_pixmap (f, width, height, depth, pimg, pixmap))
     return 0;
 # ifdef HAVE_XRENDER
-  Display *display = FRAME_X_DISPLAY (f);
-  int event_basep, error_basep;
-  if (picture && XRenderQueryExtension (display, &event_basep, &error_basep))
-    {
-      if (depth <= 0)
-	depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
-      if (depth == 32 || depth == 24 || depth == 8)
-        {
-          XRenderPictFormat *format;
-          XRenderPictureAttributes attr;
-
-          /* FIXME: Do we need to handle all possible bit depths?
-             XRenderFindStandardFormat supports PictStandardARGB32,
-             PictStandardRGB24, PictStandardA8, PictStandardA4,
-             PictStandardA1, and PictStandardNUM (what is this?!).
-
-             XRenderFindFormat may support more, but I don't
-             understand the documentation.  */
-          format = XRenderFindStandardFormat (display,
-                                              depth == 32 ? PictStandardARGB32
-                                              : depth == 24 ? PictStandardRGB24
-                                              : PictStandardA8);
-          *picture = XRenderCreatePicture (display, *pixmap, format, 0, &attr);
-        }
-      else
-        {
-          image_error ("Specified image bit depth is not supported by XRender");
-          *picture = 0;
-        }
-    }
+  if (picture)
+    *picture = x_create_xrender_picture (f, *pixmap, depth);
 # endif
 
   return 1;
@@ -3387,6 +3425,11 @@ Create_Pixmap_From_Bitmap_Data (struct frame *f, struct image *img, char *data,
 				   img->width, img->height,
 				   fg, bg,
 				   DefaultDepthOfScreen (FRAME_X_SCREEN (f)));
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+  if (img->pixmap)
+    img->picture = x_create_xrender_picture (f, img->pixmap, 0);
+# endif
+
 #elif defined HAVE_NTGUI
   img->pixmap
     = w32_create_pixmap_from_bitmap_data (img->width, img->height, data);
@@ -4359,18 +4402,30 @@ xpm_load (struct frame *f, struct image *img)
 	  image_clear_image (f, img);
 	  rc = XpmNoMemory;
 	}
-      else if (img->mask_img)
-	{
-          img->mask = XCreatePixmap (FRAME_X_DISPLAY (f), FRAME_X_DRAWABLE (f),
-				     img->mask_img->width,
-				     img->mask_img->height,
-				     img->mask_img->depth);
-	  if (img->mask == NO_PIXMAP)
-	    {
-	      image_clear_image (f, img);
-	      rc = XpmNoMemory;
-	    }
-	}
+      else
+        {
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+          img->picture = x_create_xrender_picture (f, img->pixmap,
+                                                   img->ximg->depth);
+# endif
+          if (img->mask_img)
+            {
+              img->mask = XCreatePixmap (FRAME_X_DISPLAY (f), FRAME_X_DRAWABLE (f),
+                                         img->mask_img->width,
+                                         img->mask_img->height,
+                                         img->mask_img->depth);
+              if (img->mask == NO_PIXMAP)
+                {
+                  image_clear_image (f, img);
+                  rc = XpmNoMemory;
+                }
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+              else
+                img->mask_picture = x_create_xrender_picture
+                  (f, img->mask, img->mask_img->depth);
+# endif
+            }
+        }
     }
 #endif
 
diff --git a/src/xterm.c b/src/xterm.c
index d55bc3890d..9a6eda4488 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3049,14 +3049,12 @@ x_composite_image (struct glyph_string *s, Pixmap dest,
       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 (display,
                                                 DefaultVisual (display, 0));
       destination = XRenderCreatePicture (display, dest,
                                           default_format, 0, &attr);
 
-      XRenderComposite (display, PictOpSrc,
+      XRenderComposite (display, s->img->mask_picture ? PictOpOver : PictOpSrc,
                         s->img->picture, s->img->mask_picture, destination,
                         srcX, srcY,
                         srcX, srcY,
@@ -3315,6 +3313,7 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap)
 	     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;
@@ -3325,9 +3324,9 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap)
 	  xgcv.function = GXcopy;
 	  XChangeGC (display, s->gc, mask, &xgcv);
 
-	  XCopyArea (display, s->img->pixmap, pixmap, s->gc,
-		     s->slice.x, s->slice.y,
-		     s->slice.width, s->slice.height, x, y);
+	  x_composite_image (s, pixmap,
+                             s->slice.x, s->slice.y,
+                             x, y, s->slice.width, s->slice.height);
 	  XSetClipMask (display, s->gc, None);
 	}
       else
-- 
2.21.0

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

Alan writes:

> On Sat, Nov 23, 2019 at 01:02:50AM +0100, Adam Sjøgren wrote:
>> 
>> Note, though, how the RGB image looks different than the two others, it
>> has a border added.
>
> This is caused by the smoothing of the image. After a LOT of searching
> around I found out we need to set the repeat value for the Picture to
> pad so that when it’s applying the filter and looking outside the
> image bounds it will look for the nearest real pixel rather than
> inventing a transparent black one.
>
> Patch attached.

Looks much better after your hunt:


[-- Attachment #2: atv4.png --]
[-- Type: image/png, Size: 97095 bytes --]

[-- Attachment #3: Type: text/plain, Size: 214 bytes --]



  Thanks for fixing this issue!

    Adam

-- 
 "I wish *I* was a tiger!"                                     Adam Sjøgren
 "A common lament."                                       asjo@koldfront.dk
 

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

On Sun, Nov 24, 2019 at 06:52:21PM +0100, Adam Sjøgren wrote:
> Alan writes:
> 
> > On Sat, Nov 23, 2019 at 01:02:50AM +0100, Adam Sjøgren wrote:
> >> 
> >> Note, though, how the RGB image looks different than the two others, it
> >> has a border added.
> >
> > This is caused by the smoothing of the image. After a LOT of searching
> > around I found out we need to set the repeat value for the Picture to
> > pad so that when it’s applying the filter and looking outside the
> > image bounds it will look for the nearest real pixel rather than
> > inventing a transparent black one.
> >
> > Patch attached.
> 
> Looks much better after your hunt:

Every time I look at those screenshots I think that perhaps we should
turn off the smoothing completely, then remember I turned it on
because photos of real things looked awful without it.

Anyway, I’ll wait a few days and then push if nobody objects.

Thanks for your help.
-- 
Alan Third

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Unknown]

Alan writes:

> Every time I look at those screenshots I think that perhaps we should
> turn off the smoothing completely, then remember I turned it on
> because photos of real things looked awful without it.

Good point.

But hard to tell whether to smooth or not, I guess? xpm's are probably
often not-photos, but png's...

> Thanks for your help.

Thank you for fixing the problem!

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Alan Third]

[-- Attachment #1.1: Type: text/plain, Size: 879 bytes --]

On Tue, Nov 26, 2019 at 09:40:06PM +0100, Adam Sjøgren wrote:
> Alan writes:
>
> > Every time I look at those screenshots I think that perhaps we should
> > turn off the smoothing completely, then remember I turned it on
> > because photos of real things looked awful without it.
>
> Good point.
>
> But hard to tell whether to smooth or not, I guess? xpm's are probably
> often not-photos, but png's...

It might be worth smoothing on scaling down, but not on scaling up.
That way if you zoom in you get exact pixels, but zooming out you
don’t get aliasing effects.

I don’t know if that would add any other problems...

Patch attached ‐ this goes on top of the patch for bug#38109:

    https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38109#137

It is also only for XRender. I’ve no idea how to go about doing it in
other terms.
-- 
Alan Third

[-- Attachment #1.2: Type: text/html, Size: 1265 bytes --]

[-- Attachment #2: 0001-Don-t-smooth-images-when-zooming-in.patch --]
[-- Type: application/x-patch, Size: 1737 bytes --]

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> It might be worth smoothing on scaling down, but not on scaling up.
> That way if you zoom in you get exact pixels, but zooming out you
> don’t get aliasing effects.

I think that makes sense, but I wonder whether some people would think
that zoomed-in images then look "blocky"?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38109: Updated Emacs to HEAD, consistently not scaling now

[Alan Third]

On Sun, Nov 24, 2019 at 05:26:12PM +0000, Alan Third wrote:
> On Sat, Nov 23, 2019 at 01:02:50AM +0100, Adam Sjøgren wrote:
> > 
> > Note, though, how the RGB image looks different than the two others, it
> > has a border added.
> 
> This is caused by the smoothing of the image. After a LOT of searching
> around I found out we need to set the repeat value for the Picture to
> pad so that when it’s applying the filter and looking outside the
> image bounds it will look for the nearest real pixel rather than
> inventing a transparent black one.
> 
> Patch attached.

I’ve pushed the patch to master. I’m closing this bug report.
-- 
Alan Third

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Alan Third]

On Wed, Nov 27, 2019 at 01:32:15PM +0100, Lars Ingebrigtsen wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > It might be worth smoothing on scaling down, but not on scaling up.
> > That way if you zoom in you get exact pixels, but zooming out you
> > don’t get aliasing effects.
> 
> I think that makes sense, but I wonder whether some people would think
> that zoomed-in images then look "blocky"?

I think I’d expect to see pixels if I zoomed in rather than a blurry
mess, but that may just be me.

I tried to implement this on the NS port and failed, although I’m sure
it’s possible. I really don’t know if it’s worth bothering with.
-- 
Alan Third

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

>> I think that makes sense, but I wonder whether some people would think
>> that zoomed-in images then look "blocky"?
>
> I think I’d expect to see pixels if I zoomed in rather than a blurry
> mess, but that may just be me.

I'd prefer that, too, but it's not what systems software commonly does,
I think?  For instance, the Apple OS-es aggressively blurify when
upscaling images.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> It might be worth smoothing on scaling down, but not on scaling up.
> That way if you zoom in you get exact pixels, but zooming out you
> don’t get aliasing effects.
>
> I don’t know if that would add any other problems...

You apparently didn't apply this patch?

[...]

> * src/image.c (image_set_transform [HAVE_XRENDER]): Use different filter
> when zooming in vs zooming out.

I think it makes sense...  although I'm not sure of what the effects are
if you just increase the image size by, say, a couple percent.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

On Sun, Aug 02, 2020 at 07:52:25PM +0200, Lars Ingebrigtsen wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > It might be worth smoothing on scaling down, but not on scaling up.
> > That way if you zoom in you get exact pixels, but zooming out you
> > don’t get aliasing effects.
> >
> > I don’t know if that would add any other problems...
> 
> You apparently didn't apply this patch?

There didn't appear to be much enthusiasm so I just left it in case
someone requested it in the future.

> > * src/image.c (image_set_transform [HAVE_XRENDER]): Use different filter
> > when zooming in vs zooming out.
> 
> I think it makes sense...  although I'm not sure of what the effects are
> if you just increase the image size by, say, a couple percent.

I've attached an updated version of this patch and a screenshot
showing the result with a couple of levels of scaling.

Annoyingly we now use Cairo by default which is unaffected by this
change, so only plain X users will see this behaviour.
-- 
Alan Third

[-- Attachment #2: scaling.png --]
[-- Type: image/png, Size: 25663 bytes --]

[-- Attachment #3: 0001-Don-t-smooth-images-when-scaling-up-bug-38394.patch --]
[-- Type: text/plain, Size: 2026 bytes --]

From bdd090bf8cc29bd5427d69f10bf0738a74596ba7 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 2 Aug 2020 20:43:56 +0100
Subject: [PATCH] Don't smooth images when scaling up (bug#38394)

* src/image.c (image_set_transform [HAVE_XRENDER]): Use different filter
when scaling up vs scaling down.
---
 src/image.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/image.c b/src/image.c
index e7e0a93313..ce4c34df7b 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2114,6 +2114,15 @@ image_set_transform (struct frame *f, struct image *img)
   double rotation = 0.0;
   compute_image_rotation (img, &rotation);
 
+# if !defined USE_CAIRO && defined HAVE_XRENDER
+  /* We want scale up operations to use a nearest neighbour filter to
+     show real pixels instead of munging them, but scale down
+     operations to use a blended filter, to avoid aliasing and the like.
+
+     TODO: implement for Cairo, NS and Windows.  */
+  bool scale_down = (width < img->width) || (height < img->height);
+# endif
+
   /* Perform scale transformation.  */
 
   matrix3x3 matrix
@@ -2246,14 +2255,14 @@ image_set_transform (struct frame *f, struct image *img)
              XDoubleToFixed (matrix[1][2]),
              XDoubleToFixed (matrix[2][2])}}};
 
-      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
-			       0, 0);
+      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture,
+                               scale_down ? FilterBest : FilterNearest, 0, 0);
       XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
 
       if (img->mask_picture)
         {
           XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture,
-                                   FilterBest, 0, 0);
+                                   scale_down ? FilterBest : FilterNearest, 0, 0);
           XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture,
                                       &tmat);
         }
-- 
2.26.1

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> I've attached an updated version of this patch and a screenshot
> showing the result with a couple of levels of scaling.

That looks fine to me.

> Annoyingly we now use Cairo by default which is unaffected by this
> change, so only plain X users will see this behaviour.

Oh, I didn't know that...  so this would make the plain X users (a very
small minority now, I guess) have different "embiggening" scaling than
all the rest?  There's no way to make Cairo not do the ultra-smoothing
that it does now? 

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Alan Third]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Mon, Aug 03, 2020 at 08:10:28AM +0200, Lars Ingebrigtsen wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > Annoyingly we now use Cairo by default which is unaffected by this
> > change, so only plain X users will see this behaviour.
> 
> Oh, I didn't know that...  so this would make the plain X users (a very
> small minority now, I guess) have different "embiggening" scaling than
> all the rest?  There's no way to make Cairo not do the ultra-smoothing
> that it does now? 

It should be possible, I just don't know how. I've never done any
Cairo work before...

Turned out to be pretty simple. Patch attached.
-- 
Alan Third

[-- Attachment #2: v2-0001-Don-t-smooth-images-when-scaling-up-bug-38394.patch --]
[-- Type: text/plain, Size: 2882 bytes --]

From 6d04d11b8fbcd14bdd1f0c173169033c1bc0d422 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 2 Aug 2020 20:43:56 +0100
Subject: [PATCH v2] Don't smooth images when scaling up (bug#38394)

* src/image.c (image_set_transform [HAVE_XRENDER]): Use different filter
when scaling up vs scaling down.
---
 src/image.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/image.c b/src/image.c
index e7e0a93313..a6b24e6652 100644
--- a/src/image.c
+++ b/src/image.c
@@ -259,6 +259,8 @@ cr_put_image_to_cr_data (struct image *img)
 	  cairo_matrix_t matrix;
 	  cairo_pattern_get_matrix (img->cr_data, &matrix);
 	  cairo_pattern_set_matrix (pattern, &matrix);
+          cairo_pattern_set_filter
+            (pattern, cairo_pattern_get_filter (img->cr_data));
 	  cairo_pattern_destroy (img->cr_data);
 	}
       cairo_surface_destroy (surface);
@@ -2114,6 +2116,15 @@ image_set_transform (struct frame *f, struct image *img)
   double rotation = 0.0;
   compute_image_rotation (img, &rotation);
 
+# if defined HAVE_XRENDER
+  /* We want scale up operations to use a nearest neighbour filter to
+     show real pixels instead of munging them, but scale down
+     operations to use a blended filter, to avoid aliasing and the like.
+
+     TODO: implement for NS and Windows.  */
+  bool scale_down = (width < img->width) || (height < img->height);
+# endif
+
   /* Perform scale transformation.  */
 
   matrix3x3 matrix
@@ -2230,6 +2241,8 @@ image_set_transform (struct frame *f, struct image *img)
 			      matrix[1][1], matrix[2][0], matrix[2][1]};
   cairo_pattern_t *pattern = cairo_pattern_create_rgb (0, 0, 0);
   cairo_pattern_set_matrix (pattern, &cr_matrix);
+  cairo_pattern_set_filter (pattern, scale_down
+                            ? CAIRO_FILTER_BEST : CAIRO_FILTER_NEAREST);
   /* Dummy solid color pattern just to record pattern matrix.  */
   img->cr_data = pattern;
 # elif defined (HAVE_XRENDER)
@@ -2246,14 +2259,14 @@ image_set_transform (struct frame *f, struct image *img)
              XDoubleToFixed (matrix[1][2]),
              XDoubleToFixed (matrix[2][2])}}};
 
-      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture, FilterBest,
-			       0, 0);
+      XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->picture,
+                               scale_down ? FilterBest : FilterNearest, 0, 0);
       XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->picture, &tmat);
 
       if (img->mask_picture)
         {
           XRenderSetPictureFilter (FRAME_X_DISPLAY (f), img->mask_picture,
-                                   FilterBest, 0, 0);
+                                   scale_down ? FilterBest : FilterNearest, 0, 0);
           XRenderSetPictureTransform (FRAME_X_DISPLAY (f), img->mask_picture,
                                       &tmat);
         }
-- 
2.26.1

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Robert Pluim]

>>>>> On Mon, 3 Aug 2020 10:10:06 +0100, Alan Third <alan@idiocy.org> said:

    Alan> diff --git a/src/image.c b/src/image.c
    Alan> index e7e0a93313..a6b24e6652 100644
    Alan> --- a/src/image.c
    Alan> +++ b/src/image.c
    Alan> @@ -259,6 +259,8 @@ cr_put_image_to_cr_data (struct image *img)
    Alan>  	  cairo_matrix_t matrix;
    Alan>  	  cairo_pattern_get_matrix (img->cr_data, &matrix);
    Alan>  	  cairo_pattern_set_matrix (pattern, &matrix);
    Alan> +          cairo_pattern_set_filter
    Alan> +            (pattern, cairo_pattern_get_filter (img->cr_data));
    Alan>  	  cairo_pattern_destroy (img->cr_data);
    Alan>  	}
    Alan>        cairo_surface_destroy (surface);
    Alan> @@ -2114,6 +2116,15 @@ image_set_transform (struct frame *f, struct image *img)
    Alan>    double rotation = 0.0;
    Alan>    compute_image_rotation (img, &rotation);
 
    Alan> +# if defined HAVE_XRENDER
    Alan> +  /* We want scale up operations to use a nearest neighbour filter to
    Alan> +     show real pixels instead of munging them, but scale down
    Alan> +     operations to use a blended filter, to avoid aliasing and the like.
    Alan> +
    Alan> +     TODO: implement for NS and Windows.  */
    Alan> +  bool scale_down = (width < img->width) || (height < img->height);
    Alan> +# endif
    Alan> +
    Alan>    /* Perform scale transformation.  */
 
    Alan>    matrix3x3 matrix
    Alan> @@ -2230,6 +2241,8 @@ image_set_transform (struct frame *f, struct image *img)
    Alan>  			      matrix[1][1], matrix[2][0], matrix[2][1]};
    Alan>    cairo_pattern_t *pattern = cairo_pattern_create_rgb (0, 0, 0);
    Alan>    cairo_pattern_set_matrix (pattern, &cr_matrix);
    Alan> +  cairo_pattern_set_filter (pattern, scale_down

Isnʼt scale_down only defined if HAVE_XRENDER? This usage is under
USE_CAIRO

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Alan Third]

On Mon, Aug 03, 2020 at 04:04:56PM +0200, Robert Pluim wrote:
> 
> Isnʼt scale_down only defined if HAVE_XRENDER? This usage is under
> USE_CAIRO

I'm not sure if you can have Cairo without XRender at the moment, but
I'll make it explicit.

Thanks!
-- 
Alan Third

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Robert Pluim]

>>>>> On Mon, 3 Aug 2020 16:13:50 +0100, Alan Third <alan@idiocy.org> said:

    Alan> On Mon, Aug 03, 2020 at 04:04:56PM +0200, Robert Pluim wrote:
    >> 
    >> Isnʼt scale_down only defined if HAVE_XRENDER? This usage is under
    >> USE_CAIRO

    Alan> I'm not sure if you can have Cairo without XRender at the moment, but
    Alan> I'll make it explicit.

I donʼt know that either, I only ran the code through my mental
compiler :-)

Robert

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> It should be possible, I just don't know how. I've never done any
> Cairo work before...
>
> Turned out to be pretty simple. Patch attached.

Then I think this should be applied to Emacs 28, and we'll get some
feedback on whether people prefer it this way or not.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Alan Third]

On Tue, Aug 04, 2020 at 10:52:47AM +0200, Lars Ingebrigtsen wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > It should be possible, I just don't know how. I've never done any
> > Cairo work before...
> >
> > Turned out to be pretty simple. Patch attached.
> 
> Then I think this should be applied to Emacs 28, and we'll get some
> feedback on whether people prefer it this way or not.

I worked out how to do it under NS too, so I've added that and pushed
to master.
-- 
Alan Third

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Lars Ingebrigtsen]

Alan Third <alan@idiocy.org> writes:

> I worked out how to do it under NS too, so I've added that and pushed
> to master.

Great!

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[David Ponce]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

Hello,

With this patch applied (https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=519a93e067f459ceddb57573261a52118086b73d), I noticed that small icon images look bad with image defaut attributes, but look as expected when I add the attribute :scale 1 (see the attached screenshots). The issue is that by default on an HiDPI screen, images are scaled up (in my configuration the default scale is 1.2).

I don't know if there is a way to force the smoothing behavior, if not, maybe an image attribute could be considered ?

Thanks!

[-- Attachment #2: Screenshot_default-scale1.2.png --]
[-- Type: image/png, Size: 974 bytes --]

[-- Attachment #3: Screenshot_scale1.png --]
[-- Type: image/png, Size: 937 bytes --]

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Alan Third]

On Fri, Aug 14, 2020 at 08:03:04AM +0200, David Ponce wrote:
> Hello,
> 
> With this patch applied
> (https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=519a93e067f459ceddb57573261a52118086b73d),
> I noticed that small icon images look bad with image defaut
> attributes, but look as expected when I add the attribute :scale 1
> (see the attached screenshots). The issue is that by default on an
> HiDPI screen, images are scaled up (in my configuration the default
> scale is 1.2).
> 
> I don't know if there is a way to force the smoothing behavior, if
> not, maybe an image attribute could be considered ?

Is this on X?

Maybe we should only go to nearest neighbour when the scaling is >= 2?
Or greater than the scale factor? Hmm, I'm not sure what's best here.
-- 
Alan Third

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[David Ponce]

On 14/08/2020 22:20, Alan Third wrote:
> On Fri, Aug 14, 2020 at 08:03:04AM +0200, David Ponce wrote:
>> Hello,
>>
>> With this patch applied
>> (https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=519a93e067f459ceddb57573261a52118086b73d),
>> I noticed that small icon images look bad with image defaut
>> attributes, but look as expected when I add the attribute :scale 1
>> (see the attached screenshots). The issue is that by default on an
>> HiDPI screen, images are scaled up (in my configuration the default
>> scale is 1.2).
>>
>> I don't know if there is a way to force the smoothing behavior, if
>> not, maybe an image attribute could be considered ?
> 	
> Is this on X?

Yes, here are the Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS
LIBSYSTEMD JSON PDUMPER LCMS2

> 
> Maybe we should only go to nearest neighbour when the scaling is >= 2?
> Or greater than the scale factor? Hmm, I'm not sure what's best here.
> 

Not sure either.
Maybe an option could define the min scale to go to nearest neighbour?
By default (nil?) it could be the scale factor?

An image attribute could make sense too, similar to :scale, but for smoothing.

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[Alan Third]

On Fri, Aug 14, 2020 at 11:14:22PM +0200, David Ponce wrote:
> On 14/08/2020 22:20, Alan Third wrote:
> > Maybe we should only go to nearest neighbour when the scaling is >= 2?
> > Or greater than the scale factor? Hmm, I'm not sure what's best here.
> > 
> 
> Not sure either.
> Maybe an option could define the min scale to go to nearest neighbour?
> By default (nil?) it could be the scale factor?
> 
> An image attribute could make sense too, similar to :scale, but for smoothing.

Can you try replacing this line (2125):

    bool scale_down = (width < img->width) || (height < img->height);

with

    bool scale_down = (double)width/img->width < 1.2;

and have a play about with the number and see how it looks?
-- 
Alan Third

bug#38394: Fwd: Use different image filtering when zooming in vs zooming out

[David Ponce]

On 15/08/2020 01:10, Alan Third wrote:
> On Fri, Aug 14, 2020 at 11:14:22PM +0200, David Ponce wrote:
>> On 14/08/2020 22:20, Alan Third wrote:
>>> Maybe we should only go to nearest neighbour when the scaling is >= 2?
>>> Or greater than the scale factor? Hmm, I'm not sure what's best here.
>>>
>>
>> Not sure either.
>> Maybe an option could define the min scale to go to nearest neighbour?
>> By default (nil?) it could be the scale factor?
>>
>> An image attribute could make sense too, similar to :scale, but for smoothing.
> 
> Can you try replacing this line (2125):
> 
>      bool scale_down = (width < img->width) || (height < img->height);
> 
> with
> 
>      bool scale_down = (double)width/img->width < 1.2;
> 
> and have a play about with the number and see how it looks?
> 

I did try your change. With value 1.2 (the default scale factor on my configuration) images look smooth.
They don't, as soon as I change the image scaling to be greater than 1.2.

IMO, regardless of the scaling it is important to have a way to control how some kind of images will look, like UI elements which must always look smoothed.
So, an option to control this is important in such cases, regardless of the scaling factor.
It should be possible to use small icons scaled by 1.5 or 2 for example, for UI elements.
And such UI elements should still look good, even if the default scaling factor is less than 1.5.

Thanks!