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

[Sergey Bylokhov]

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;
             }
         }

>
> -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
>>>>>>
>>>>>
>>>>
>>>
>>
>


-- 
Best regards, Sergey.