GNU bug report logs - #79285
[Patch] support :font-features in face

Previous Next

Full log

Message #35 received at 79285 <at> debbugs.gnu.org (full text, mbox):

From: Binbin YE <phantom2501 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79285 <at> debbugs.gnu.org
Subject: Re: bug#79285: [Patch] support :font-features in face
Date: Sun, 31 Aug 2025 19:05:48 +0900

[Message part 1 (text/plain, inline)]

Hi Eli,

Thanks for the comments. Some quick responses:

>  +       if (SYMBOLP (feature_sym) && FIXNUMP (feature_val))
>  +         {
> > +           /* Convert symbol to HarfBuzz tag.  */
>  +           const char *feature_name
>  +             = SSDATA (SYMBOL_NAME (feature_sym));
>  +           hb_tag_t tag = hb_tag_from_string (feature_name, -1);
>
> We will need to DEF_DLL_FN and LOAD_DLL_FN for hb_tag_from_string, for
> this to work on Windows.  But you can leave this to me, if you want.

I don't have access to a Windows machine, it would be great if you can help
me with that.

>  +  /* Cache features array to store enabled font features.  */
>  +  static hb_feature_t *hb_features;
>  +  static ptrdiff_t hb_features_size;
>  +  unsigned int num_features = 0;
>  +  if (!NILP (font_features))
>  +    num_features = hb_features_from_lisp (font_features, &hb_features,
> > +                                       &hb_features_size);
>  +  hb_bool_t success
>  +    = hb_shape_full (hb_font, hb_buffer,
> > +                  num_features == 0 ? NULL : hb_features,
>  +                  num_features, NULL);
>
> Hmm... not sure I understand what kind of caching is being used here.
> AFAIU, hb_features are recomputed anew each time we call hbfont_shape,
> so how does this caching help?

I saw the comment a few lines below for the hb_buffer (static hb_buffer_t
*hb_buffer = NULL;) to have less allocation and
I followed that. It is true that the features are recomputed but the array
can be reused in the next call. If new
allocation is preferred I can change to that.


> One more comment: did you try this code with text that we normally
> don't pass to the shaping engine to display, like plain-ASCII text?
> Emacs normally calls the shaping engine only for characters whose
> slots in composition-function-table are non-nil.  Otherwise we display
> the font glyphs directly without shaping.  (This is an optimization:
> using the shaping engine slows down redisplay, because it is
> implemented by calling to Lisp, which then calls back into C.)  So,
> for example, if someone places a face with :font-features attribute on
> plain-ASCII text, they will probably not see any effect.
>

Yes you are right about it. I was confused why it did not have any effect
initially. And I figured I needed to do
something with composition-function-table. This is my test code

#+begin_src elisp
  (set-char-table-range composition-function-table
    ?0
    '(["." 0 font-shape-gstring]))

  (set-char-table-range composition-function-table
    ?!
    '(["\\(!==\\)" 0 font-shape-gstring]))
#+end_src

#+begin_src elisp
  (set-face-attribute 'default nil :font "JetBrains Mono" :height 100
    :font-features '((zero . 0) (ss19 . 1) (zero . 1)))
#+end_src

#+begin_src elisp
  (set-face-attribute 'default nil :font "JetBrains Mono" :height 100
    :font-features '((zero . 1)))
#+end_src


> If I'm right, then we will need to make changes in display-engine
> functions like composition_compute_stop_pos, composition_reseat_it,
> find_composition, and others, to force the text which has such a face
> attribute to be handed to HarfBuzz for shaping.  An alternative is to
> require that use of this face attribute needs special setup of
> composition-function-table, but that is IMO worse because it will slow
> down display of the relevant characters even if they don't have the
> face with this attribute.

Thanks for pointing that out, I can explore that part of the code. I have
not looked much into composite.c

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.