bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

Recipe:
start emacs -Q etc/HELLO
move point to character position 358 (ARABIC LETTER SEEN)
hit C-SPC to activate the mark

Expected result:
mark is activated, no change to shaping

Actual result:
The character at position 357 is no longer composed with the one at
position 358. (screenshot with-mark.png) whereas it was composed
before (screenshot without-mark.png)

The same thing works fine in a separate buffer in an RTL paragraph.

[-- Attachment #2: without-mark.png --]
[-- Type: image/png, Size: 1150 bytes --]

[-- Attachment #3: with-mark.png --]
[-- Type: image/png, Size: 1265 bytes --]

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Basil L. Contovounesios]

Pip Cet <pipcet@gmail.com> writes:

> Recipe:
> start emacs -Q etc/HELLO
> move point to character position 358 (ARABIC LETTER SEEN)
> hit C-SPC to activate the mark
>
> Expected result:
> mark is activated, no change to shaping
>
> Actual result:
> The character at position 357 is no longer composed with the one at
> position 358. (screenshot with-mark.png) whereas it was composed
> before (screenshot without-mark.png)
>
> The same thing works fine in a separate buffer in an RTL paragraph.

FWIW, a similar thing was discussed in bug#35721.

-- 
Basil

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Pip Cet]

On Sun, May 31, 2020 at 12:40 PM Basil L. Contovounesios
<contovob@tcd.ie> wrote:
> > The same thing works fine in a separate buffer in an RTL paragraph.
> FWIW, a similar thing was discussed in bug#35721.

Thanks for the hint!

What I don't understand is why it works in pure RTL paragraphs,
though. If I yank the Arabic greeting into a new buffer, I can place
the mark anywhere and nothing happens to the shaping.

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Sun, 31 May 2020 13:10:47 +0000
> Cc: 41626@debbugs.gnu.org
> 
> What I don't understand is why it works in pure RTL paragraphs,
> though. If I yank the Arabic greeting into a new buffer, I can place
> the mark anywhere and nothing happens to the shaping.

Because testing where the face changes while going backwards in a
buffer is "tricky".  See handle_stop_backwards and its callers.  Maybe
there's some bug in the logic there, I don't know.

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

On Sun, May 31, 2020 at 2:59 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Sun, 31 May 2020 13:10:47 +0000
> > Cc: 41626@debbugs.gnu.org
> >
> > What I don't understand is why it works in pure RTL paragraphs,
> > though. If I yank the Arabic greeting into a new buffer, I can place
> > the mark anywhere and nothing happens to the shaping.
>
> Because testing where the face changes while going backwards in a
> buffer is "tricky".  See handle_stop_backwards and its callers.

Thanks!

I don't see why we need to invalidate composition state in
handle_stop_backwards.

How's this?

[-- Attachment #2: 0001-Avoid-losing-composition-state-in-handle_stop_backwa.patch --]
[-- Type: text/x-patch, Size: 1143 bytes --]

From 01fe1722036e6cbbf428e2068a666c36d718c74e Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Sun, 31 May 2020 16:58:19 +0000
Subject: [PATCH] Avoid losing composition state in handle_stop_backwards

* src/xdisp.c (handle_stop_backwards): Save composition iterator state
across our forward scan.
---
 src/xdisp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/xdisp.c b/src/xdisp.c
index ea28395cf5..e93b0bc201 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -8692,6 +8692,7 @@ handle_stop_backwards (struct it *it, ptrdiff_t charpos)
   ptrdiff_t where_we_are = (bufp ? IT_CHARPOS (*it) : IT_STRING_CHARPOS (*it));
   struct display_pos save_current = it->current;
   struct text_pos save_position = it->position;
+  struct composition_it save_cmp_it = it->cmp_it;
   struct text_pos pos1;
   ptrdiff_t next_stop;
 
@@ -8719,6 +8720,7 @@ handle_stop_backwards (struct it *it, ptrdiff_t charpos)
   it->bidi_p = true;
   it->current = save_current;
   it->position = save_position;
+  it->cmp_it = save_cmp_it;
   next_stop = it->stop_charpos;
   it->stop_charpos = it->prev_stop;
   handle_stop (it);
-- 
2.27.0.rc0

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Sun, 31 May 2020 17:08:56 +0000
> Cc: contovob@tcd.ie, 41626@debbugs.gnu.org
> 
> I don't see why we need to invalidate composition state in
> handle_stop_backwards.
> 
> How's this?

Good catch!  Please install, and thanks.

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Eli Zaretskii]

> Date: Sun, 31 May 2020 20:30:48 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: contovob@tcd.ie, 41626@debbugs.gnu.org
> 
> Good catch!  Please install, and thanks.

On master, of course.

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Pip Cet]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 31 May 2020 20:30:48 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: contovob@tcd.ie, 41626@debbugs.gnu.org
>> 
>> Good catch!  Please install, and thanks.
>
> On master, of course.

Done, closing this bug. Thank you!

bug#41626: 28.0.50; activating mark breaks RTL shaping in etc/HELLO (auto-composition-mode)

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Sun, 31 May 2020 10:02:40 +0000
> 
> Recipe:
> start emacs -Q etc/HELLO
> move point to character position 358 (ARABIC LETTER SEEN)
> hit C-SPC to activate the mark
> 
> Expected result:
> mark is activated, no change to shaping
> 
> Actual result:
> The character at position 357 is no longer composed with the one at
> position 358. (screenshot with-mark.png) whereas it was composed
> before (screenshot without-mark.png)
> 
> The same thing works fine in a separate buffer in an RTL paragraph.

This is a known problem with how we handle character composition and
faces when they intermix.  At the time, my conclusion was that fixing
this would be next to impossible without changing the way we handle
the "stop position" during iteration through buffer text.

Of course, I could be mistaken.