all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Pip Cet <pipcet@gmail.com>
Cc: valizadeh.ho@gmail.com, 41005@debbugs.gnu.org, nicholasdrozd@gmail.com
Subject: bug#41005: problem with rendering Persian text in Emacs 27
Date: Thu, 04 Jun 2020 16:15:07 +0300	[thread overview]
Message-ID: <83y2p3as6c.fsf@gnu.org> (raw)
In-Reply-To: <878sh35j6f.fsf@gmail.com> (message from Pip Cet on Thu, 04 Jun 2020 08:28:24 +0000)

> From: Pip Cet <pipcet@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  41005@debbugs.gnu.org,
>   nicholasdrozd@gmail.com
> 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.

Right, thanks.  When I added that binding of bidi-display-reordering,
we didn't yet have HarfBuzz, and the other shapers' Arabic shaping
wasn't affected by the local text direction like HarfBuzz is.

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

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.

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

diff --git a/src/hbfont.c b/src/hbfont.c
index 576c5fe..4b3f64e 100644
--- a/src/hbfont.c
+++ b/src/hbfont.c
@@ -26,6 +26,7 @@
 #include "composite.h"
 #include "font.h"
 #include "dispextern.h"
+#include "buffer.h"
 
 #ifdef HAVE_NTGUI
 
@@ -438,7 +439,11 @@ hbfont_shape (Lisp_Object lgstring, Lisp_Object direction)
 
   /* If the caller didn't provide a meaningful DIRECTION, let HarfBuzz
      guess it. */
-  if (!NILP (direction))
+  if (!NILP (direction)
+      /* If they bind bidi-display-reordering to nil, the DIRECTION
+	 they provide is meaningless, and we should let HarfBuzz guess
+	 the real direction.  */
+      && !NILP (BVAR (current_buffer, bidi_display_reordering)))
     {
       hb_direction_t dir = HB_DIRECTION_LTR;
       if (EQ (direction, QL2R))





  reply	other threads:[~2020-06-04 13:15 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-01 18:02 bug#41005: problem with rendering Persian text in Emacs 27 hossein valizadeh
2020-05-01 18:51 ` Eli Zaretskii
     [not found]   ` <CAMyfNNp7FiFgAN5EVcVauawiy8ZB7U+eKY7qOeqZOnbMQfs5iQ@mail.gmail.com>
2020-06-03 14:34     ` Eli Zaretskii
2020-06-03 17:24 ` Nicholas Drozd
2020-06-03 18:01   ` Eli Zaretskii
2020-06-04  2:39     ` hossein valizadeh
2020-06-04  3:01       ` hossein valizadeh
2020-06-04  4:01         ` Eli Zaretskii
2020-06-04  4:10           ` Eli Zaretskii
2020-06-04  6:27             ` hossein valizadeh
2020-06-04  8:28               ` Pip Cet
2020-06-04 13:15                 ` Eli Zaretskii [this message]
2020-06-04 19:52                   ` Pip Cet
2020-06-05  4:46                     ` hossein valizadeh
2020-06-05  6:21                       ` Eli Zaretskii
2020-06-05 11:07                         ` Basil L. Contovounesios
2020-06-05 12:32                         ` hossein valizadeh
2020-06-05 12:53                           ` Eli Zaretskii
2020-06-05 13:05                             ` Pip Cet
2020-06-05 14:13                               ` Eli Zaretskii
2020-06-06  8:38                                 ` Pip Cet
2020-06-06  9:04                                   ` Eli Zaretskii
2020-06-06  9:11                                     ` Pip Cet
2020-06-06  9:24                                       ` Eli Zaretskii
2020-06-06 13:09                                         ` Pip Cet
2020-06-05 14:23                               ` hossein valizadeh
2020-06-05 14:25                                 ` Eli Zaretskii
2020-06-05  6:39                       ` Pip Cet
2020-06-05  8:01                     ` Eli Zaretskii
2020-06-05  8:41                       ` Pip Cet
2020-06-05 11:42                         ` Eli Zaretskii
2020-07-21 12:40                           ` Amin Bandali
2020-07-21 13:34                             ` Robert Pluim
2020-07-21 17:53                               ` Amin Bandali
2020-07-21 18:27                                 ` Eli Zaretskii
2020-07-22  2:12                                   ` Amin Bandali
2020-07-22 14:20                                     ` Eli Zaretskii
2020-07-24  4:11                                       ` Amin Bandali
2020-07-24  6:09                                         ` Eli Zaretskii
2020-07-25  4:19                                           ` Amin Bandali
2020-07-25  6:48                                             ` Eli Zaretskii
2020-07-25 15:53                                               ` Amin Bandali
2020-07-25 16:28                                                 ` Eli Zaretskii
2020-07-25 16:44                                                   ` Amin Bandali
2020-07-25 16:56                                                     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83y2p3as6c.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=41005@debbugs.gnu.org \
    --cc=nicholasdrozd@gmail.com \
    --cc=pipcet@gmail.com \
    --cc=valizadeh.ho@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.