[PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Evgeny Zajcev]

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

Motivation:

It is essential to have functionality where image size adjusts
automatically to the display conditions.  Right now we have `em'
element to specify image size relative to the font size.  However,
this is not enough for the grain control of how the image is displayed.
Because for the same font, but different font sizes, ratio of font
height to font size and ratio of average font width to font size
**differs**.  Making it impossible to have the same image to look the same
for different font sizes of the same font.

Here is an example.  I need an image which occupies 2 chars, but in
the same time its height must not exceed line height:

  (defun my-em-width-ratio ()
    (let ((info (font-info (face-font 'default))))
      ;; avg-width / pixel-size
      (/ (float (aref info 11)) (aref info 2))))

  (defun my-em-height-ratio ()
    (let ((info (font-info (face-font 'default))))
      ;; height / pixel-size
      (/ (float (aref info 3)) (aref info 2))))

  (list 'image :type 'svg :file "file.svg" :scale 1.0 :ascent 'center
         :width (cons (* 2 (my-em-width-ratio)) 'em)
         :max-height (cons (* 1 (my-em-height-ratio)) 'em))

Note that `em' means font size and not the font height, but for some
fonts font size and font height differs.

This works very well.  However, if I execute `M-x text-scale-decrease
RET' or `M-x text-scale-increase RET' image starts looking
differently, not fitting into 2 chars width.  Because font ratios
changes.

Before scaling:
  (my-em-width-ratio)  => 0.5111111111111111
  (my-em-height-ratio) => 1.0666666666666667

After scaling:
  (my-em-width-ratio)  => 0.5135135135135135
  (my-em-height-ratio) => 1.054054054054054

With applied patch and image specified as:

  (list 'image :type 'svg :file "file.svg" :scale 1.0 :ascent 'center
         :width '(2 . cw)
         :max-height '(1 . ch))

Image looks the same and occupies exactly the same amount of
characters for different font sizes of the same font.

Thanks.

-- 
lg

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

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Eli Zaretskii]

> From: Evgeny Zajcev <lg.zevlg@gmail.com>
> Date: Thu, 21 Mar 2024 17:53:09 +0300
> 
> With applied patch and image specified as:
> 
>   (list 'image :type 'svg :file "file.svg" :scale 1.0 :ascent 'center
>          :width '(2 . cw)
>          :max-height '(1 . ch))

ENOPATCH

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Evgeny Zajcev]

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

чт, 21 мар. 2024 г. в 19:57, Eli Zaretskii <eliz@gnu.org>:

> > From: Evgeny Zajcev <lg.zevlg@gmail.com>
> > Date: Thu, 21 Mar 2024 17:53:09 +0300
> >
> > With applied patch and image specified as:
> >
> >   (list 'image :type 'svg :file "file.svg" :scale 1.0 :ascent 'center
> >          :width '(2 . cw)
> >          :max-height '(1 . ch))
>
> ENOPATCH
>

:)) sorry, here it is

-- 
lg

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

