[OpenJDK 2D-Dev] [9] request for review: 8078382: Wrong glyph is displayed for a derived font

[Alexey Ivanov]

Hi Sergey,

On 31.05.2016 19:43, Sergey Bylokhov wrote:
> On 31.05.16 19:30, Phil Race wrote:
>>> The patch changes the order of font selection: bold will be used, if
>>> possible, as the base for bold-italic instead of italic which is the
>>> current default. It also fixes the issue where italic is returned
>>> instead of bold.
>>
>> OK. +1
>
> I am not sure but should FontFamily.getClosestStyle() be updated also?:
>         case Font.BOLD|Font.ITALIC:
>             if (italic != null) {
>                 return italic;
>             } else if (bold != null) {
>                 return bold;
>             } else {
>                 return plain;
>             }
>         }
>

Good catch!
The discussion is in progress…


Regards,
Alexey

>>
>> -phil.
>>
>>>
>>>
>>> Thank you in advance,
>>> Alexey
>>>
>>> On 22.07.2015 19:33, Alexey Ivanov wrote:
>>>> Hi Phil,
>>>>
>>>> On 16.07.2015 21:38, Phil Race wrote:
>>>>> On 07/16/2015 06:08 AM, Andrew Brygin wrote:
>>>>>> Hi Phil,
>>>>>>
>>>>>>  another option to avoid the problem is to be a bit more specific
>>>>>> regarding the
>>>>>>  required font when we obtaining  lcd glyph from GDI.
>>>>>>  If we specify a particular name of the font which we used to
>>>>>> construct the
>>>>>>  glyph vector, then we will get glyphs exactly for desired 
>>>>>> characters:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~bae/8078382/9/webrev.01/
>>>>>>
>>>>>>  This change affects only the case of lcd glyphs on windows,
>>>>>>  it reduces the scope of required testing.
>>>>>
>>>>> This is heading in the direction I was thinking of but since GDI is
>>>>> excepting a face name
>>>>> (what we call a family name), I am not sure if this will always work
>>>>> as is.
>>>>> There are possible issues with using a localized name and the length
>>>>> of the full name exceeding what Windows allows here.
>>>>> And there may be unintended consequences that are not immediately
>>>>> obvious.
>>>>> I would like to try limit this to the case where we can positively
>>>>> identify that the
>>>>> font is not the one we expected. And do it cheaply too.
>>>>> I do not have a quick answer here but roughly the algorithm might be
>>>>> - specify family, check (somehow) if GDI selects the same base font
>>>>> - if not, try the facename approach (if the name fits). Did we
>>>>> succeed and get the same base font ?
>>>>> - if not, bail on GDI for this case and do the rasterisation 
>>>>> ourselves.
>>>>>
>>>>> "cheaply" might depend on caching a success value on the first
>>>>> attempt, so
>>>>> that we only do it once, ever, for a font. Then the problem becomes
>>>>> how to
>>>>> do the verification. One approach might be to call GetFontData() 
>>>>> which
>>>>> will give us some chunk of the font file and we can see if the name
>>>>> (or something
>>>>> else we can quickly and reliably parse) is exactly that we expect ..
>>>>
>>>> It looks there's no easy way to detect whether GDI selected the same
>>>> base font or not. GetTextFace function doesn't help it: it always
>>>> returns the face name passed in LOGFONT except in the cases where
>>>> there's no such font.
>>>>
>>>> I haven't found any other API which could tell us what font GDI
>>>> selected. So the only alternative is to use GetFontData and parse the
>>>> font file itself. Yet I can't find any example how to use this
>>>> function. I'll keep searching in that direction.
>>>>
>>>>
>>>> Regards,
>>>> Alexey
>>>>
>>>>>
>>>>> Leaving aside the 'wrong glyph' case, I have to suppose it is
>>>>> possible that
>>>>> there are other un-noticed cases where we use a different base font
>>>>> than
>>>>> that selected by GDI. The algorithms are not defined anywhere I can
>>>>> locate.
>>>>>
>>>>>>
>>>>>>  However, there seems to be a copy&paste error in FontFamily.java:
>>>>>>  on lines 340 - 341 we check that bold font fits our needs but use
>>>>>> italic
>>>>>>  anyway. Was it done by purpose, or this is really an error?
>>>>>
>>>>> That is  a copy/paste mistake. It should be fixed.
>>>>>
>>>>> -phil.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Andrew
>>>>>>
>>>>>> On 7/15/2015 7:25 PM, Phil Race wrote:
>>>>>>> This probably needs more examination and perhaps a more complex 
>>>>>>> fix.
>>>>>>> The observation that GDI bases bold-italic on the bold version not
>>>>>>> the
>>>>>>> italic version is an implementation choice just as we had done the
>>>>>>> opposite. It is possible some other time it does the opposite or 
>>>>>>> some
>>>>>>> other platform does the opposite. I have supposed it is harder to
>>>>>>> synthesise italic than to do 'over-strike'. And this GDI usage
>>>>>>> applies only to LCD glyphs.
>>>>>>>
>>>>>>> Maybe what we need to do is see if we can detect the cases when
>>>>>>> GDI and JDK  disagree on the actual font and remap the glyph id.
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>> On 7/15/15 4:12 AM, Andrew Brygin wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>>  could you please review a fix for 8078382?
>>>>>>>>
>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8078382
>>>>>>>> webrev: http://cr.openjdk.java.net/~bae/8078382/9/webrev.00/
>>>>>>>>
>>>>>>>>  The problem is caused by following peculiarity of the Code New
>>>>>>>>  Roman font: this font provides plain, italic and bold variants.
>>>>>>>>  In bold and italic variants of the font, different glyphs
>>>>>>>>  correspond to the apostrophe character (0039):
>>>>>>>> bold: 0039 -> 0x250 (592)
>>>>>>>> italic: 0039 -> 0x256 (598)
>>>>>>>>
>>>>>>>>  So, we translate character to glyphs using italic variant
>>>>>>>>  of the font, and then request glyph images from GDI.
>>>>>>>>  However, GDI uses the bold variant of the font in order
>>>>>>>>  to compose glyph images for artificial bold-italic variant,
>>>>>>>>  and we have got a glyph image for ® instead of apostrophe.
>>>>>>>>
>>>>>>>>  Suggested fix is to select bold variant (if possible) as a
>>>>>>>>  base for artificial bold-italic.
>>>>>>>>
>>>>>>>>  There is no regression test because it requires a specific font
>>>>>>>>  to be installed on a test system. The font can be found here:
>>>>>>>>  http://www.dafont.com/code-new-roman.font
>>>>>>>>
>>>>>>>> Please take a look.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Andrew
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
>