'Re: [OpenJDK 2D-Dev] [11] Review Request: 8202696 glyphs in textfield only shown when thai baht-char'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [11] Review Request: 8202696 glyphs in textfield only shown when thai baht-char
From:       Philip Race <philip.race () oracle ! com>
Date:       2018-06-22 5:03:44
Message-ID: 5B2C8330.1070708 () oracle ! com
[Download RAW message or body]

We don't run the tests in a turkish locale, like we don't build in that 
locale,
so it is not expected in general, so it was OK already, but this is 
still fine by me.

-phil.

On 6/21/18, 9:52 PM, Dipak Kumar wrote:
> Hi Naoto,
> 
> Thanks for the review.
> I have incorporated your suggestion and please find the updated webrev at \
> http://cr.openjdk.java.net/~dkumar/8202696/webrev.03/ . Request you to have a look \
> at it again and let me know your comments. 
> Thanks,
> Dipak
> 
> 
> -----Original Message-----
> From: Naoto Sato
> Sent: 21 June 2018 22:52
> To: Dipak Kumar<dipak.kumar@oracle.com>; Philip Race<philip.race@oracle.com>; \
>                 2d-dev@openjdk.java.net; i18n-dev@openjdk.java.net
> Subject: Re: [11] Review Request: 8202696 glyphs in textfield only shown when thai \
> baht-character is added 
> Hi Dipak,
> 
> Please use Locale.toLowerCase(Locale.ROOT) instead of no-arg toLowerCase(), as it \
> could fail in tr locale (line 60 in the test case). 
> Naoto
> 
> On 6/18/18 12:29 AM, Dipak Kumar wrote:
> > Hi Phil,
> > 
> > I am able to make the test automated after incorporating your suggestion to limit \
> > the test case to logical fonts. I observed that "canDisplayUpTo()" which \
> > internally uses "canDisplay()" does return expected values for logical fonts. 
> > Thanks for your guidance.
> > 
> > Please find the updated webrev at \
> > http://cr.openjdk.java.net/~dkumar/8202696/webrev.02/ . Request you to review it \
> > again and let me know your comments. 
> > Thanks,
> > Dipak
> > 
> > -----Original Message-----
> > From: Phil Race
> > Sent: 16 June 2018 00:11
> > To: Dipak Kumar<dipak.kumar@oracle.com>; 2d-dev@openjdk.java.net;
> > i18n-dev@openjdk.java.net; Naoto Sato<naoto.sato@oracle.com>
> > Subject: Re: [11] Review Request: 8202696 glyphs in textfield only
> > shown when thai baht-character is added
> > 
> > The exclusion ranges only apply to the logical fonts, so the iteration you are \
> > doing looking for any font is now not testing the case you are trying to fix, \
> > except by chance. You will need to limit the iteration to the logical fonts. 
> > I don't know why canDisplay(..) did not do what I would expect ..
> > particularly
> > since canDisplayUpTo should be using the same information.
> > 
> > -phil.
> > 
> > On 06/14/2018 12:07 AM, Dipak Kumar wrote:
> > > Hi Phil,
> > > 
> > > Thanks for reviewing the patch and providing useful information.
> > > Regarding your suggestion of using "canDisplay()" in test, I tried using this \
> > > function to make the test headless. But this function is returning true even \
> > > without proposed fix in fontconfig.properties file. Font exclusion is not \
> > > coming into picture in this function call. 
> > > In the previously written test case, I have added a check for font support \
> > > (whether characters can be displayed or not using function "canDisplayUpTo()"). \
> > > Please find the updated webrev at \
> > > http://cr.openjdk.java.net/~dkumar/8202696/webrev.01/ . 
> > > Request you to have a look into this again and let me know your inputs.
> > > 
> > > Many thanks,
> > > Dipak
> > > 
> > > -----Original Message-----
> > > From: Phil Race
> > > Sent: 13 June 2018 03:26
> > > To: Dipak Kumar<dipak.kumar@oracle.com>; 2d-dev@openjdk.java.net;
> > > i18n-dev@openjdk.java.net; Naoto Sato<naoto.sato@oracle.com>
> > > Subject: Re: [11] Review Request: 8202696 glyphs in textfield only
> > > shown when thai baht-character is added
> > > 
> > > BTW I think the synopsis really needs updating !
> > > 
> > > Once you know what is going on, continuing to title it after the random \
> > > empirical observation of the reporter is not necessary. I've changed it to : 
> > > "Remove exclusion range for phonetic chars in windows fontconfig.properties"
> > > 
> > > Please use that in the commit - when you get to it.
> > > 
> > > -phil.
> > > 
> > > On 06/12/2018 01:46 PM, Phil Race wrote:
> > > > I can only guess why this range was excluded.
> > > > My guess is that it was something to do with an attempt at allowing
> > > > as many of these glyphs as possible to come from (for example) a
> > > > Chinese font in a chinese locale even though we always put the latin
> > > > font first ..
> > > > 
> > > > The interesting reason why adding a Thai Baht magically makes them
> > > > appears is Thai forces TextLayout .. and it (apparently) bypasses
> > > > the exclusion range.
> > > > I suspect this is because layout needs to operate on physical fonts
> > > > so gets the list of physical fonts and operates on these explicitly
> > > > bypassing CompositeFont which is where this support exists.
> > > > You can prove that just by using one of these phonetic chars in
> > > > Font2DTest without adding Thai or anything else and switching to
> > > > TextLayout .. lo and behold .. it appears.
> > > > I wonder how long that has been the case ? Perhaps so long as to be
> > > > effectively forever ..
> > > > 
> > > > That makes a case for getting rid of all these ranges.
> > > > The use for the ranges is to control what physical font is used for
> > > > glyphs in cases where some font that may be "earlier" in the list
> > > > supplies glyphs that you'd actually prefer to come from some other
> > > > font.
> > > > That may be useful if the exclusion ranges were able to be applied
> > > > depending on your locale, but this alphabetic exclusion is
> > > > particularly questionable.
> > > > 
> > > > So the change is OK, but can't the test be automated .. and headless ?
> > > > You just need to do something like ask if the font "canDisplay()"
> > > > these code points.
> > > > However the test is fragile in the sense that it assumes that
> > > > logical fonts on Windows are capable of supporting them.
> > > > 
> > > > -phil
> > > > 
> > > > 
> > > > On 06/04/2018 11:32 PM, Dipak Kumar wrote:
> > > > > Hi,
> > > > > 
> > > > > Please review below fix :
> > > > > 
> > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8202696
> > > > > Webrev: http://cr.openjdk.java.net/~dkumar/8202696/webrev.00/
> > > > > 
> > > > > Characters which are not getting displayed actually belong to
> > > > > Phonetic Extensions
> > > > > (https://en.wikipedia.org/wiki/Phonetic_Extensions ).
> > > > > These characters are not getting rendered properly because they are
> > > > > in exclusion range mentioned in the windows.fontconfig.properties file.
> > > > > In the above fix, font exclusion ranges have been adjusted so that
> > > > > these characters do not fall within these ranges
> > > > > 
> > > > > Font related Jtreg tests and Mach5 client tests are fine.
> > > > > 
> > > > > Thanks,
> > > > > Dipak


[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic