Subject: bug#79285: [Patch] support :font-features in face

From: Binbin YE <[email-masked]>
Date: Sun, 31 Aug 2025 11:42:50 +0900

Hi Eli,

Thanks for the review and explanation, I addressed the issues you brought
up and it should be ready for the next round.

> If you edit with Emacs, then the .dir-locals.el file in our repository
> should set up the indentation for you, and then marking the region and
> typing C-M-\ will reindent the region according to our rules.

Somehow my Emacs does not respect the .dir-locals.el file, but I used
clang-format to format the changed lines

> Also, our style is not to use braces when the block has only one
> statement.

I have to say I had a hard time with this :P I should have removed the
braces from the one statement blocks.

> About "enough space" -- how many features could a font have?  And what
> to do if it has more than 32?  GNU coding conventions frown on
> arbitrary limits, and 32 sounds like it's quite arbitrary.  Can we do
> better?

I changed it to dynamically increase the size using existing xpalloc pattern

>  +          Lisp_Object feature_sym = XCAR (feature_spec);
>  +          Lisp_Object feature_val = XCDR (feature_spec);
>  +
>  +          if (SYMBOLP (feature_sym) && FIXNUMP (feature_val))
>
> Are we sure the value cannot be larger than most-positive-fixnum?
> Emacs can handle larger values if needed.

According to the harfbuzz document and the fonts I've seen, mostly the
values are either 0 or 1, occasionally you see 3
for different variations. This should be fine.

> As mentioned above, I think we should be smarter here: instead of
> overwriting the value of :font-features of the target face, we should
> merge the two lists so that it includes the features from both faces.
> Font features from FROM should override the same features from TO, but
> font features which exist in TO but not in FROM should be left in the
> resulting list.

I added support for attribute merging according to the review comment.

> In addition, this will need s NEWS item and a suitable addition to the
> ELisp manual, where the face attributes are described.

That's right, I've added both.

> Last, but not least, to accept a contribution of this size, we will
> need you to sign a copyright-assignment agreement with the FSF.  If
> you are prepared to do that, I will send you the form to fill and the
> instructions to go with it.

I have signed and sent the scanned version and I am currently waiting for
the digital copy from FSF.
From 0c9dbaff62669cca54c23df053f655dc47f91ab7 Mon Sep 17 00:00:00 2001
From: Binbin Ye <[email-masked]>
Date: Sun, 31 Aug 2025 11:35:33 +0900
Subject: [PATCH] support configuring font features in face

