bug#37932: [PATCH] Support hidpi fringes and images with Cairo

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

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

Here is a patch that takes a slightly different approach than my
previous one and fixes both fringe bitmaps and general image
(including "widgets": checkboxes, arrows, etc) rendering in hidpi
screens. I've attached a number of before/after screenshots.

I added two new fields `scale_x` and `scale_y` to the frame struct. I
believe going through the rif was overkilling in terms of indirections
and in terms of legibility, specially if I wanted to implement
independent scale factors for x and y in order to honour the previous
implementation. Now it's simply f->scale_x and f->scale_y. These
fields are initialized when the frame is created. They are floating
point numbers, truncation or rounding is done after scaling.

To be rigorous, these fields should belong to the terminal or to the
display_info, because they're common to all frames in the terminal
(indeed, they are closely related to resx and resy). But display_info
structs are backend specific and adding macros or inlines to frame.h
that reference them would have required including TERM_HEADER in many
many places. Also, having them at the frame level is handy since scale
factors are often, if not always, referenced in a context where a
frame is at scope.

The strategy is simple: scale fringe bitmaps and images geometries
beforehand, so as to allow geometry calculations, but postpone actual
scaling of the contents until the moment of their actual rendering to
screen. The image module has a lot of image loading functions. In
order to reduce the risk of regressions, I decided to adjust width and
height at the end of each function, immediately before returning.

Even if I've implemented this only for cairo (both with and without
gtk), adding support for x without cairo and for w32 shouldn't be
difficult at all. The ns backend simply sets both scale factors to 1,
thus disabling scaling. Other unsupported backends now should at least
carry better geometry computations, although contents won't be
actually scaled.

This doesn't support dynamic changes of dpi or scale factor. For two
or three days (and their nights) I strived to get that working. I was
able to catch xrandr resolution-change events, I was able to catch
scale-change gtk notifications, but the assumption that everything is
set just once at the beginning is firmly entrenched in the code. In
particular, rescaling fonts on the fly turned out to be an
insurmountable problem. I tried many, many things: clearing font
caches, glyph caches, matrix caches, resetting the frame font
parameter, remapping the default face (a la C-x C-+). I finally got it
working but only for the default face, I could have gone and remapped
also modeline faces, etc. I don't think such hacks are worth the
price.

In general, what people is doing to get x working with two monitors at
different dpis is to set a common gtk scale factor that is good for
the highest resolution screen (often = 2) and then scale the lowest
resolution screen down using xrandr. This is an old hack now
"officially" implemented in Ubuntu. So, if you launched  an emacs
frame in a hidpi laptop and then plugged an external monitor with the
same or lower resolution, there won't be any problem. Otherwise, if
your emacs frame started in the lowest resolution screen, then it
should be scaled up, so the only way to go is to close it and open a
frame afresh. Sorry for that.

So, all in all, this fixes hidpi problems and should also work in the
usual multi-monitor multi-dpi scenario. For a more comprehensive
solution I see no other way than writing a modern gtk backend, which
is also the way to move towards wayland.

Best regards
--
Carlos

[-- Attachment #2: tetris-after.png --]
[-- Type: image/png, Size: 69224 bytes --]

[-- Attachment #3: widgets-after.png --]
[-- Type: image/png, Size: 294625 bytes --]

[-- Attachment #4: fringe-after.png --]
[-- Type: image/png, Size: 297966 bytes --]

[-- Attachment #5: tetris-before.png --]
[-- Type: image/png, Size: 51011 bytes --]

[-- Attachment #6: widgets-before.png --]
[-- Type: image/png, Size: 250735 bytes --]

[-- Attachment #7: fringe-before.png --]
[-- Type: image/png, Size: 298590 bytes --]

[-- Attachment #8: 0001-Support-hidpi-screens-with-cairo.patch --]
[-- Type: text/x-patch, Size: 10801 bytes --]

From 479215d0ee6d05a36faa01dff2116a44cf729f30 Mon Sep 17 00:00:00 2001
From: Carlos Pita <carlosjosepita@gmail.com>
Date: Fri, 25 Oct 2019 22:04:11 -0300
Subject: [PATCH] Support hidpi screens with cairo

Add hidpi support for fringe bitmaps and images. Although the
framework is general, this commit only implements support for
cairo.

* src/frame.h (struct frame): Add fields `scale_x' and `scale_y'.
(FRAME_LEFT_FRINGE_WIDTH, FRAME_RIGHT_FRINGE_WIDTH): Scale widths.

* src/fringe.c (draw_fringe_bitmap_1): Scale bitmap width and height.

* src/image.c (lookup_image, xbm_load, xpm_load, pbm_load)
(png_load_body, jpeg_load_body, tiff_load, gif_load)
(imagemagick_load, svg_load, gs_load): Scale images width and height.

* src/nsfns.m (Fx_create_frame): Initialize scale factors to 1.

* src/w32fns.c (Fx_create_frame): Initialize scale factors using a
base of 96dpi.

* src/w32term.c (w32_get_scale_factor): Remove function.
(w32_draw_underwave): Use new `scale_x' and `scale_y' fields.

* src/window.h (WINDOW_LEFT_FRINGE_WIDTH, WINDOW_RIGHT_FRINGE_WIDTH):
Scale widths.

* src/xfns.c (Fx_create_frame): Initialize scale factors using a base
of 96dpi.

* src/xterm.c (x_cr_draw_image): Set surface scale before drawing.
(x_get_scale_factor): Remove function.
(x_draw_underwave): Use new `scale_x' and `scale_y' fields.
---
 src/frame.h   |  7 +++++--
 src/fringe.c  |  4 ++--
 src/image.c   | 40 ++++++++++++++++++++++++++++++++++++++++
 src/nsfns.m   |  2 ++
 src/w32fns.c  |  3 +++
 src/w32term.c | 19 +------------------
 src/window.h  | 12 ++++++------
 src/xfns.c    |  3 +++
 src/xterm.c   | 25 +++----------------------
 9 files changed, 65 insertions(+), 50 deletions(-)

diff --git a/src/frame.h b/src/frame.h
index f408f12394..aa3685c213 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -636,6 +636,9 @@ #define EMACS_FRAME_H
   unsigned long background_pixel;
   unsigned long foreground_pixel;
 
+  /* Factors used to scale pixel geometries.  */
+  double scale_x, scale_y;
+
 #ifdef NS_IMPL_COCOA
   /* NSAppearance theme used on this frame.  */
   enum ns_appearance_type ns_appearance;
@@ -1415,12 +1418,12 @@ FRAME_FRINGE_COLS (struct frame *f)
 INLINE int
 FRAME_LEFT_FRINGE_WIDTH (struct frame *f)
 {
-  return frame_dimension (f->left_fringe_width);
+  return frame_dimension (f->left_fringe_width) * f->scale_x;
 }
 INLINE int
 FRAME_RIGHT_FRINGE_WIDTH (struct frame *f)
 {
-  return frame_dimension (f->right_fringe_width);
+  return frame_dimension (f->right_fringe_width) * f->scale_x;
 }
 
 /* Total width of fringes in pixels.  */
diff --git a/src/fringe.c b/src/fringe.c
index d878d929cc..807c7803c8 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -602,9 +602,9 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
 
   p.which = which;
   p.bits = fb->bits;
-  p.wd = fb->width;
+  p.wd = fb->width * f->scale_x;
 
-  p.h = fb->height;
+  p.h = fb->height * f->scale_y;
   p.dh = (period > 0 ? (p.y % period) : 0);
   p.h -= p.dh;
 
diff --git a/src/image.c b/src/image.c
index 08e420837a..1a7324b406 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2304,6 +2304,8 @@ lookup_image (struct frame *f, Lisp_Object spec)
 	  value = image_spec_value (spec, QCheight, NULL);
 	  img->height = (FIXNUMP (value)
 			 ? XFIXNAT (value) : DEFAULT_IMAGE_HEIGHT);
+          img->width *= f->scale_x;
+          img->height *= f->scale_y;
 	}
       else
 	{
@@ -3792,6 +3794,9 @@ xbm_load (struct frame *f, struct image *img)
 	}
     }
 
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return success_p;
 }
 
@@ -4472,6 +4477,10 @@ xpm_load (struct frame *f, struct image *img)
   xpm_free_color_cache ();
 #endif
   SAFE_FREE ();
+
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return rc == XpmSuccess;
 }
 
@@ -4951,6 +4960,9 @@ xpm_load (struct frame *f,
 				  SSDATA (data) + SBYTES (data));
     }
 
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return success_p;
 }
 
@@ -6168,6 +6180,10 @@ pbm_load (struct frame *f, struct image *img)
      img->height = height; */
 
   xfree (contents);
+
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return 1;
 }
 
