Bug 445846 - "Bi-directional text rendering" should be renamed to something like "Support for complex scripts"
Summary: "Bi-directional text rendering" should be renamed to something like "Support ...
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: master
Platform: Other Linux
: NOR minor
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-11-21 02:30 UTC by ninjalj
Modified: 2022-12-06 23:06 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ninjalj 2021-11-21 02:30:32 UTC
SUMMARY

At https://invent.kde.org/utilities/konsole/-/merge_requests/524 @tcanabrava commented:

«When we are in bidirectional text mode, for some reason, konsole treated a sequence of braile characters as "something that could be grouped into a single string", passed directly to the painter->drawText, producing strange results.»

From https://bugs.kde.org/show_bug.cgi?id=401094#c11

« Possible solutions:
 * Add an option for enabling the feature. I'm against this. It will be broken eventually, because many developers won't test yet another rendering mode. Also: hack.
  * Rename "Bi-directional text rendering" option to something like "Support for complex scripts" and enable special > rendering for complex scripts like Malayalam when it is enabled. RTL scripts already have the same problems, so > users wouldn't be suprised. This solution has the same issues as previous one, but it's already there.»

While the behavior changed as per option 2 above, the rename never happened.

ADDITIONAL INFORMATION

Some history:
 - Per https://bugs.kde.org/show_bug.cgi?id=393644, commit https://github.com/KDE/konsole/commit/a565bc97337a3bfc3a027f46aa2dec3e9a6f8618 , which clips characters to its cell, broke Arabic and other scripts that need to render whole words (in Arabic, there are different glyphs for initial, medial, final and isolated positions).
 - Commit bdd98f8561ec92098794e411517f630c87a1dc02 attempted a fix by grouping characters with Right or ArabicLetter directionality.
 - Bug https://bugs.kde.org/show_bug.cgi?id=401094 is for Malayalam. As it happens, many Brahmic scripts, while typically being Left-To-Right, also need to render whole runs of text as a unit (a syllable may be represented by a glyph for a consonant cluster with dependant vowels in its vecinity).
 - Commit 94ff722fc0e68987bd743663bc63ef99ff4e0706 was a first attempt to fix Malayalam.
 - Commit 8ad28a12574cadc7a41e152ec683380d7743c2a8 groups characters by script when bidi is enabled and the character is not double width.
Comment 1 Ahmad Samir 2021-11-21 11:48:30 UTC
IMHO, this makes sense.
Comment 2 Duncan 2021-11-22 08:32:04 UTC
"Support for complex scripts" doesn't work in konsole AKA terminal context because there, many (most?) would think "scripts" refers to shell-scripts -- I was browsing new bugs and was *very* confused seeing the bug title.

Would "complex fonts" be the correct term?  But that would get tangled up with font choice.  Maybe "complex character rendering"?
Comment 3 Ahmad Samir 2021-11-23 09:33:13 UTC
To disambiguate, "Support for complex scripts (RTL, Chinese, Urdu ...etc)" ?
Comment 4 Duncan 2021-11-25 00:10:48 UTC
(In reply to Ahmad Samir from comment #3)
> To disambiguate, "Support for complex scripts (RTL, Chinese, Urdu ...etc)" ?

Works for me! =:^)
Comment 5 ninjalj 2021-11-29 16:33:05 UTC
Chinese doesn't need this setting, it just needs support for wide characters (Unicode property East_Asian_Width), which take 2 cells. Urdu is a RTL language.

So maybe something like "Support for complex scripts (Right to left languages, Brahmic/Indic scripts, ...)"?

Currently, at src/widgets/EditProfileAdvancedPage.ui we have this:

      <widget class="QCheckBox" name="enableBidiRenderingButton">
       <property name="sizePolicy">
        <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
         <horstretch>0</horstretch>
         <verstretch>0</verstretch>
        </sizepolicy>
       </property>
       <property name="toolTip">
        <string>Enable Bi-Directional display on terminals (valid for Arabic, Farsi or Hebrew only)</string>
       </property>
       <property name="text">
        <string>Bi-Directional text rendering</string>
       </property>
      </widget>


