bug#46491: [feature/pgtk] Unused macro warning in image.c without ImageMagick

bug#46491: [feature/pgtk] Unused macro warning in image.c without ImageMagick

[Basil L. Contovounesios]

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

X-Debbugs-Cc: Yuuki Harano <masm+emacs@masm11.me>
Severity: minor
Tags: patch

The following is the only build warning I currently get on feature/pgtk:

  image.c:110: warning: macro "DONT_CREATE_TRANSFORMED_IMAGEMAGICK_IMAGE"
    is not used [-Wunused-macros]
    110 | # define DONT_CREATE_TRANSFORMED_IMAGEMAGICK_IMAGE

Is the attached patch the right fix?  If so, I can push it after adding
the bug#number to it.

Thanks,

-- 
Basil

In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-02-13 built on tia
Repository revision: 175ebb010477ff0e174fe5145154397f43dee2d2
Repository branch: feature/pgtk
Windowing system distributor 'System Description: Debian GNU/Linux bullseye/sid

Configured using:
 'configure 'CC=ccache gcc' 'CFLAGS=-O2 -march=native' --config-cache
 --prefix=/home/blc/.local --program-suffix=-pgtk
 --enable-checking=structs --with-file-notification=yes --with-pgtk
 --with-xwidgets'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY
PDUMPER PGTK PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XIM
XWIDGETS GTK3 ZLIB

Important settings:
  value of $LANG: en_IE.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-unused-macro-build-warning-in-image.c.patch --]
[-- Type: text/x-diff, Size: 1575 bytes --]

From d60bc973c8ac840e02297931f86ebb05b50b20e6 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sat, 13 Feb 2021 14:51:48 +0000
Subject: [PATCH] Fix unused macro build warning in image.c

* src/image.c
[HAVE_PGTK] (DONT_CREATE_TRANSFORMED_IMAGEMAGICK_IMAGE): Define only
if HAVE_IMAGEMAGICK is also defined, as it is otherwise unused.
(image_set_transform): Reduce nesting of preprocessor conditionals.
---
 src/image.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/image.c b/src/image.c
index cc97ca4d7c..09370fa833 100644
--- a/src/image.c
+++ b/src/image.c
@@ -99,7 +99,7 @@ #define BLUE16_FROM_ULONG(color)	(BLUE_FROM_ULONG (color) * 0x101)
 					      Lisp_Object, unsigned long);
 #endif	/* USE_CAIRO */
 
-#ifdef HAVE_PGTK
+#if defined HAVE_PGTK && defined HAVE_IMAGEMAGICK
 /* On pgtk, we don't want to create scaled image.
  * If we create scaled image on scale=2.0 environment,
  * the created image is half size and Gdk scales it back,
@@ -2307,12 +2307,11 @@ compute_image_rotation (struct image *img, double *rotation)
 static void
 image_set_transform (struct frame *f, struct image *img)
 {
-# ifdef HAVE_IMAGEMAGICK
-#  ifndef DONT_CREATE_TRANSFORMED_IMAGEMAGICK_IMAGE
+# if (defined HAVE_IMAGEMAGICK \
+      && !defined DONT_CREATE_TRANSFORMED_IMAGEMAGICK_IMAGE)
   /* ImageMagick images already have the correct transform.  */
   if (EQ (image_spec_value (img->spec, QCtype, NULL), Qimagemagick))
     return;
-#  endif
 # endif
 
 # if !defined USE_CAIRO && defined HAVE_XRENDER
-- 
2.30.0

bug#46491: [feature/pgtk] Unused macro warning in image.c without ImageMagick

[Alan Third]

On Sat, Feb 13, 2021 at 04:34:22PM +0000, Basil L. Contovounesios wrote:
> X-Debbugs-Cc: Yuuki Harano <masm+emacs@masm11.me>
> Severity: minor
> Tags: patch
> 
> The following is the only build warning I currently get on feature/pgtk:
> 
>   image.c:110: warning: macro "DONT_CREATE_TRANSFORMED_IMAGEMAGICK_IMAGE"
>     is not used [-Wunused-macros]
>     110 | # define DONT_CREATE_TRANSFORMED_IMAGEMAGICK_IMAGE
> 
> Is the attached patch the right fix?  If so, I can push it after adding
> the bug#number to it.

I think your patch looks right, but I'm confused as to why this is in
the PGTK branch at all. The imagemagick transforms are intentionally
excluded from the native transforms, but this disables the imagemagick
transforms completely and, presumably, replaces them with native
transforms.
-- 
Alan Third

bug#46491: [feature/pgtk] Unused macro warning in image.c without ImageMagick

