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.ku...@oracle.com>; Philip Race <philip.r...@oracle.com>; 2d-...@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.ku...@oracle.com>; 2d-...@openjdk.java.net; > i18n-dev@openjdk.java.net; Naoto Sato <naoto.s...@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.ku...@oracle.com>; 2d-...@openjdk.java.net; >> i18n-dev@openjdk.java.net; Naoto Sato <naoto.s...@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 >