[-- Attachment #2: 0001-Add-support-for-ch-and-cw-dimension-specifiers-for-t.patch --]
[-- Type: text/x-patch, Size: 3713 bytes --]

From b13bae3e21c6f8f503e7cbd00003694813acdaca Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Thu, 21 Mar 2024 17:47:29 +0300
Subject: [PATCH] Add support for `ch' and `cw' dimension specifiers for the
 image

---
 doc/lispref/display.texi |  7 +++++--
 src/dispextern.h         |  5 +++++
 src/image.c              | 19 ++++++++++++++++---
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 4dbb4afb20d..73671a21e7f 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5788,8 +5788,11 @@ Image Descriptors
 length in @dfn{ems}@footnote{In typography an em is a distance
 equivalent to the height of the type.  For example when using 12 point
 type 1 em is equal to 12 points.  Its use ensures distances and type
-remain proportional.}.  One em is equivalent to the height of the font
-and @var{value} may be an integer or a float.
+remain proportional.}.  One em is equivalent to the size of the font
+and @var{value} may be an integer or a float.  Also, dimension can be
+specified in @code{(@var{value} . ch)} and @code{(@var{value} . cw)}
+forms, where @code{ch} means height of the canonical character and
+@code{cw} means width of the canonical character.
 
   The following is a list of properties that are meaningful for all
 image types (there are also properties which are meaningful only for
diff --git a/src/dispextern.h b/src/dispextern.h
index 3a4d6095f73..462b370b810 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3176,6 +3176,11 @@ reset_mouse_highlight (Mouse_HLInfo *hlinfo)
   int face_font_size;
   char *face_font_family;
 
+  /* Details of the font used to calculate image size relative to the
+     canonical character size, with `ch' and `cw' specifiers. */
+  int face_font_height;
+  int face_font_width;
+
   /* True if this image has a `transparent' background -- that is, is
      uses an image mask.  The accessor macro for this is
      `IMAGE_BACKGROUND_TRANSPARENT'.  */
diff --git a/src/image.c b/src/image.c
index 9a465f0b180..eb0460759f0 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2557,9 +2557,18 @@ image_get_dimension (struct image *img, Lisp_Object symbol)
 
   if (FIXNATP (value))
     return min (XFIXNAT (value), INT_MAX);
-  if (CONSP (value) && NUMBERP (CAR (value)) && EQ (Qem, CDR (value)))
-    return scale_image_size (img->face_font_size, 1, XFLOATINT (CAR (value)));
-
+  if (CONSP (value) && NUMBERP (CAR (value)))
+    {
+      if (EQ (Qem, CDR (value)))
+        return scale_image_size (img->face_font_size,
+                                 1, XFLOATINT (CAR (value)));
+      if (EQ (Qch, CDR (value)))
+        return scale_image_size (img->face_font_height,
+                                 1, XFLOATINT (CAR (value)));
+      if (EQ (Qcw, CDR (value)))
+        return scale_image_size (img->face_font_width,
+                                 1, XFLOATINT (CAR (value)));
+    }
   return -1;
 }
 
@@ -3383,6 +3392,8 @@ lookup_image (struct frame *f, Lisp_Object spec, int face_id)
       img->face_foreground = foreground;
       img->face_background = background;
       img->face_font_size = font_size;
+      img->face_font_height = face->font->height;
+      img->face_font_width = face->font->average_width;
       img->face_font_family = xmalloc (strlen (font_family) + 1);
       strcpy (img->face_font_family, font_family);
       img->load_failed_p = ! img->type->load_img (f, img);
@@ -12760,6 +12771,8 @@ syms_of_image (void)
   DEFSYM (QCmax_height, ":max-height");
 
   DEFSYM (Qem, "em");
+  DEFSYM (Qch, "ch");
+  DEFSYM (Qcw, "cw");
 
 #ifdef HAVE_NATIVE_TRANSFORMS
   DEFSYM (Qscale, "scale");
-- 
2.25.1

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Eli Zaretskii]

> From: Evgeny Zajcev <lg.zevlg@gmail.com>
> Date: Thu, 21 Mar 2024 22:14:51 +0300
> Cc: emacs-devel@gnu.org
> 
> чт, 21 мар. 2024 г. в 19:57, Eli Zaretskii <eliz@gnu.org>:
> 
>  > From: Evgeny Zajcev <lg.zevlg@gmail.com>
>  > Date: Thu, 21 Mar 2024 17:53:09 +0300
>  > 
>  > With applied patch and image specified as:
>  > 
>  >   (list 'image :type 'svg :file "file.svg" :scale 1.0 :ascent 'center
>  >          :width '(2 . cw)
>  >          :max-height '(1 . ch))
> 
>  ENOPATCH
> 
> :)) sorry, here it is

Thanks.

Alan, could you please take a look and comment on this?

I have a couple of minor stylistic comments below.

> diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
> index 4dbb4afb20d..73671a21e7f 100644
> --- a/doc/lispref/display.texi
> +++ b/doc/lispref/display.texi
> @@ -5788,8 +5788,11 @@ Image Descriptors
>  length in @dfn{ems}@footnote{In typography an em is a distance
>  equivalent to the height of the type.  For example when using 12 point
>  type 1 em is equal to 12 points.  Its use ensures distances and type
> -remain proportional.}.  One em is equivalent to the height of the font
> -and @var{value} may be an integer or a float.
> +remain proportional.}.  One em is equivalent to the size of the font
> +and @var{value} may be an integer or a float.  Also, dimension can be