@@ -6810,6 +6826,9 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
       image_put_x_image (f, img, mask_img, 1);
     }
 
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return 1;
 }
 
@@ -7383,6 +7402,10 @@ jpeg_load_body (struct frame *f, struct image *img,
   /* Put ximg into the image.  */
   image_put_x_image (f, img, ximg, 0);
   SAFE_FREE ();
+
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return 1;
 }
 
@@ -7831,6 +7854,10 @@ tiff_load (struct frame *f, struct image *img)
   image_put_x_image (f, img, ximg, 0);
 
   xfree (buf);
+
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return 1;
 }
 
@@ -8428,6 +8455,9 @@ gif_load (struct frame *f, struct image *img)
   /* Put ximg into the image.  */
   image_put_x_image (f, img, ximg, 0);
 
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return 1;
 }
 
@@ -9214,6 +9244,9 @@ imagemagick_load (struct frame *f, struct image *img)
                                           SBYTES (data), NULL);
     }
 
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return success_p;
 }
 
@@ -9538,6 +9571,9 @@ svg_load (struct frame *f, struct image *img)
 				   : SSDATA (original_filename)));
     }
 
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return success_p;
 }
 
@@ -9886,6 +9922,10 @@ gs_load (struct frame *f, struct image *img)
 			  make_fixnum (img->height),
 			  window_and_pixmap_id,
 			  pixel_colors);
+
+  img->width *= f->scale_x;
+  img->height *= f->scale_y;
+
   return PROCESSP (img->lisp_data);
 }
 
diff --git a/src/nsfns.m b/src/nsfns.m
index 184fd71678..028b0eaf13 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -1121,6 +1121,8 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side.
   else
       f = make_frame (1);
 
+  f->scale_x = f->scale_y = 1;
+
   XSETFRAME (frame, f);
 
   f->terminal = dpyinfo->terminal;
diff --git a/src/w32fns.c b/src/w32fns.c
index 4ef075f715..3f827be89c 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -5866,6 +5866,9 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
   else
     f = make_frame (true);
 
+  f->scale_x = dpyinfo->resx / 96;
+  f->scale_y = dpyinfo->resy / 96;
+
   XSETFRAME (frame, f);
 
   parent_frame = gui_display_get_arg (dpyinfo, parameters, Qparent_frame,
diff --git a/src/w32term.c b/src/w32term.c
index 888b7368d2..1d746500ee 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -304,22 +304,6 @@ w32_restore_glyph_string_clip (struct glyph_string *s)
     }
 }
 
