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

List:       openjdk-i18n-dev
Subject:    Re: <i18n dev> [OpenJDK 2D-Dev] [9] Review request for 8073400: Some Monospaced logical fonts have a
From:       Masayoshi Okutsu <masayoshi.okutsu () oracle ! com>
Date:       2016-03-24 6:18:23
Message-ID: 56F386AF.4040807 () oracle ! com
[Download RAW message or body]

Looks good to me.

Masayoshi

On 3/21/2016 4:56 PM, dmitry markov wrote:
> Hello,
> Thank you very much for the review.
>
> I added a simple test for the fix. Please find the updated webrev 
> here: http://cr.openjdk.java.net/~dmarkov/8073400/jdk9/webrev.01/
> Could you take a look at it, please?
>
> thank you in advance,
> Dmitry
> On 20/03/2016 07:11, Masayoshi Okutsu wrote:
>> The fix looks Okay to me. But there should be a regression test for 
>> this particular case? I wonder if we can have a test case to verify 
>> width(s) of monospaced glyphs.
>>
>> Masayoshi
>>
>>
>> On 3/18/2016 4:54 AM, Phil Race wrote:
>>> I think you are still waiting on i18n to reply here since the 
>>> exclusion ranges
>>> are mainly used for ensuring that the glyphs come from the right 
>>> font which
>>> is mainly for locale reasons. If i18n see no problems then I am OK 
>>> with this.
>>>
>>> -phil.
>>>
>>> On 02/17/2016 05:01 AM, dmitry markov wrote:
>>>> Hello Phil,
>>>>
>>>> Thank you for the review.
>>>>
>>>> According to unicode table the characters u201c and u201d are in 
>>>> General Punctuation block, see 
>>>> http://unicode-table.com/en/blocks/general-punctuation/
>>>> Many characters from this block are not supported by Courier New. 
>>>> So I do not think we need to remove the whole block from the 
>>>> exclusion range, since it will not have any effect except few 
>>>> characters like u201c and u201d. That's why I removed such small 
>>>> characters set (u2018 - u201F) from the exclusion range, but I can 
>>>> change this if it is necessary.
>>>>
>>>> At the same time the characters set (u2018 - u201F) is supported by 
>>>> Arial and Times New Roman which are mapped to other logical fonts. 
>>>> The adjusted exclusion range seems 'quite safe' from this point of 
>>>> view. So I do not think we need to modify any code such as 
>>>> FontConfiguration class and so on to apply these changes only to 
>>>> monospaced fonts. Please correct me if I am wrong.
>>>>
>>>> Adding i18n-dev.
>>>>
>>>> Internationalization team,
>>>> Could you review a small change in windows.fontconfig.properties, 
>>>> please?
>>>>
>>>>     bug: https://bugs.openjdk.java.net/browse/JDK-8073400
>>>>     webrev: 
>>>> http://cr.openjdk.java.net/~dmarkov/8073400/jdk9/webrev.00/
>>>>
>>>> Problem description:
>>>> On Windows some characters (in particular, left and right double 
>>>> quotation marks) have width differing from other characters' 
>>>> widths, when Monospaced logical font is used.
>>>> The default monospaced font for windows platform is Courier New. It 
>>>> contains the desired characters, (i.e. '\u201c' and '\u201d'). 
>>>> However the characters are in 'exclusion ranges' for this font due 
>>>> to settings in windows.fontconfig.properties. So when we try to 
>>>> obtain glyphs for these characters and calculate their bounds, we 
>>>> fallback to another font (Lucida) and use its glyphs.
>>>>
>>>> Fix:
>>>> Remove the following set of characters u2018 - u201F from the 
>>>> exclusion ranges.
>>>>
>>>> Thank you in advance,
>>>> Dmitry
>>>> On 10/02/2016 23:25, Phil Race wrote:
>>>>> I expect the exclusion range is there for a reason.
>>>>> I think (guess) that the intent was that where there is a sequence 
>>>>> like this :
>>>>>
>>>>> sequence.sansserif.x-windows-950=alphabetic,chinese-ms950,dingbats,symbol,chinese-ms950-extb 
>>>>>
>>>>>
>>>>> then those code points should come from the chinese font.
>>>>>
>>>>> Perhaps your adjusted exclusion range should apply only to the 
>>>>> monospaced fonts
>>>>> which are already putting the locale font first.
>>>>>
>>>>> Unfortunately it doesn't appear that this is possible with the 
>>>>> supported syntax
>>>>> https://docs.oracle.com/javase/1.5.0/docs/guide/intl/fontconfig.html#windows 
>>>>>
>>>>>
>>>>> i.e it supports selection only by charactersubsetname, not also by 
>>>>> logicalfontname.
>>>>> But you should check the code to see if this is in fact the case.
>>>>> There may need to be a non-ideal decision or a revision to that 
>>>>> spec. and its
>>>>> supporting code.
>>>>>
>>>>> And why be so narrow ? It seems you have made an even odder situation
>>>>> in having just those two code points 'un'-excluded.
>>>>> The argument that those were the two a customer complained about 
>>>>> does not
>>>>> hold up very well.
>>>>>
>>>>> I think you should also run this whole change+discussion by 
>>>>> i18n-dev ..
>>>>>
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 01/14/2016 02:32 AM, dmitry markov wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Could you review the fix for jdk9, please?
>>>>>>
>>>>>>     bug: https://bugs.openjdk.java.net/browse/JDK-8073400
>>>>>>     webrev: 
>>>>>> http://cr.openjdk.java.net/~dmarkov/8073400/jdk9/webrev.00/
>>>>>>
>>>>>> Problem description:
>>>>>> On Windows some characters (in particular, left and right double 
>>>>>> quotation marks) have width differing from other characters' 
>>>>>> widths, when Monospaced logical font is used.
>>>>>> The default monospaced font for windows platform is Courier New. 
>>>>>> It contains the desired characters, (i.e. '\u201c' and '\u201d'). 
>>>>>> However the characters are in 'exclusion ranges' for this font 
>>>>>> due to settings in windows.fontconfig.properties. So when we try 
>>>>>> to obtain glyphs for these characters and calculate their bounds, 
>>>>>> we fallback to another font (Lucida) and use its glyphs.
>>>>>>
>>>>>> Fix:
>>>>>> Remove the following set of characters u2018 - u201F from the 
>>>>>> exclusion ranges.
>>>>>>
>>>>>> Thanks,
>>>>>> Dmitry
>>>>>
>>>>
>>>
>>
>

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