Here, you changed the description of "em" from "height of the font" to
"size of the font".  Is this intentional, and if so, why it is better
to say "size" here?

> +  /* Details of the font used to calculate image size relative to the
> +     canonical character size, with `ch' and `cw' specifiers. */
                                                               ^^
Please leave two spaces after the last sentence of the comment.

> +  if (CONSP (value) && NUMBERP (CAR (value)))
> +    {
> +      if (EQ (Qem, CDR (value)))
> +        return scale_image_size (img->face_font_size,
> +                                 1, XFLOATINT (CAR (value)));
> +      if (EQ (Qch, CDR (value)))
> +        return scale_image_size (img->face_font_height,
> +                                 1, XFLOATINT (CAR (value)));
> +      if (EQ (Qcw, CDR (value)))
> +        return scale_image_size (img->face_font_width,
> +                                 1, XFLOATINT (CAR (value)));

Minor efficiency comment: it is better to compute CDR(value) just once
and store it in a temporary:

    if (CONSP (value) && NUMBERP (CAR (value)))
      {
        Lisp_Object dim = CDR (value);

        if (EQ (Qem, dim))
          return scale_image_size (img->face_font_size,
                                   1, XFLOATINT (CAR (value)));
        if (EQ (Qch, dim))
          return scale_image_size (img->face_font_height,
                                   1, XFLOATINT (CAR (value)));

etc.  (Optimizing compilers will do this automatically, but an
unoptimized build might become a tad faster.)

Finally, please include a ChangeLog-style commit log message for this
patch; see CONTRIBUTE for how we expect that to be formatted (and you
can use "git log" to see what we do in practice).

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Evgeny Zajcev]

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

чт, 28 мар. 2024 г. в 13:06, Eli Zaretskii <eliz@gnu.org>:

> > From: Evgeny Zajcev <lg.zevlg@gmail.com>
> > Date: Thu, 21 Mar 2024 22:14:51 +0300
> > Cc: emacs-devel@gnu.org
> >
> > чт, 21 мар. 2024 г. в 19:57, Eli Zaretskii <eliz@gnu.org>:
> >
> >  > From: Evgeny Zajcev <lg.zevlg@gmail.com>
> >  > Date: Thu, 21 Mar 2024 17:53:09 +0300
> >  >
> >  > With applied patch and image specified as:
> >  >
> >  >   (list 'image :type 'svg :file "file.svg" :scale 1.0 :ascent 'center
> >  >          :width '(2 . cw)
> >  >          :max-height '(1 . ch))
> >
> >  ENOPATCH
> >
> > :)) sorry, here it is
>
> Thanks.
>
> Alan, could you please take a look and comment on this?
>
> I have a couple of minor stylistic comments below.
>
> > diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
> > index 4dbb4afb20d..73671a21e7f 100644
> > --- a/doc/lispref/display.texi
> > +++ b/doc/lispref/display.texi
> > @@ -5788,8 +5788,11 @@ Image Descriptors
> >  length in @dfn{ems}@footnote{In typography an em is a distance
> >  equivalent to the height of the type.  For example when using 12 point
> >  type 1 em is equal to 12 points.  Its use ensures distances and type
> > -remain proportional.}.  One em is equivalent to the height of the font
> > -and @var{value} may be an integer or a float.
> > +remain proportional.}.  One em is equivalent to the size of the font
> > +and @var{value} may be an integer or a float.  Also, dimension can be
>
> Here, you changed the description of "em" from "height of the font" to
> "size of the font".  Is this intentional, and if so, why it is better
> to say "size" here?
>