Both the text and the tooltip need to be rewritten, and I can't think off-hand of something short, concise and sufficiently clear to people without previous knowledge of RTL and Indic text rendering.

BTW, the setting now does two things:
 - It enables RTL display by not putting a Left-to-Right Mark in front of text to be rendered.
 - It allows rendering a run of text as a whole unit.
Comment 6 Ahmad Samir 2021-11-30 09:32:28 UTC
Well, if it's a choice between UI text being clear or short, I vote clear; ideally it should be both, but until someone has a better suggestion, we can go with your text.
Comment 7 Bug Janitor Service 2021-12-04 22:19:54 UTC
A possibly relevant merge request was started @ https://invent.kde.org/utilities/konsole/-/merge_requests/544
Comment 8 Bug Janitor Service 2022-08-06 18:51:54 UTC
A possibly relevant merge request was started @ https://invent.kde.org/utilities/konsole/-/merge_requests/722
Comment 9 Kurt Hindenburg 2022-08-27 01:10:48 UTC
Git commit 76f879cd70fb494ab2334d2660b34679546f3d9d by Kurt Hindenburg, on behalf of Matan Ziv-Av.
Committed on 26/08/2022 at 19:24.
Pushed by hindenburg into branch 'master'.

Draw characters in exact positions

QT can't be made to draw monospaced text (if the font does not cooperate),
so avoid combining characters, using a QPainter::drawText() call for each
character.

For bidi text support this change requires konsole to reorder and reshape
the characters. This is done using the ICU library (which QT also uses).

This change allows for some improvements related to text rendering:

- More precise bidi reordering, which is no longer changed by characters'
  attributes and selection.
- underlines drawn separately from the text, allowing for differing
  underline modes (double, curly, dashed, dotted, colored).
- Overriding font for emoji characters.

This commit fixes a few bugs and addresses a lot more:

Feature requests: More standard conforming RTL and various underlines:
Related: bug 403729, bug 387811, bug 416508, bug 452087, bug 425973, bug 430822, bug 442742, bug 441037, bug 430822, bug 440070, bug 450017, bug 453086, bug 381593, bug 451716

Using non-monospace font:



Emoji:

Regression: devanagari rendering

M  +2    -0    CMakeLists.txt
M  +1    -0    src/CMakeLists.txt
M  +47   -25   src/FontDialog.cpp
M  +2    -1    src/FontDialog.h
M  +127  -23   src/Screen.cpp
M  +18   -5    src/Screen.h
M  +28   -13   src/Vt102Emulation.cpp
M  +0    -29   src/autotests/CharacterTest.cpp
M  +0    -1    src/autotests/CharacterTest.h
M  +1    -1    src/autotests/TerminalCharacterDecoderTest.cpp
M  +119  -29   src/characters/Character.h
M  +1    -1    src/characters/Hangul.cpp
M  +5    -4    src/decoders/HTMLDecoder.cpp
M  +1    -1    src/decoders/PlainTextDecoder.cpp
M  +3    -0    src/profile/Profile.cpp
M  +29   -0    src/profile/Profile.h
M  +101  -6    src/terminalDisplay/TerminalDisplay.cpp
M  +7    -0    src/terminalDisplay/TerminalDisplay.h
M  +42   -0    src/terminalDisplay/TerminalFonts.cpp
M  +12   -0    src/terminalDisplay/TerminalFonts.h
M  +542  -269  src/terminalDisplay/TerminalPainter.cpp
M  +40   -13   src/terminalDisplay/TerminalPainter.h
M  +2    -0    src/widgets/EditProfileAdvancedPage.ui
M  +113  -3    src/widgets/EditProfileAppearancePage.ui
M  +61   -3    src/widgets/EditProfileDialog.cpp
M  +6    -1    src/widgets/EditProfileDialog.h

https://invent.kde.org/utilities/konsole/commit/76f879cd70fb494ab2334d2660b34679546f3d9d