-static void
-w32_get_scale_factor(struct w32_display_info *dpyinfo, int *scale_x, int *scale_y)
-{
-  const int base_res = 96;
-
-  *scale_x = *scale_y = 1;
-
-  if (dpyinfo)
-    {
-      if (dpyinfo->resx > base_res)
-	*scale_x = floor (dpyinfo->resx / base_res);
-      if (dpyinfo->resy > base_res)
-	*scale_y = floor (dpyinfo->resy / base_res);
-    }
-}
-
 /*
    Draw a wavy line under S. The wave fills wave_height pixels from y0.
 
@@ -336,8 +320,7 @@ w32_draw_underwave (struct glyph_string *s, COLORREF color)
 {
   struct w32_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
 
-  int scale_x, scale_y;
-  w32_get_scale_factor (dpyinfo, &scale_x, &scale_y);
+  double scale_x = s->f->scale_x, scale_y = s->f->scale_y;
 
   int wave_height = 3 * scale_y, wave_length = 2 * scale_x, thickness = scale_y;
   int dx, dy, x0, y0, width, x1, y1, x2, y2, odd, xmax;
diff --git a/src/window.h b/src/window.h
index 71946a5695..07e24551f0 100644
--- a/src/window.h
+++ b/src/window.h
@@ -828,14 +828,14 @@ #define WINDOW_MARGINS_WIDTH(W)			\
    + WINDOW_RIGHT_MARGIN_WIDTH (W))
 
 /* Pixel-widths of fringes.  */