Yes, this is intentional, because saying "height of the font" in docs, when
font's pixel size is used in code, is misleading and it took me some time
to understand why image renders smaller then font height if '(1 . em) is
specified as dimension modifier.  That's why I started using coefficient
(calculated with `my-em-height-ratio') to `em' specifier


> > +  /* Details of the font used to calculate image size relative to the
> > +     canonical character size, with `ch' and `cw' specifiers. */
>                                                                ^^
> Please leave two spaces after the last sentence of the comment.
>

noted

> +  if (CONSP (value) && NUMBERP (CAR (value)))
> > +    {
> > +      if (EQ (Qem, CDR (value)))
> > +        return scale_image_size (img->face_font_size,
> > +                                 1, XFLOATINT (CAR (value)));
> > +      if (EQ (Qch, CDR (value)))
> > +        return scale_image_size (img->face_font_height,
> > +                                 1, XFLOATINT (CAR (value)));
> > +      if (EQ (Qcw, CDR (value)))
> > +        return scale_image_size (img->face_font_width,
> > +                                 1, XFLOATINT (CAR (value)));
>
> Minor efficiency comment: it is better to compute CDR(value) just once
> and store it in a temporary:
>
>     if (CONSP (value) && NUMBERP (CAR (value)))
>       {
>         Lisp_Object dim = CDR (value);
>
>         if (EQ (Qem, dim))
>           return scale_image_size (img->face_font_size,
>                                    1, XFLOATINT (CAR (value)));
>         if (EQ (Qch, dim))
>           return scale_image_size (img->face_font_height,
>                                    1, XFLOATINT (CAR (value)));
>
>
Sure, thought about it, but I think the compiler should do such things,
value is not volatile.  I did not know about compilers that don't do basic
optimizations

will do

etc.  (Optimizing compilers will do this automatically, but an
> unoptimized build might become a tad faster.)
>
> Finally, please include a ChangeLog-style commit log message for this
> patch; see CONTRIBUTE for how we expect that to be formatted (and you
> can use "git log" to see what we do in practice).
>

Yeah, will send an update soon.

Thank you

-- 
lg

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

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Evgeny Zajcev]

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

чт, 28 мар. 2024 г. в 13:50, Evgeny Zajcev <lg.zevlg@gmail.com>:

>
> Yeah, will send an update soon.
>
>
Here is an update.  ChangeLog entry is included in the attached file

-- 
lg

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

[-- Attachment #2: 0001-Add-support-for-ch-and-cw-dimension-specifiers-for-t.patch --]
[-- Type: text/x-patch, Size: 3827 bytes --]

From 449e860fdec65e92ebca50e99a21afc9ea9c4206 Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Thu, 21 Mar 2024 17:47:29 +0300
Subject: [PATCH] Add support for `ch' and `cw' dimension specifiers for the
 image

* src/frame.c (image_get_dimension): Handle `ch' and `cw' dimension
specifiers in addition to `em'
---
 doc/lispref/display.texi |  7 +++++--
 src/dispextern.h         |  5 +++++
 src/image.c              | 19 +++++++++++++++++--
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 4dbb4afb20d..73671a21e7f 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5788,8 +5788,11 @@ Image Descriptors
 length in @dfn{ems}@footnote{In typography an em is a distance
 equivalent to the height of the type.  For example when using 12 point
 type 1 em is equal to 12 points.  Its use ensures distances and type
-remain proportional.}.  One em is equivalent to the height of the font
-and @var{value} may be an integer or a float.
+remain proportional.}.  One em is equivalent to the size of the font
+and @var{value} may be an integer or a float.  Also, dimension can be
+specified in @code{(@var{value} . ch)} and @code{(@var{value} . cw)}
+forms, where @code{ch} means height of the canonical character and
+@code{cw} means width of the canonical character.
 
   The following is a list of properties that are meaningful for all
 image types (there are also properties which are meaningful only for
diff --git a/src/dispextern.h b/src/dispextern.h
index 3a4d6095f73..341fb3ac077 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3176,6 +3176,11 @@ reset_mouse_highlight (Mouse_HLInfo *hlinfo)
   int face_font_size;
   char *face_font_family;
 
+  /* Details of the font used to calculate image size relative to the
+     canonical character size, with `ch' and `cw' specifiers.  */
+  int face_font_height;
+  int face_font_width;
+
   /* True if this image has a `transparent' background -- that is, is
      uses an image mask.  The accessor macro for this is
      `IMAGE_BACKGROUND_TRANSPARENT'.  */
diff --git a/src/image.c b/src/image.c
index 9a465f0b180..244a694a52b 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2557,9 +2557,20 @@ image_get_dimension (struct image *img, Lisp_Object symbol)
 
   if (FIXNATP (value))
     return min (XFIXNAT (value), INT_MAX);
-  if (CONSP (value) && NUMBERP (CAR (value)) && EQ (Qem, CDR (value)))
-    return scale_image_size (img->face_font_size, 1, XFLOATINT (CAR (value)));
+  if (CONSP (value) && NUMBERP (CAR (value)))
+    {
+      Lisp_Object dim = CDR (value);
 
+      if (EQ (Qem, dim))
+        return scale_image_size (img->face_font_size,
+                                 1, XFLOATINT (CAR (value)));
+      if (EQ (Qch, dim))
+        return scale_image_size (img->face_font_height,
+                                 1, XFLOATINT (CAR (value)));
+      if (EQ (Qcw, dim))
+        return scale_image_size (img->face_font_width,
+                                 1, XFLOATINT (CAR (value)));
+    }
   return -1;
 }
 
@@ -3383,6 +3394,8 @@ lookup_image (struct frame *f, Lisp_Object spec, int face_id)
       img->face_foreground = foreground;
       img->face_background = background;
       img->face_font_size = font_size;
+      img->face_font_height = face->font->height;
+      img->face_font_width = face->font->average_width;
       img->face_font_family = xmalloc (strlen (font_family) + 1);
       strcpy (img->face_font_family, font_family);
       img->load_failed_p = ! img->type->load_img (f, img);
@@ -12760,6 +12773,8 @@ syms_of_image (void)
   DEFSYM (QCmax_height, ":max-height");
 
   DEFSYM (Qem, "em");
+  DEFSYM (Qch, "ch");
+  DEFSYM (Qcw, "cw");
 
 #ifdef HAVE_NATIVE_TRANSFORMS
   DEFSYM (Qscale, "scale");
-- 
2.25.1

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Eli Zaretskii]

> From: Evgeny Zajcev <lg.zevlg@gmail.com>
> Date: Thu, 28 Mar 2024 14:29:12 +0300
> Cc: Alan Third <alan@idiocy.org>, emacs-devel@gnu.org
> 
> чт, 28 мар. 2024 г. в 13:50, Evgeny Zajcev <lg.zevlg@gmail.com>:
> 
>  Yeah, will send an update soon.
> 
> Here is an update.  ChangeLog entry is included in the attached file

Thanks, installed on the master branch.

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Evgeny Zajcev]

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