support configuring font features in face when using harfbuzz backend
using `:font-features'

* src/dispextern.h (lface_attribute_index): Add
LFACE_FONT_FEATURES_INDEX

* src/font.c (font_load_for_lface): Store font featurs in extra info

* src/hbfont.c (hb_features_from_lisp, hbfont_shape): Convert face
settings to harfbuzz features and pass to hb_shape_full

* src/xfaces.c (Finternal_set_lisp_face_attribute)
(lface_fully_specified_p, check_lface_attrs, init_xfaces)
(syms_of_xfaces): Add definition of font features attribute and update
setter function
(lface_hash, lface_same_font_attributes_p): Update face comparsion and
hashing for font features
(merge_face_font_features, merge_face_vectors, merge_face_ref): Support
font features attribute merging

* etc/NEWS: Announce face attribute update

* doc/lispref/display.texi: Add documentation for font features face
attribute
---
 doc/lispref/display.texi |  10 ++++
 etc/NEWS                 |   7 +++
 src/dispextern.h         |   2 +-
 src/font.c               |   6 ++
 src/hbfont.c             |  60 +++++++++++++++++++-
 src/xfaces.c             | 116 ++++++++++++++++++++++++++++++++++++++-
 6 files changed, 197 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index d044763523b..1b7f5f67e23 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2917,6 +2917,16 @@ Face Attributes
 character, point can be placed on what is seemingly a line at the end
 of the buffer---but Emacs can't highlight that ``line'', because it
 doesn't really exist.
+
+@item :font-features
+Font features to turn on or off.  Its value is an association list of
+open type tag and the tag value pairs in the following form:
+
+@example
+((@code{zero} . @var{1}) (@code{ss01} . @var{0}))
+@end example
+
+The attribute is only effective when using @code{harfbuzz} font backend.
 @end table
 
 @defun font-family-list &optional frame
diff --git a/etc/NEWS b/etc/NEWS
index 4a193484591..9dca8165708 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -71,6 +71,13 @@ done from early-init.el, such as adding to 'package-directory-list'.
 
 * Changes in Emacs 31.1
 
+** New face option ':font-features'.
+Emacs now supports enable or disable font features when using harfbuzz
+font backend with an alist.
+
+'(set-face-attribute 'default nil :font "Font name" :height 85
+  :font-features '((zero . 1) (ss19 . 1) (cv01 . 0) ))'
+
 ** 'prettify-symbols-mode' attempts to ignore undisplayable characters.
 Previously, such characters would be rendered as, e.g., white boxes.
 
diff --git a/src/dispextern.h b/src/dispextern.h
index 19ab104d2e6..6fc98d4302f 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1705,10 +1705,10 @@ #define FONT_TOO_HIGH(ft)						\
   LFACE_FONTSET_INDEX,
   LFACE_DISTANT_FOREGROUND_INDEX,
   LFACE_EXTEND_INDEX,
+  LFACE_FONT_FEATURES_INDEX,
   LFACE_VECTOR_SIZE
 };
 
-
 /* Box types of faces.  */
 
 enum face_box_type
diff --git a/src/font.c b/src/font.c
index dfe479f9355..767de52f2fa 100644
--- a/src/font.c
+++ b/src/font.c
@@ -3482,6 +3482,12 @@ font_load_for_lface (struct frame *f, Lisp_Object *attrs, Lisp_Object spec)
     {
       name = Ffont_get (spec, QCuser_spec);
       if (STRINGP (name)) font_put_extra (entity, QCuser_spec, name);
+
+      /* Store font features from face attributes for use during
+       * shaping.  */
+      Lisp_Object font_features = attrs[LFACE_FONT_FEATURES_INDEX];
+      if (CONSP (font_features))
+	font_put_extra (entity, QCfont_features, font_features);
     }
   return entity;
 }
diff --git a/src/hbfont.c b/src/hbfont.c
index 3d8dfb4b0e6..8655b9505ea 100644
--- a/src/hbfont.c
+++ b/src/hbfont.c
@@ -239,6 +239,49 @@ hbfont_otf_capability (struct font *font)
 
 /* Support functions for HarfBuzz shaper.  */
 
+/* Convert Lisp font features to HarfBuzz features.
+   FEATURES is a list of (feature . value) pairs.
+   Returns the number of features converted, fills HB_FEATURES array.
+   HB_FEATURES_SIZE is the current size of HB_FEATURES array. The
+   function allocate more depends on size of FEATURES.  */
+static ptrdiff_t
+hb_features_from_lisp (Lisp_Object features,
+		       hb_feature_t **hb_features,
+		       ptrdiff_t *hb_features_size)
+{
+  ptrdiff_t count = 0;
+
+  for (Lisp_Object tail = features; CONSP (tail); tail = XCDR (tail))
+    {
+      Lisp_Object feature_spec = XCAR (tail);
+      if (CONSP (feature_spec))
+	{
+	  if (count >= *hb_features_size)
+	    *hb_features = xpalloc (*hb_features, hb_features_size,
+				    count - *hb_features_size + 1, -1,
+				    sizeof **hb_features);
+
+	  Lisp_Object feature_sym = XCAR (feature_spec);
+	  Lisp_Object feature_val = XCDR (feature_spec);
+
+	  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);
+
+	      (*hb_features)[count].tag = tag;
+	      (*hb_features)[count].value = XFIXNUM (feature_val);
+	      (*hb_features)[count].start = 0;
+	      (*hb_features)[count].end = (unsigned int) -1;
+	      count++;
+	    }
+	}
+    }
+  return count;
+}
+
 static bool combining_class_loaded = false;
 static Lisp_Object canonical_combining_class_table;
 
@@ -491,7 +534,22 @@ hbfont_shape (Lisp_Object lgstring, Lisp_Object direction)
   if (!hb_font)
     return make_fixnum (0);
 
-  hb_bool_t success = hb_shape_full (hb_font, hb_buffer, NULL, 0, NULL);
+  /* Get font features from the font object.  */
+  Lisp_Object font_features
+    = Ffont_get (LGSTRING_FONT (lgstring), QCfont_features);
+
+  /* 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);
+
   if (font->driver->end_hb_font)
     font->driver->end_hb_font (font, hb_font);
   if (!success)
diff --git a/src/xfaces.c b/src/xfaces.c
index 7626dfeb75c..ca94983e9f9 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -1804,6 +1804,8 @@ #define LFACE_FONT(LFACE)	    AREF (LFACE, LFACE_FONT_INDEX)
 #define LFACE_INHERIT(LFACE)	    AREF (LFACE, LFACE_INHERIT_INDEX)
 #define LFACE_FONTSET(LFACE)	    AREF (LFACE, LFACE_FONTSET_INDEX)
 #define LFACE_EXTEND(LFACE)	    AREF (LFACE, LFACE_EXTEND_INDEX)
+#define LFACE_FONT_FEATURES(LFACE) \
+  AREF (LFACE, LFACE_FONT_FEATURES_INDEX)
 #define LFACE_DISTANT_FOREGROUND(LFACE) \
   AREF (LFACE, LFACE_DISTANT_FOREGROUND_INDEX)
 
@@ -1914,6 +1916,11 @@ check_lface_attrs (Lisp_Object attrs[LFACE_VECTOR_SIZE])
 	   || STRINGP (attrs[LFACE_FONTSET_INDEX])
 	   || RESET_P (attrs[LFACE_FONTSET_INDEX])
 	   || NILP (attrs[LFACE_FONTSET_INDEX]));
+  eassert (UNSPECIFIEDP (attrs[LFACE_FONT_FEATURES_INDEX])
+	   || IGNORE_DEFFACE_P (attrs[LFACE_FONT_FEATURES_INDEX])
+	   || RESET_P (attrs[LFACE_FONT_FEATURES_INDEX])
+	   || NILP (attrs[LFACE_FONT_FEATURES_INDEX])
+	   || CONSP (attrs[LFACE_FONT_FEATURES_INDEX]));
 #endif
 }
 
@@ -2162,7 +2169,8 @@ lface_fully_specified_p (Lisp_Object attrs[LFACE_VECTOR_SIZE])
 
   for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
     if (i != LFACE_FONT_INDEX && i != LFACE_INHERIT_INDEX
-        && i != LFACE_DISTANT_FOREGROUND_INDEX)
+	&& i != LFACE_DISTANT_FOREGROUND_INDEX
+	&& i != LFACE_FONT_FEATURES_INDEX)
       if ((UNSPECIFIEDP (attrs[i]) || IGNORE_DEFFACE_P (attrs[i])))
 	break;
 
@@ -2272,6 +2280,60 @@ merge_face_heights (Lisp_Object from, Lisp_Object to, Lisp_Object invalid)
   return result;
 }
 
+/* Merges the face font features FROM with the face font features TO,
+   and returns the merged font features.  */
+
+static Lisp_Object
+merge_face_font_features (Lisp_Object from, Lisp_Object to)
+{
+  if (CONSP (from))
+    {
+      if (NILP (to) || UNSPECIFIEDP (to))
+	to = Qnil;
+      else
+	/* Copy to before merging so that comparsion can know it is a
+       different attribute.  */
+	to = Fcopy_alist (to);
+
+      Lisp_Object tail = to;
+      while (CONSP (tail) && !NILP (XCDR (tail)))
+	tail = XCDR (tail);
+
+      for (Lisp_Object from_tail = from; !NILP (from_tail);
+	   from_tail = XCDR (from_tail))
+	{
+	  Lisp_Object font_feature = XCAR (from_tail);
+
+	  if (CONSP (font_feature))
+	    {
+	      Lisp_Object feature_sym = XCAR (font_feature);
+	      Lisp_Object feature_val = XCDR (font_feature);
+
+	      if (NILP (tail))
+		{
+		  /* Construct TAIL to a list and let TO point to the
+		   * start of the list.  */
+		  tail = Fcons (font_feature, Qnil);
+		  to = tail;
+		  continue;
+		}
+	      Lisp_Object exising_feature = Fassq (feature_sym, to);
+
+	      if (CONSP (exising_feature))
+		/* If TO already contains the feature, update the
+		 * value.  */
+		XSETCDR (exising_feature, feature_val);
+	      else
+		{
+		  XSETCDR (tail, Fcons (font_feature, Qnil));
+		  tail = XCDR (tail);
+		}
+	    }
+	}
+    }
+
+  return to;
+}
 
 /* Merge two Lisp face attribute vectors on frame F, FROM and TO, and
    store the resulting attributes in TO, which must be already be
@@ -2318,6 +2380,10 @@ merge_face_vectors (struct window *w,
 	    to[i] = merge_face_heights (from[i], to[i], to[i]);
 	    font_clear_prop (to, FONT_SIZE_INDEX);
 	  }
+	else if (i == LFACE_FONT_FEATURES_INDEX
+		 && !EQ (to[i], from[i]))
+	  to[i] = merge_face_font_features (from[i], to[i]);
+
 	else if (i != LFACE_FONT_INDEX && ! EQ (to[i], from[i]))
 	  {
 	    to[i] = from[i];
@@ -2917,6 +2983,15 @@ merge_face_ref (struct window *w,
 		  else
 		    err = true;
 		}
+	      else if (EQ (keyword, QCfont_features))
+		{
+		  if (NILP (value) || CONSP (value))
+		    to[LFACE_FONT_FEATURES_INDEX]
+		      = merge_face_font_features (
+			value, to[LFACE_FONT_FEATURES_INDEX]);
+		  else
+		    err = true;
+		}
 	      else
 		err = true;
 
@@ -3645,6 +3720,38 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
 	}
 #endif /* HAVE_WINDOW_SYSTEM */
     }
+  else if (EQ (attr, QCfont_features))
+    {
+      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value)
+	  && !RESET_P (value))
+	{
+	  /* Validate font features list format: Should be a list of
+	     (feature . value) pairs, without deplicate feature  */
+	  if (!NILP (value))
+	    {
+	      Lisp_Object tail;
+	      Lisp_Object seen = Qnil;
+	      for (tail = value; CONSP (tail); tail = XCDR (tail))
+		{
+		  Lisp_Object feature_spec = XCAR (tail);
+		  if (!CONSP (feature_spec)
+		      || !SYMBOLP (XCAR (feature_spec))
+		      || !FIXNUMP (XCDR (feature_spec)))
+		    signal_error ("Invalid font features format",
+				  value);
+		  Lisp_Object feature_sym = XCAR (feature_spec);
+		  if (!NILP (Fmemq (feature_sym, seen)))
+		    signal_error ("Duplicate font feature tag",
+				  feature_sym);
+		  seen = Fcons (feature_sym, seen);
+		}
+	      if (!NILP (tail))
+		signal_error ("Invalid font features format", value);
+	    }
+	}
+      old_value = LFACE_FONT_FEATURES (lface);
+      ASET (lface, LFACE_FONT_FEATURES_INDEX, value);
+    }
   else if (EQ (attr, QCinherit))
     {
       Lisp_Object tail;
@@ -4213,6 +4320,8 @@ DEFUN ("internal-get-lisp-face-attribute", Finternal_get_lisp_face_attribute,
     value = LFACE_FONT (lface);
   else if (EQ (keyword, QCfontset))
     value = LFACE_FONTSET (lface);
+  else if (EQ (keyword, QCfont_features))
+    value = LFACE_FONT_FEATURES (lface);
   else
     signal_error ("Invalid face attribute name", keyword);
 
@@ -4539,7 +4648,7 @@ lface_hash (Lisp_Object *v)
 	  ^ XHASH (v[LFACE_WEIGHT_INDEX])
 	  ^ XHASH (v[LFACE_SLANT_INDEX])
 	  ^ XHASH (v[LFACE_SWIDTH_INDEX])
-	  ^ XHASH (v[LFACE_HEIGHT_INDEX]));
+	  ^ XHASH (v[LFACE_FONT_FEATURES_INDEX]));
 }
 
 #ifdef HAVE_WINDOW_SYSTEM
@@ -4563,6 +4672,7 @@ lface_same_font_attributes_p (Lisp_Object *lface1, Lisp_Object *lface2)
 	  && EQ (lface1[LFACE_WEIGHT_INDEX], lface2[LFACE_WEIGHT_INDEX])
 	  && EQ (lface1[LFACE_SLANT_INDEX], lface2[LFACE_SLANT_INDEX])
 	  && EQ (lface1[LFACE_FONT_INDEX], lface2[LFACE_FONT_INDEX])
+	  && EQ (lface1[LFACE_FONT_FEATURES_INDEX], lface2[LFACE_FONT_FEATURES_INDEX])
 	  && (EQ (lface1[LFACE_FONTSET_INDEX], lface2[LFACE_FONTSET_INDEX])
 	      || (STRINGP (lface1[LFACE_FONTSET_INDEX])
 		  && STRINGP (lface2[LFACE_FONTSET_INDEX])
@@ -7366,6 +7476,7 @@ init_xfaces (void)
   face_attr_sym[LFACE_FONTSET_INDEX] = QCfontset;
   face_attr_sym[LFACE_DISTANT_FOREGROUND_INDEX] = QCdistant_foreground;
   face_attr_sym[LFACE_EXTEND_INDEX] = QCextend;
+  face_attr_sym[LFACE_FONT_FEATURES_INDEX] = QCfont_features;
 }
 
 void
@@ -7397,6 +7508,7 @@ syms_of_xfaces (void)
   DEFSYM (QCwidth, ":width");
   DEFSYM (QCfont, ":font");
   DEFSYM (QCfontset, ":fontset");
+  DEFSYM (QCfont_features, ":font-features");
   DEFSYM (QCdistant_foreground, ":distant-foreground");
   DEFSYM (QCbold, ":bold");
   DEFSYM (QCitalic, ":italic");
-- 
2.47.2