-#define WINDOW_LEFT_FRINGE_WIDTH(W)			\
-  (W->left_fringe_width >= 0				\
-   ? W->left_fringe_width				\
+#define WINDOW_LEFT_FRINGE_WIDTH(W)				\
+  (W->left_fringe_width >= 0					\
+   ? (W->left_fringe_width * WINDOW_XFRAME (W)->scale_x)	\
    : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
 
-#define WINDOW_RIGHT_FRINGE_WIDTH(W)			\
-  (W->right_fringe_width >= 0				\
-   ? W->right_fringe_width				\
+#define WINDOW_RIGHT_FRINGE_WIDTH(W)				\
+  (W->right_fringe_width >= 0					\
+   ? (W->right_fringe_width * WINDOW_XFRAME (W)->scale_x)	\
    : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
 
 #define WINDOW_FRINGES_WIDTH(W)		\
diff --git a/src/xfns.c b/src/xfns.c
index 20e63a2650..3970fe3dda 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -3736,6 +3736,9 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
   else
     f = make_frame (true);
 
+  f->scale_x = dpyinfo->resx / 96;
+  f->scale_y = dpyinfo->resy / 96;
+
   parent_frame = gui_display_get_arg (dpyinfo,
                                       parms,
                                       Qparent_frame,
diff --git a/src/xterm.c b/src/xterm.c
index 05d6a214dd..c34670aba2 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -635,6 +635,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 
   cairo_surface_t *surface;
   cairo_pattern_get_surface (image, &surface);
+  cairo_surface_set_device_scale (surface, 1. / f->scale_x, 1. / f->scale_y);
   cairo_format_t format = cairo_image_surface_get_format (surface);
   if (format != CAIRO_FORMAT_A8 && format != CAIRO_FORMAT_A1)
     {
@@ -3663,23 +3664,6 @@ x_draw_stretch_glyph_string (struct glyph_string *s)
   s->background_filled_p = true;
 }
 
-static void
-x_get_scale_factor(Display *disp, int *scale_x, int *scale_y)
-{
-  const int base_res = 96;
-  struct x_display_info * dpyinfo = x_display_info_for_display (disp);
-
-  *scale_x = *scale_y = 1;
-
-  if (dpyinfo)
-    {
-      if (dpyinfo->resx > base_res)
-	*scale_x = floor (dpyinfo->resx / base_res);
-      if (dpyinfo->resy > base_res)
-	*scale_y = floor (dpyinfo->resy / base_res);
-    }
-}
-
 /*
    Draw a wavy line under S. The wave fills wave_height pixels from y0.
 
@@ -3693,12 +3677,8 @@ x_get_scale_factor(Display *disp, int *scale_x, int *scale_y)
 static void
 x_draw_underwave (struct glyph_string *s)
 {
-  Display *display = FRAME_X_DISPLAY (s->f);
-
   /* Adjust for scale/HiDPI.  */
-  int scale_x, scale_y;
-
-  x_get_scale_factor (display, &scale_x, &scale_y);
+  double scale_x = s->f->scale_x, scale_y = s->f->scale_y;
 
   int wave_height = 3 * scale_y, wave_length = 2 * scale_x;
 
@@ -3709,6 +3689,7 @@ x_draw_underwave (struct glyph_string *s)
   int dx, dy, x0, y0, width, x1, y1, x2, y2, xmax, thickness = scale_y;;
   bool odd;
   XRectangle wave_clip, string_clip, final_clip;
+  Display *display = FRAME_X_DISPLAY (s->f);
 
   dx = wave_length;
   dy = wave_height - 1;
-- 
2.20.1

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

The patch is also online at
https://github.com/memeplex/emacs/commit/4ddc051d18d710d1af98ee5e51e9728b77d13191

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

I realized there is a problem with images. According to create-image:

Optional PROPS are additional image attributes to assign to the image,
like, e.g. `:mask MASK'.  If the property `:scale' is not given and the
display has a high resolution (more exactly, when the average width of a
character in the default font is more than 10 pixels), the image is
automatically scaled up in proportion to the default font.

So even if my patch correctly scales some images, it's scaling others
twice. Probably not all images pass through create-image.

I'll give a look at this tomorrow.

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

Ok, the problem is that widget-image-insert (in wid-edit.el) is
directly inserting the image with insert-image, without creating the
spec with create-image first, so the spec doesn't get the :scale
property. Similarly, gamegrid.el is creating its own, unscaled, image
spec.

IMO there is something smelly in using the :scale attribute like
create-image does, scaling to compensate hidpi is a very different
concern than "application level" scaling.

Anyway, do you think that:

1. I should fix those two modules to use create-image or at least add :scale, or
2. put a mark to unscaled images so that they are always scaled at the
end in x_cr_draw_image, or
3. disable this auto :scale feature when the new low-level scaling
mechanism is on

?

I think I prefer 3.

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

Here is an attempt to fix the above issues:

https://github.com/memeplex/emacs/commit/473beb25dd947c34392708ca40f1b3bf80613007

* It uses the scaling mechanism already present in image.c (notice
that imagemagick does its own scaling), so it works even without cairo
(except for fringes, of course).

* It removes the auto :scale property. I think this is the right thing
to do, applications that want to use the :scale parameter to transform
their images shouldn't lose the hidpi scaling nor explicitly call
image-compute-scaling-factor.

* The scale factor is slightly different than the one computed by
image-compute-scaling-factor, but anyway it's the scale factor that
was already computed by x/w32_scale_factor. Moreover, it has separated
x and y values.

A bit surprisingly the patch results in a code reduction of ~20LOC.
And there are image-compute-scaling-factor and image-scaling-factor
still remaining in image.el. The only user inside emacs is shr.el.

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Sat, 26 Oct 2019 05:05:52 -0300
> 
> Here is an attempt to fix the above issues:
> 
> https://github.com/memeplex/emacs/commit/473beb25dd947c34392708ca40f1b3bf80613007

Thanks, but please continue posting the patches to the bug tracker, so
that the record of these discussions is complete.  It would also make
it easier for people to review the patches in Emacs while having
instant and easy access to the current code.

> * It removes the auto :scale property. I think this is the right thing
> to do, applications that want to use the :scale parameter to transform
> their images shouldn't lose the hidpi scaling nor explicitly call
> image-compute-scaling-factor.

I don't think I'm qualified to make an intelligent decision on this by
myself (and you just asked the question not long ago, and proceeded to
coding without giving anyone any time to answer it :-().  I don't know
enough about this to make such a decision.  maybe Robert, Alan, and
others would chime in and voice their opinions.  failing that, please
tell more about the pro's and con's here, so that we could see what
kind of trade-offs are we talking about.

Thanks.

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

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

On Sat, Oct 26, 2019, 6:14 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Carlos Pita <carlosjosepita@gmail.com>
> > Date: Sat, 26 Oct 2019 05:05:52 -0300
> >
> > Here is an attempt to fix the above issues:
> >
> >
> https://github.com/memeplex/emacs/commit/473beb25dd947c34392708ca40f1b3bf80613007
>
> Thanks, but please continue posting the patches to the bug tracker, so
>

Ok, no problem.

myself (and you just asked the question not long ago, and proceeded to
> coding without giving anyone any time to answer it :-().


It's just a proposal, I don't mind coding it again if there are good reason
to do so. I've rewritten this so many times now :)

>
>

[-- Attachment #2: Type: text/html, Size: 1668 bytes --]

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Robert Pluim]

>>>>> On Sat, 26 Oct 2019 12:14:19 +0300, Eli Zaretskii <eliz@gnu.org> said:
    >> * It removes the auto :scale property. I think this is the right thing
    >> to do, applications that want to use the :scale parameter to transform
    >> their images shouldn't lose the hidpi scaling nor explicitly call
    >> image-compute-scaling-factor.

If I read the patch correctly, you now auto-scale inside image.c,
which shouldn't change anything when not using :scale. What's the
effect when code passes in eg :scale 2? Autoscaling + frame scaling,
so for a frame where scale == 2 we get * 4? I guess that would be
sensible, but weʼd need to explain it clearly. [1]

    Eli> I don't think I'm qualified to make an intelligent decision on this by
    Eli> myself (and you just asked the question not long ago, and proceeded to
    Eli> coding without giving anyone any time to answer it :-().  I don't know
    Eli> enough about this to make such a decision.  maybe Robert, Alan, and
    Eli> others would chime in and voice their opinions.  failing that, please
    Eli> tell more about the pro's and con's here, so that we could see what
    Eli> kind of trade-offs are we talking about.

I think Lars did the autoscaling work. If things continue to work as
before for the normal case, and we explain the interaction between
:scale and frame scale I have no objections.

Robert

Footnotes:
[1]  And it would get people out of the "1 physical display pixel == 1
     logical display pixel" mindset that needed to be fixed in so many
     places in the X backend.

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

> If I read the patch correctly, you now auto-scale inside image.c,
> which shouldn't change anything when not using :scale. What's the

Right, AFAICS scaling of images was always done inside image.c, that's
where transformation routines are coded. The difference is more in the
usage of the :scale parameter. Previously there were two cases:

1. The caller doesn't pass a :scale in the spec. Then a :scale
property compensating for the dpi of the screen is automatically added
to the spec if the image-scaling-factor customization option is set to
'auto.  The computed scale factor assumes a representative character
to be 10px width.

2. The caller passes a :scale in the spec. Then the image is scaled
using this number and no extra dpi-adapting-scaling is done.

I see two shortcomings with this approach:

a. One I've already mentioned above: scaling required by the
application shouldn't interfere with scaling done to fit screen
resolution, which is a concern at a very different level.

b. Moreover, I'm not sure how compatible is this 10px char width
assumption with the 96dpi base resolution assumed to compute scale
factors in xterm.c and w32term.c. OTOH, the 96dpi assumption is
compatible with what gtk does. Anyway, with my patch each platform is
able to set a different base resolution if that's necessary.

> effect when code passes in eg :scale 2? Autoscaling + frame scaling,
> so for a frame where scale == 2 we get * 4? I guess that would be
> sensible, but weʼd need to explain it clearly. [1]

User code should be mostly unaware of the underlying resolution except
for very specialized cases. Currently, that is reflected in the
default 'auto image-scaling-factor. It's true that, as I commented in
point 1 above, nowadays one can pass a explicit :scale in order to
turn auto-scaling off, although in most cases that would probably
introduce a bug by *inadvertently* turning auto-scaling off. We can
add another property to the spec in order to enforce real pixel size,
or the user can divide the desired scale by the result of
image-compute-scaling-factor (in the same way the xterm backend is
circumventing gtk auto-scaling by applying the inverse scale operation
first). I prefer adding a new spec property to turn-off auto-scaling,
because that way both image-compute-scaling-factor and
image-scaling-factor could be removed from the codebase and also the
inconsistency between 96dpi and 10px criteria wouldn't be a problem
any more.

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

I think I'm going to attempt scaling the fringe bitmaps in a generic
way as first suggested in #37689 [1], so as to get rid of the cairo
dependency. But, even if not a logical dependency, the organization of
that code would depend on the outcome of #37755 [2], which I had
closed in the tracker but now reopened in my mind, as shown by my last
post there :)

---

[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37689
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Stefan Kangas]

Carlos Pita <carlosjosepita@gmail.com> writes:

> I think I'm going to attempt scaling the fringe bitmaps in a generic
> way as first suggested in #37689 [1], so as to get rid of the cairo
> dependency. But, even if not a logical dependency, the organization of
> that code would depend on the outcome of #37755 [2], which I had
> closed in the tracker but now reopened in my mind, as shown by my last
> post there :)
>
> ---
>
> [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37689
> [2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755

(That was 4 years ago.)

Did you make any progress here?

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

> Did you make any progress here?

Sorry, Stefan, there is not much that I'm able to recall about this,
it has been many years since last time I looked at the code.

AFAICR I bumped into a number of corner cases and hard assumptions
about dpi and so I concluded that the pure gtk backend was the way to
go.

Best regards,
Carlos

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Stefan Kangas]

Carlos Pita <carlosjosepita@gmail.com> writes:

> Sorry, Stefan, there is not much that I'm able to recall about this,
> it has been many years since last time I looked at the code.
>
> AFAICR I bumped into a number of corner cases and hard assumptions
> about dpi and so I concluded that the pure gtk backend was the way to
> go.

Thanks for getting back to us.  Do you think some version of your
changes would still be worth installing, or would they need
more work to be considered an improvement?

Could you send your latest changes as a patch?

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Carlos Pita]

I would say that the lengthy discussions in:

- https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37689
- https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37770
- https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755

are my only legacy. Rereading them I found some relevant remarks about
the problems I bumped into at the time.

But that's it. My apologies, I'm checking and I'm not even able to
find the fork where I had been working on this.

Why are you interested in it? Isn't it kind of obsolete now that there
is a pure GTK backend?



On Thu, Jan 11, 2024 at 3:21 PM Stefan Kangas <stefankangas@gmail.com> wrote:
>
> Carlos Pita <carlosjosepita@gmail.com> writes:
>
> > Sorry, Stefan, there is not much that I'm able to recall about this,
> > it has been many years since last time I looked at the code.
> >
> > AFAICR I bumped into a number of corner cases and hard assumptions
> > about dpi and so I concluded that the pure gtk backend was the way to
> > go.
>
> Thanks for getting back to us.  Do you think some version of your
> changes would still be worth installing, or would they need
> more work to be considered an improvement?
>
> Could you send your latest changes as a patch?

bug#37932: [PATCH] Support hidpi fringes and images with Cairo

[Stefan Kangas]

Carlos Pita <carlosjosepita@gmail.com> writes:

> I would say that the lengthy discussions in:
>
> - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37689
> - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37770
> - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755
>
> are my only legacy. Rereading them I found some relevant remarks about
> the problems I bumped into at the time.
>
> But that's it. My apologies, I'm checking and I'm not even able to
> find the fork where I had been working on this.

OK, thanks for looking nevertheless.

> Why are you interested in it? Isn't it kind of obsolete now that there
> is a pure GTK backend?

It hasn't yet been obsoleted in Emacs.  Maybe eventually, but not yet.
Until that happens, we're always happy to take improvements.

I'm closing this bug now.  Thanks for your time and effort.