Subject: Re: bug#52888: 29.0.50; font_{delete_unmatched,score} do not handle nil FONT_WEIGHT_INDEX

From: Eli Zaretskii <[email-masked]>
Date: Thu, 30 Dec 2021 20:39:28 +0200

> From: Sean Whitton <[email-masked]>
> Cc: [email-masked]
> Date: Thu, 30 Dec 2021 10:13:13 -0700
> 
> > Is the patch supposed to allow Emacs to handle these fonts, or is it
> > just the protection against assertion violations?
> 
> The latter -- the code implicitly assumes that the weight will always be
> a fixnum, but that is not so.  I want to fix that implicit assumption.
> 
> > If the latter, isn't it better to teach the font driver to handle
> > these fonts correctly?
> >
> > AFAIU, your patch basically will cause Emacs to reject such fonts and
> > not use them, which is tantamount to telling users to configure Emacs
> > to ignore them via, say, face-ignored-fonts.  Is that right, or am I
> > missing something?
> 
> I don't think it is equivalent to face-ignored-fonts.  The weight field
> in the entity vector is examined only when the weight field in the font
> spec is non-nil.  So my code does not categorically reject these fonts:
> it rejects them only when the user requested a specific weight, AFAICT.

Does it really make sense to accept these fonts in some situations,
but not in others?  AFAIU, what you suggest would cause Emacs to
accept these fonts when :weight is not mentioned (and so defaults to
'normal'), but to reject them if the 'normal' weight is specified
explicitly, is that right?  If so, it's confusing, and users will
complain.  Rejecting such fonts outright is at least consistent, and
thus better than semi-support.

> I don't know enough about these variable weight TTFs to judge whether it
> is worth anyone's time adding better support for them in Emacs.  In the
> case of Inconsolata-VariableFont_wdth,wght.ttf, the font authors provide
> separate .ttf files for each weight too, so there doesn't seem to be an
> expectation that applications know how to read the combined file.

I installed on the release branch a temporary fix, similar to what you
suggested, to avoid undefined behavior with those fonts, but I don't
think we should install something like that on master.  On master, I
think ftfont.c and its ilk should be fixed to handle these fonts
correctly, or reject them if we cannot DTRT with them for some reason.
I think the fact that we create invalid font entities from such fonts
is a clear sign that the font backend mishandles them, and if so,
that's where this problem should be corrected: we should create valid
font entities to begin with, with ;weight and other similar attributes
having numerical values, as expected.

Thanks.