вс, 31 мар. 2024 г. в 11:44, Eli Zaretskii <eliz@gnu.org>:

> > From: Evgeny Zajcev <lg.zevlg@gmail.com>
> > Date: Thu, 28 Mar 2024 14:29:12 +0300
> > Cc: Alan Third <alan@idiocy.org>, emacs-devel@gnu.org
> >
> > чт, 28 мар. 2024 г. в 13:50, Evgeny Zajcev <lg.zevlg@gmail.com>:
> >
> >  Yeah, will send an update soon.
> >
> > Here is an update.  ChangeLog entry is included in the attached file
>
> Thanks, installed on the master branch.
>

Hm, I don't see it in commits list for some reason

-- 
lg

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

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Eli Zaretskii]

> From: Evgeny Zajcev <lg.zevlg@gmail.com>
> Date: Mon, 1 Apr 2024 12:43:13 +0300
> Cc: alan@idiocy.org, emacs-devel@gnu.org
> 
> вс, 31 мар. 2024 г. в 11:44, Eli Zaretskii <eliz@gnu.org>:
> 
>  > From: Evgeny Zajcev <lg.zevlg@gmail.com>
>  > Date: Thu, 28 Mar 2024 14:29:12 +0300
>  > Cc: Alan Third <alan@idiocy.org>, emacs-devel@gnu.org
>  > 
>  > чт, 28 мар. 2024 г. в 13:50, Evgeny Zajcev <lg.zevlg@gmail.com>:
>  > 
>  >  Yeah, will send an update soon.
>  > 
>  > Here is an update.  ChangeLog entry is included in the attached file
> 
>  Thanks, installed on the master branch.
> 
> Hm, I don't see it in commits list for some reason

