[konsole] [Bug 393644] Regression: Bi-Directional rendering doesn't work after this commit

[Kurt Hindenburg]

https://bugs.kde.org/show_bug.cgi?id=393644

Kurt Hindenburg  changed:

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
 Resolution|--- |FIXED
  Latest Commit||https://commits.kde.org/kon
   ||sole/bdd98f8561ec92098794e4
   ||11517f630c87a1dc02

--- Comment #6 from Kurt Hindenburg  ---
Git commit bdd98f8561ec92098794e411517f630c87a1dc02 by Kurt Hindenburg, on
behalf of Mariusz Glebocki.
Committed on 03/05/2018 at 00:46.
Pushed by hindenburg into branch 'master'.

Restore Bi-Directional text support

Summary:
Fix regression introduced by commit a565bc9 (Clip character drawing
to its own cell). When the first character in a text fragment is
classified as RTL, rest of the fragment is also considered to be RTL
and is passed to `drawText()` as one string. The rendering is not
perfect (especially when RTL and LTR characters are mixed), but it
works as before.

{F5830191}

Test Plan:
* Display example sentences using a program which prints text directly
  to terminal (e.g. `echo`, `cat`):
```
حروف به هم پیوسته

کِی‌دی‌ئی (به انگلیسی: KDE) پروژه‌ای برای توسعه یک
میزکار آزاد و متن باز است.
```
* Display the sentences in Konsole built before
  commit a565bc97337a3bfc3a027f46aa2dec3e9a6f8618
* Compare visually

Reviewers: #konsole, sassanh, hindenburg

Reviewed By: #konsole, sassanh, hindenburg

Subscribers: hindenburg, #konsole

Tags: #konsole

Differential Revision: https://phabricator.kde.org/D12655

M  +28   -2src/TerminalDisplay.cpp

https://commits.kde.org/konsole/bdd98f8561ec92098794e411517f630c87a1dc02

-- 
You are receiving this mail because:
You are watching all bug changes.

[konsole] [Bug 393644] Regression: Bi-Directional rendering doesn't work after this commit

[Mariusz Glebocki]

https://bugs.kde.org/show_bug.cgi?id=393644

--- Comment #5 from Mariusz Glebocki  ---
I've created review request for the bug: https://phabricator.kde.org/D12655

I'll resolve clipping problems separately.

-- 
You are receiving this mail because:
You are watching all bug changes.

[konsole] [Bug 393644] Regression: Bi-Directional rendering doesn't work after this commit

[Mariusz Glebocki]

https://bugs.kde.org/show_bug.cgi?id=393644

Mariusz Glebocki  changed:

   What|Removed |Added

   Assignee|konsole-de...@kde.org   |m...@arccos-1.net
 Status|UNCONFIRMED |CONFIRMED
 Ever confirmed|0   |1

--- Comment #4 from Mariusz Glebocki  ---
I did a quick hack which checks characters direction property, and joins
right-to-left characters into one string before rendering. The example text
looks as it should. I'll read a bit about using different characters with
right-to-left characters (e.g. digits) and improve my fix.

As for clipping, I'll see how it works with clipping area enlarged by about
25%. This should restore all those details located just outside the cell, but
still prevent problematic characters from taking several cells.

-- 
You are receiving this mail because:
You are watching all bug changes.

[konsole] [Bug 393644] Regression: Bi-Directional rendering doesn't work after this commit

[Sassan Haradji]

https://bugs.kde.org/show_bug.cgi?id=393644

--- Comment #3 from Sassan Haradji  ---
That commit brings another issue for me, italic w and some other letters are
clipped.

-- 
You are receiving this mail because:
You are watching all bug changes.

[konsole] [Bug 393644] Regression: Bi-Directional rendering doesn't work after this commit

[Sassan Haradji]

https://bugs.kde.org/show_bug.cgi?id=393644

--- Comment #2 from Sassan Haradji  ---
(In reply to Mariusz Glebocki from comment #1)

> This makes each character to be drawn separately, which prevents offsets
> made by non-fixed-width characters

It'll be really hard to achieve bi-directional text rendering when we're
rendering each character separately. Persian characters connect to each other
and if you render them separately they can't connect. I guess we should avoid
this "separate rendering" at least for Persian and Arabic languages (I don't
know about Hebrew.)

P.S.Sample Persian renders:
Consider this:
حروف به هم پیوسته
If you render each character separately you'll get this:
ح‌ر‌و‌ف ب‌ه ه‌م پ‌ی‌و‌س‌ت‌ه
As you think letters change their shapes if they're rendered separately.

-- 
You are receiving this mail because:
You are watching all bug changes.

[konsole] [Bug 393644] Regression: Bi-Directional rendering doesn't work after this commit

[Mariusz Glebocki]

https://bugs.kde.org/show_bug.cgi?id=393644

Mariusz Glebocki  changed:

   What|Removed |Added

 CC||m...@arccos-1.net

--- Comment #1 from Mariusz Glebocki  ---
I guess right to left mark (and other marks) is separated from rest of the
text, so the rendering engine does not use it for the text.
I'll look into the code later.

(In reply to Sassan Haradji from comment #0)
> I wonder why we've added this condition.

This makes each character to be drawn separately, which prevents offsets made
by non-fixed-width characters (see commit msg)

-- 
You are receiving this mail because:
You are watching all bug changes.