Bug 60534 - Brackets (..),{..},[..] inverted to )..(,}..{,]..[ when switch to RTL text direction with Graphite fonts only. ( Affect :all Libo applications).
Summary: Brackets (..),{..},[..] inverted to )..(,}..{,]..[ when switch to RTL text ...
Status: RESOLVED NOTOURBUG
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
4.0.0.3 release
Hardware: Other All
: medium normal
Assignee: navin patidar
URL:
Whiteboard: target:4.1.0
Keywords:
Depends on:
Blocks: RTL-CTL
  Show dependency treegraph
 
Reported: 2013-02-09 09:42 UTC by navin patidar
Modified: 2016-11-16 06:45 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
screenshot (56.77 KB, image/png)
2013-02-09 09:42 UTC, navin patidar
Details

Note You need to log in before you can comment on or make changes to this bug.
Description navin patidar 2013-02-09 09:42:18 UTC
Created attachment 74478 [details]
screenshot

Steps to reproduce bug. 

1. Start any of  Libo app.
2. Enable CTL.
3. Type text enclosed within brackets e.g. (Libreoffice) in RTL mode.
4. Change text font to “Linux Biolinum G”.


Result :
Brackets are inverted.  )Libreoffice(
Comment 1 Lior Kaplan 2013-02-09 10:16:07 UTC

*** This bug has been marked as a duplicate of bug 59892 ***
Comment 2 navin patidar 2013-02-09 11:13:20 UTC
hi Lior  
this bug isn't duplicate of bug 59892, here only brackets are inverted not complete text.
Comment 3 Urmas 2013-02-09 16:28:28 UTC

*** This bug has been marked as a duplicate of bug 33302 ***
Comment 4 Lior Kaplan 2013-02-09 18:56:12 UTC
@navin - You're right, this is brackets only, so not a duplicate of bug 59892.
@Urmas - This isn't a duplicate of bug 33302, as this also reproducible on Liux, while bug 33302 is mac only.
Comment 5 Commit Notification 2013-04-08 08:13:13 UTC
navin patidar committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=83d9c5562c27b5f766157eba70bebd320463a0af

fix fdo#60534 : use DefaultCharMapper::mapChar() to map RTL string unicodes.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 6 Ahmad Harthi 2013-04-08 09:01:40 UTC
FIXED, thanks Navin, need to check 33302 now :)
Comment 7 Commit Notification 2013-04-08 17:31:35 UTC
Rene Engelhard committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=08ba028992a77dee32dd76030bc409c6d3b39e36

Revert "fix fdo#60534 : use DefaultCharMapper::mapChar() to map RTL string unicodes."



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 8 Rene Engelhard 2013-04-08 17:32:51 UTC
Navin: Sorry, I needed to revert this.

layout/DefaultCharMapper.h seems to be a *internal* ICU header (see e.g. http://packages.debian.org/wheezy/libicu-dev; and I don't believe this changed between 48 and 51) and thus you can't use it. It breaks builds with system icu.
Comment 9 Khaled Hosny 2013-04-08 18:25:24 UTC
The original commit does not seem right to me either. Graphite fonts are supposed to handle mirroring on its own, so mirroring mirroed chars before passing text to Graphite engine is likely to break RTL text since the engine will re-mirror it. So I think the problem is that LibreOffice it telling Graphite to render that string of text RTL while it shouldn’t (I can reproduce the same issue by using, e.g., hb-view with a Graphite font and forcing direction to rtl).
Comment 10 Khaled Hosny 2013-04-08 18:32:53 UTC
Of course if the Graphite font is Arabic enabled, e.g. Scheherazade Graphite, the brackets are rendered correctly without this patch, so it is either a Graphite limitation (OpenType does not show this because mirroring is done by the engine not the font, unlike Graphite) or a mishandling of right aligned text in LibreOffice (right alignment is not the same as right to left direction).
Comment 11 navin patidar 2013-04-09 07:02:18 UTC
(In reply to comment #10)
> Of course if the Graphite font is Arabic enabled, e.g. Scheherazade
> Graphite, the brackets are rendered correctly without this patch, so it is
> either a Graphite limitation (OpenType does not show this because mirroring
> is done by the engine not the font, unlike Graphite) or a mishandling of
> right aligned text in LibreOffice (right alignment is not the same as right
> to left direction).

When i was working on this bug, checked with two graphite fonts Linux Biolinum G, Linux Libertine G and noticed that weak characters e.g. {,< are not mirrored when   writing direction is RTL. I assumed neither graphite engine nor graphite font does mirroring in case of RTL so we need to do that mirroring outside of graphite engine and then pass it to graphite engine.

As you mentioned some graphite font like Scheherazade Graphite(RTL enabled) does mirroring.
I found that Scheherazade Graphite font is also not perfect when text is changed to LTR, brackets are inverted.
Comment 12 Khaled Hosny 2013-04-09 10:00:22 UTC
I(In reply to comment #11)
> (In reply to comment #10)
> > Of course if the Graphite font is Arabic enabled, e.g. Scheherazade
> > Graphite, the brackets are rendered correctly without this patch, so it is
> > either a Graphite limitation (OpenType does not show this because mirroring
> > is done by the engine not the font, unlike Graphite) or a mishandling of
> > right aligned text in LibreOffice (right alignment is not the same as right
> > to left direction).
> 
> When i was working on this bug, checked with two graphite fonts Linux
> Biolinum G, Linux Libertine G and noticed that weak characters e.g. {,< are
> not mirrored when   writing direction is RTL. I assumed neither graphite
> engine nor graphite font does mirroring in case of RTL so we need to do that
> mirroring outside of graphite engine and then pass it to graphite engine.
> 
> As you mentioned some graphite font like Scheherazade Graphite(RTL enabled)
> does mirroring.
> I found that Scheherazade Graphite font is also not perfect when text is
> changed to LTR, brackets are inverted.

I think the problem here is the naïve way the Graphite integration code is handling text directionality; setting the graphite segment to rtl based on SAL_LAYOUT_BIDI_RTL is wrong IMO, this flag just tells what the base direction of the paragraph is, the code should instead run the BiDi algorithm on the text string, split it into contiguous directional runs and pass them one by one to Graphite engine, just like the OpenType integration code does. This way Latin text will never get into an RTL run and vice versa.
Comment 13 Khaled Hosny 2016-11-16 06:45:41 UTC
I can reproduce this exact same behaviour with Firefox using Linux Libertine G fonts, as such this is either a font bug or Graphite limitation, nothing for us to fix here.