How about now?

Btw, your commit log message was grossly inaccurate...

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[lg.zevlg]

> 1 апр. 2024 г., в 14:42, Eli Zaretskii <eliz@gnu.org> написал(а):
> 
> 
>> 
>> From: Evgeny Zajcev <lg.zevlg@gmail.com>
>> Date: Mon, 1 Apr 2024 12:43:13 +0300
>> Cc: alan@idiocy.org, emacs-devel@gnu.org
>> 
>> вс, 31 мар. 2024 г. в 11:44, Eli Zaretskii <eliz@gnu.org>:
>> 
>>> From: Evgeny Zajcev <lg.zevlg@gmail.com>
>>> Date: Thu, 28 Mar 2024 14:29:12 +0300
>>> Cc: Alan Third <alan@idiocy.org>, emacs-devel@gnu.org
>>> 
>>> чт, 28 мар. 2024 г. в 13:50, Evgeny Zajcev <lg.zevlg@gmail.com>:
>>> 
>>> Yeah, will send an update soon.
>>> 
>>> Here is an update.  ChangeLog entry is included in the attached file
>> 
>> Thanks, installed on the master branch.
>> 
>> Hm, I don't see it in commits list for some reason
> 
> How about now?
> 

Yeah, see it now

> Btw, your commit log message was grossly inaccurate...

Why so, whats wrong?

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[lg.zevlg]

> 2 апр. 2024 г., в 02:02, lg.zevlg@gmail.com написал(а):
> 
> 
>> 1 апр. 2024 г., в 14:42, Eli Zaretskii <eliz@gnu.org> написал(а):
>> 
>> 
>>> 
>>> From: Evgeny Zajcev <lg.zevlg@gmail.com>
>>> Date: Mon, 1 Apr 2024 12:43:13 +0300
>>> Cc: alan@idiocy.org, emacs-devel@gnu.org
>>> 
>>> вс, 31 мар. 2024 г. в 11:44, Eli Zaretskii <eliz@gnu.org>:
>>> 
>>>> From: Evgeny Zajcev <lg.zevlg@gmail.com>
>>>> Date: Thu, 28 Mar 2024 14:29:12 +0300
>>>> Cc: Alan Third <alan@idiocy.org>, emacs-devel@gnu.org
>>>> 
>>>> чт, 28 мар. 2024 г. в 13:50, Evgeny Zajcev <lg.zevlg@gmail.com>:
>>>> 
>>>> Yeah, will send an update soon.
>>>> 
>>>> Here is an update.  ChangeLog entry is included in the attached file
>>> 
>>> Thanks, installed on the master branch.
>>> 
>>> Hm, I don't see it in commits list for some reason
>> 
>> How about now?
>> 
> 
> Yeah, see it now
> 
>> Btw, your commit log message was grossly inaccurate...
> 
> Why so, whats wrong?

Ah, I see now, wrong modification filename, blame cut&paste from sample changelog entry

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Alan Third]

On Thu, Mar 28, 2024 at 01:50:37PM +0300, Evgeny Zajcev wrote:
> Yes, this is intentional, because saying "height of the font" in docs, when
> font's pixel size is used in code, is misleading and it took me some time
> to understand why image renders smaller then font height if '(1 . em) is
> specified as dimension modifier.  That's why I started using coefficient
> (calculated with `my-em-height-ratio') to `em' specifier

Does this mean we have em wrong? It should ideally, IMO, be equivalent
to em in a web browser because that seems the most common use Emacs
users will be aware of. If we're using the wrong value then we should
change it.
-- 
Alan Third

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Evgeny Zajcev]

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

чт, 28 мар. 2024 г. в 14:41, Alan Third <alan@idiocy.org>:

