Bug 579626 - RTL text is not rendered correctly on TextLayout
Summary: RTL text is not rendered correctly on TextLayout
Status: CLOSED MOVED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.17   Edit
Hardware: PC Windows 10
: P3 major (vote)
Target Milestone: 4.24   Edit
Assignee: Platform-SWT-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
: 566914 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-04-08 00:35 EDT by Roden Roldan CLA
Modified: 2022-04-11 05:27 EDT (History)
4 users (show)

See Also:


Attachments
A test code snippet (1.72 KB, text/plain)
2022-04-08 00:35 EDT, Roden Roldan CLA
no flags Details
A sample output of the test code (3.15 KB, image/png)
2022-04-08 00:35 EDT, Roden Roldan CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Roden Roldan CLA 2022-04-08 00:35:04 EDT
Created attachment 288411 [details]
A test code snippet

The changes made on https://bugs.eclipse.org/bugs/show_bug.cgi?id=565526 caused RTL texts to be rendered incorrectly on TextLayout.

I've attached a test code that exhibits the issue. It compares the output of Text, Label, and TextLayout.
Comment 1 Roden Roldan CLA 2022-04-08 00:35:56 EDT
Created attachment 288412 [details]
A sample output of the test code
Comment 2 Phil Beauvoir CLA 2022-04-08 04:29:32 EDT
A user of our RCP app, Archi, has reported that RTL text is no longer rendered correctly when built on Eclipse 4.16 and later:

https://github.com/archimatetool/archi/issues/796

I wonder if Bug #565526 is the cause?
Comment 3 Phil Beauvoir CLA 2022-04-08 05:11:02 EDT
I researched further why, in our RCP app, RTL is now rendered incorrectly in Draw2d figures as in the attached snippet here.

When handling RTL text, Draw2d uses a class, org.eclipse.draw2d.text.BidiProcessor. and this uses the SWT TextLayout class. The TextLayout class was changed in Bug #565526 so I think this is a likely candidate.
Comment 4 Phil Beauvoir CLA 2022-04-08 05:33:51 EDT
(In reply to Phil Beauvoir from comment #2)
> A user of our RCP app, Archi, has reported that RTL text is no longer
> rendered correctly when built on Eclipse 4.16 and later:

I mean 4.17 and later.
Comment 5 Phil Beauvoir CLA 2022-04-09 07:17:36 EDT
I tested the Snippet removing the changes to class TextLayout in Bug #565526 and the text renders correctly.

I don't know how to fix this issue but it seems that RTL text is now broken for many users
Comment 6 Roden Roldan CLA 2022-04-09 22:08:04 EDT
I would personally disable Ligatures on TextLayout from Bug #562165 and its childbug fix Bug #565526, trading Ligatures for proper RTL rendering. 

Even if ligatures are disabled, the complete idea of the texts are still presented properly. It may not be visually appealing, but it would still communicate the correct information. In contrast, a sentence written backwards is not intuitive, and even if the user figured out how to "read" it, it's not something anyone would subject themselves to.

combined being not characters than critical more is backwards sentence a read to forced being user A.
Comment 7 Roden Roldan CLA 2022-04-09 22:14:26 EDT
(In reply to Roden Roldan from comment #6)
> Even if ligatures are disabled, the complete idea of the texts are still
> presented properly. It may not be visually appealing, but it would still
> communicate the correct information. In contrast, a sentence written
> backwards is not intuitive, and even if the user figured out how to "read"
> it, it's not something anyone would subject themselves to.

I have to attach a disclaimer: I am not an expert on ligatures. This is just my stance based on my current understanding of the problem.
Comment 8 Phil Beauvoir CLA 2022-04-10 05:25:08 EDT
I've just had a user who uses Hebrew text in Archi confirm that Bug #565526 is indeed the cause of this:

https://github.com/archimatetool/archi/issues/796#issuecomment-1094222770
Comment 9 Phil Beauvoir CLA 2022-04-10 08:21:50 EDT
In fact you don't have to do anything fancy to reproduce this. Just copy and paste the following text into an Eclipse editor (assumes UTF-8):


جارِ التحميل
Comment 10 Andrey Loskutov CLA 2022-04-10 08:35:08 EDT
@Niraj: if I understand previous comments right (I'm not expert in this area neither writing in BiDi language), we should either revert this patch https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/167051  or provide an improved version for 4.24, because it seem that BiDi support is now broken in StyledText. 

That is much more severe issue as wrong ligatures rendering in some corner case addressed in bug 565526.

WDYT?
Comment 11 Andrey Loskutov CLA 2022-04-10 08:36:24 EDT
*** Bug 566914 has been marked as a duplicate of this bug. ***
Comment 12 Niraj Modi CLA 2022-04-11 05:27:25 EDT
(In reply to Andrey Loskutov from comment #10)
> @Niraj: if I understand previous comments right (I'm not expert in this area
> neither writing in BiDi language), we should either revert this patch
> https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/167051  or
> provide an improved version for 4.24, because it seem that BiDi support is
> now broken in StyledText. 
> 
> That is much more severe issue as wrong ligatures rendering in some corner
> case addressed in bug 565526.
> 
> WDYT?

Working on this bug via below SWT github issue:
https://github.com/eclipse-platform/eclipse.platform.swt/issues/37