> 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.