> On Thu, Mar 28, 2024 at 01:50:37PM +0300, Evgeny Zajcev wrote:
> > Yes, this is intentional, because saying "height of the font" in docs,
> when
> > font's pixel size is used in code, is misleading and it took me some time
> > to understand why image renders smaller then font height if '(1 . em) is
> > specified as dimension modifier.  That's why I started using coefficient
> > (calculated with `my-em-height-ratio') to `em' specifier
>
> Does this mean we have em wrong? It should ideally, IMO, be equivalent
> to em in a web browser because that seems the most common use Emacs
> users will be aware of. If we're using the wrong value then we should
> change it.
>

Not wrong, but different.

I don't know what `em' in web browser means, however, in web browser,
for example, it is possible to create an image of 3em height and
display it like this:

.----------.
|          | Text line 1
|          | Text line 2
|          | Text line 3
`----------'

And image will be displayed solely and will fit into 3 lines height.

In Emacs, if image with 3em in height is displayed using slices, it
will result in a gap in the final slice, because 1em (font's size) is
less in pixels, than 1ch (font's height and final line height).

Let me elaborate: For example we have 45px font, with ascent 38px and
descent 10px, resulting in font height equal to 48px.  And we create
3em image, which will be 45*3 = 135px in height.  Now, we display this
image using slices: first slice will be: 0,48 (to occupy full line
height), second slice - 48,48, and third slice 96,39.  Last slice is
39px in height and if you display it with :ascent 'center it will have
4px gap.  If you display it with :ascent 100, it will increase final
line height by 1px, because 39-38.  You probably can calculate correct
:ascent value to get exact value of ascent in %, but, unfortunatety,
ascent is integer and in general integer number of percents is not
enough to get exact value (for example for fonts larger then 100px).
Also, this forces to modify :ascent value for the last slice, which
leads to additional garbage.

`ch' and `cw' specifiers solves two problems:
1) Makes it possible to create image intended for slices display
without gaps, and automatically adopts to text-scale changes.

2) Makes it possible to align images precisely to the width of the
given number of characters, especially this makes sense when monospace
font is used.  Also adopts to text-scale changes.

-- 
lg

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

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Alan Third]

On Thu, Mar 28, 2024 at 10:34:32PM +0300, Evgeny Zajcev wrote:
> чт, 28 мар. 2024 г. в 14:41, Alan Third <alan@idiocy.org>:
> 
> > On Thu, Mar 28, 2024 at 01:50:37PM +0300, Evgeny Zajcev wrote:
> > > Yes, this is intentional, because saying "height of the font" in docs,
> > when
> > > font's pixel size is used in code, is misleading and it took me some time
> > > to understand why image renders smaller then font height if '(1 . em) is
> > > specified as dimension modifier.  That's why I started using coefficient
> > > (calculated with `my-em-height-ratio') to `em' specifier
> >
> > Does this mean we have em wrong? It should ideally, IMO, be equivalent
> > to em in a web browser because that seems the most common use Emacs
> > users will be aware of. If we're using the wrong value then we should
> > change it.
> >
> 
> Not wrong, but different.

OK, I understand, thanks. Em should be equivalent to the font's size
(i.e. 12 points for a 12 point font) so it's good. This is different.

I'm happy with it if you are, Eli.
-- 
Alan Third

Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image

[Eli Zaretskii]

> Date: Fri, 29 Mar 2024 09:50:06 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> On Thu, Mar 28, 2024 at 10:34:32PM +0300, Evgeny Zajcev wrote:
> > чт, 28 мар. 2024 г. в 14:41, Alan Third <alan@idiocy.org>:
> > 
> > > On Thu, Mar 28, 2024 at 01:50:37PM +0300, Evgeny Zajcev wrote:
> > > > Yes, this is intentional, because saying "height of the font" in docs,
> > > when
> > > > font's pixel size is used in code, is misleading and it took me some time
> > > > to understand why image renders smaller then font height if '(1 . em) is
> > > > specified as dimension modifier.  That's why I started using coefficient
> > > > (calculated with `my-em-height-ratio') to `em' specifier
> > >
> > > Does this mean we have em wrong? It should ideally, IMO, be equivalent
> > > to em in a web browser because that seems the most common use Emacs
> > > users will be aware of. If we're using the wrong value then we should
> > > change it.
> > >
> > 
> > Not wrong, but different.
> 
> OK, I understand, thanks. Em should be equivalent to the font's size
> (i.e. 12 points for a 12 point font) so it's good. This is different.
> 
> I'm happy with it if you are, Eli.

Yes, I'm okay with that as well.

Thanks.