[Yuuki Harano]

On Sat, 13 Feb 2021 16:34:22 +0000,
	"Basil L. Contovounesios" <contovob@tcd.ie> wrote:
> Is the attached patch the right fix?  If so, I can push it after adding
> the bug#number to it.

Thanks, confirmed.  Please push it.

-- 
Yuuki Harano

bug#46491: [feature/pgtk] Unused macro warning in image.c without ImageMagick

[Yuuki Harano]

On Sat, 13 Feb 2021 23:05:45 +0000,
	Alan Third <alan@idiocy.org> wrote:
> I think your patch looks right, but I'm confused as to why this is in
> the PGTK branch at all. The imagemagick transforms are intentionally
> excluded from the native transforms, but this disables the imagemagick
> transforms completely and, presumably, replaces them with native
> transforms.

I wanted to make imagemagick images not blurry in image-mode on
scale x2.0 monitor.

With the exclusion, imagemagick generates scaled image pixel data,
and the image is rendered twice in size, so it is blurry.
Without the exclusion, imagemagick generates original image pixel data,
and the image may not be enlarged (but shrinked), so it may not be blurry.

For other than imagemagick, like png, original image pixel data is generated,
and it is not blurry on scale x2.0 monitor.

So I disabled it.

-- 
Yuuki Harano

bug#46491: [feature/pgtk] Unused macro warning in image.c without ImageMagick

[Alan Third]

On Sun, Feb 14, 2021 at 11:20:19AM +0900, Yuuki Harano wrote:
> 
> On Sat, 13 Feb 2021 23:05:45 +0000,
> 	Alan Third <alan@idiocy.org> wrote:
> > I think your patch looks right, but I'm confused as to why this is in
> > the PGTK branch at all. The imagemagick transforms are intentionally
> > excluded from the native transforms, but this disables the imagemagick
> > transforms completely and, presumably, replaces them with native
> > transforms.
> 
> I wanted to make imagemagick images not blurry in image-mode on
> scale x2.0 monitor.
> 
> With the exclusion, imagemagick generates scaled image pixel data,
> and the image is rendered twice in size, so it is blurry.
> Without the exclusion, imagemagick generates original image pixel data,
> and the image may not be enlarged (but shrinked), so it may not be blurry.
> 
> For other than imagemagick, like png, original image pixel data is generated,
> and it is not blurry on scale x2.0 monitor.

I am aware of this, I see the same thing on NS, but when native
transforms were introduced we decided that when a user asks
imagemagick to modify an image, imagemagick should actually do the
modifications and Emacs will just display the results.

I don't know that it's important, though.
-- 
Alan Third

bug#46491: [feature/pgtk] Unused macro warning in image.c without ImageMagick

[Basil L. Contovounesios]

Yuuki Harano <masm+emacs@masm11.me> writes:

> On Sat, 13 Feb 2021 16:34:22 +0000,
> 	"Basil L. Contovounesios" <contovob@tcd.ie> wrote:
>> Is the attached patch the right fix?  If so, I can push it after adding
>> the bug#number to it.
>
> Thanks, confirmed.  Please push it.

Thanks, done.

Fix unused macro build warning in image.c
c4cc80e1fb 2021-02-14 12:29:25 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c4cc80e1fbbfb50486c6ce360f2d68e96fc45887

-- 
Basil

bug#46491: [feature/pgtk] Unused macro warning in image.c without ImageMagick

[Eli Zaretskii]

> Date: Sun, 14 Feb 2021 10:03:25 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: contovob@tcd.ie, 46491@debbugs.gnu.org
> 
> > I wanted to make imagemagick images not blurry in image-mode on
> > scale x2.0 monitor.
> > 
> > With the exclusion, imagemagick generates scaled image pixel data,
> > and the image is rendered twice in size, so it is blurry.
> > Without the exclusion, imagemagick generates original image pixel data,
> > and the image may not be enlarged (but shrinked), so it may not be blurry.
> > 
> > For other than imagemagick, like png, original image pixel data is generated,
> > and it is not blurry on scale x2.0 monitor.
> 
> I am aware of this, I see the same thing on NS, but when native
> transforms were introduced we decided that when a user asks
> imagemagick to modify an image, imagemagick should actually do the
> modifications and Emacs will just display the results.
> 
> I don't know that it's important, though.

I think it's quite important to have Emacs behave the same on all
platforms.  While we no longer recommend to build Emacs with
ImageMagick, those who do build with it should be able to use its
transforms, and disabling that is not necessarily a good idea.
Instead of disabling ImageMagick transforms, why not tell users to
build Emacs without ImageMagick?