[OpenJDK 2D-Dev] [8u] RFR 8201801: RTL language (Hebrew) is presented from left to right

[Phil Race]

I see you noticed the problem with it not necessarily being an AAT font.
It seems this method is a copy/paste of the method added in JDK 9.
So this will work but looking at it, I think in JDK 9 I should have made 
this more efficient.
Probably it was a detail in the larger work, that I never came back to.
On MacOS where it calls through the PhysicalFont interface, since CFont 
is not a TrueTypeFont,
it will be retrieving all the data in the table from native to Java on 
every call to layout. That will be expensive.

You can mitigate this in your case by reversing the order here

+ if (isAAT(font) && (typo_flags & 0x80000000) != 0) {


to

+ if (((typo_flags & 0x80000000) != 0) && isAAT(font)) {


So it will only take the hit for RTL text which will save a lot of 
cases  ...
I have filed https://bugs.openjdk.java.net/browse/JDK-8210384 which I 
will fix for 12.

Up to you whether you also want to wait for that fix, or just update as 
I have suggested
and maybe take a follow-on fix later.

Note that whilst your fix will at least make sure text reads in the 
correct direction, it
means it will be using default built-in shaping rules of ICU and so may 
miss at least
some features provided by the font.

-phil.

On 09/03/2018 05:54 AM, Dmitry Markov wrote:
> Hi Prasanta,
>
> Thank you for the feedback. I missed the usage of OSX specific class 
> in shared code for some reasons. Please find the updated webrev here: 
> http://cr.openjdk.java.net/~dmarkov/8201801/jdk8u/webrev.01/ 
> <http://cr.openjdk.java.net/%7Edmarkov/8201801/jdk8u/webrev.01/>
> I replaced the usage of CFont with a new private method which tests 
> either a font is AAT or not.
>
> The issue is jdk8u (ICU) specific. It does not take place on jdk12 
> because starting from jdk9 we use Harfbuzz as default layout engine.
>
> Thanks,
> Dmitry
>
>> On 3 Sep 2018, at 11:33, Prasanta Sadhukhan 
>> <prasanta.sadhukhan at oracle.com 
>> <mailto:prasanta.sadhukhan at oracle.com>> wrote:
>>
>> Hi Dmitry,
>>
>> Not going into technicalities of the fix, but it seems it will break 
>> non-macos build as you are checking for CFont in a shared class?
>> Also, if it's the issue still exists, then why you are fixing only in 
>> 8u and not in jdk12?
>>
>> Regards
>> Prasanta
>> On 9/3/2018 3:20 PM, Dmitry Markov wrote:
>>> Hello,
>>>
>>> Could you review a fix for jdk8u, please?
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8201801
>>> webrev: http://cr.openjdk.java.net/~dmarkov/8201801/jdk8u/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Edmarkov/8201801/jdk8u/webrev.00/>
>>>
>>> Problem description:
>>> The fix for 7162125 [1] enabled font tables processing on OSX. 
>>> However there is a lack of support for RTL languages in ICU layout 
>>> engine. Quote from ICU guide: “…The AAT processing in the 
>>> LayoutEngine is relatively basic as it only applies the default 
>>> features in left-to-right text. This processing has been tested for 
>>> Devanagari text. Since AAT processing is not script-specific, it 
>>> might not work for other scripts…”, more details at 
>>> http://userguide.icu-project.org/layoutengine
>>> As a result all RTL languages on OSX are incorrectly laid out, (i.e. 
>>> the letters are reversely presented).
>>>
>>> Fix:
>>> Skip font tables for RTL languages on OSX platform.
>>>
>>> Thanks,
>>> Dmitry
>>>
>>> [1] - https://bugs.openjdk.java.net/browse/JDK-7162125
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180904/5bdd8f40/attachment-0001.html>