Subject: Re: bug#41005: problem with rendering Persian text in Emacs 27

From: Pip Cet <[email-masked]>
Date: Thu, 04 Jun 2020 19:52:42 +0000

Eli Zaretskii <[email-masked]> writes:

>> From: Pip Cet <[email-masked]>
>> Cc: Eli Zaretskii <[email-masked]>,  [email-masked],
>>   [email-masked]
>> Date: Thu, 04 Jun 2020 08:28:24 +0000
>> 
>> What happens is that font-shape-gstring is called with direction == L2R,
>> mis-shapes the text, then caches that version in the composition gstring
>> cache. The cache doesn't distinguish directions, and it's never cleared,
>> so this "infects" other buffers, but only if they're opened afterwards,
>> and only for the same words.
>> 
>> shr appears to force bidi-display-reordering off. Removing that let
>> binding from shr-insert-document avoids the problem.

>> We should consider adding direction to our gstrings, on master. While
>> we're there, let's also add script, language, and harfbuzz features to
>> the gstrings so we know we've captured the precise harfbuzz parameters?
>
> On emacs-27, I can fix this by a simpler band-aid below.

> On master, I think we should indeed add direction to the cached
> gstrings, as there could be other much more subtle situations where
> looking at bidi-display-reordering is not enough, and we could then
> still cache a composition with the wrong direction.  Patches welcome.

Sure, such subtle situations exist.

> As for script and language, for now adding them would just waste
> memory, since we don't yet have any meaningful support for
> buffer-local, let-alone paragraph-local, scripts and languages.  When
> we added HarfBuzz support, I considered adding some functionality to
> determine language and/or script from the codepoints, but decided that
> it made little sense, since HarfBuzz already does exactly that in
> hb_buffer_guess_segment_properties.  So I think we should add to the
> FIXME in hbfont.c the fact that when we do support those internally in
> Emacs, we should add these attributes to cached gstrings, but not yet
> actually add them for now.

We're going to have to change the lgstring structure, though, right?

Could we maybe get away with just doing so once? My suggestion would be
to add a single field to gstrings which would currently be L2R or R2L
but might become an alist or something when we get around to adding
those features?

> Here's the patch I propose for emacs-27:

Let's try that.