[OpenJDK 2D-Dev] RFR: 8214002 Cannot use italic font style if the font has embeded bitmap

[Ichiroh Takiguchi]

Hello Phil.

Test case is available.
Could you review the fix again ?

Bug:    https://bugs.openjdk.java.net/browse/JDK-8214002
Change: https://cr.openjdk.java.net/~itakiguchi/8214002/webrev.02/

Thanks,
Ichiroh Takiguchi

On 2018-11-28 10:13, Philip Race wrote:
> Oh .. there's no regression test. If you can't find one then I think 
> you
> can write one relatively easily by selecting the known font - MS 
> Mincho,
> making sure we are on Windows (don't try it on Mac even if the font
> exists), draw the text to a BufferedImage in plain / regular style.
> Repeat to a different BufferedImage in italic
> Compare the images, and pass only if they differ.
> 
> -phil.
> 
> On 11/27/18, 5:08 PM, Ichiroh Takiguchi wrote:
>> Hello Phil.
>>> Do you need me to push this ?
>> Yes, if possible.
>> Currently, no sponsor is assigned for this issue.
>> 
>> Ichiroh Takiguchi
>> 
>> On 2018-11-28 05:43, Phil Race wrote:
>>> On 11/27/18 9:36 AM, Ichiroh Takiguchi wrote:
>>>> Hello Phil.
>>>> 
>>>> I don't have any concern about this fix.
>>>> I'm thinking why initial programmer used FT_LOAD_RENDER instead of 
>>>> FT_LOAD_DEFAULT.
>>> Probably that this was what we wanted to do in almost all cases and 
>>> it has
>>> now turned out to be untrue ..
>>> 
>>>> On my testing, this fix was fine for me.
>>> 
>>> Do you need me to push this ?
>>> 
>>> -phil.
>>> 
>>>> 
>>>> Ichiroh Takiguchi
>>>> 
>>>> On 2018-11-27 03:59, Philip Race wrote:
>>>>> It seems fine to me. What is your concern when you say :
>>>>>> But it may change font rendering behavior...
>>>>> 
>>>>> -phil
>>>>> 
>>>>> On 11/18/18, 9:35 PM, Ichiroh Takiguchi wrote:
>>>>>> Hello Phill.
>>>>>> 
>>>>>> I tested and checked your suggested code. [1]
>>>>>> It worked fine.
>>>>>> But it may change font rendering behavior...
>>>>>> Please review the fix ?
>>>>>> 
>>>>>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8214002
>>>>>> Change: https://cr.openjdk.java.net/~itakiguchi/8214002/webrev.01/
>>>>>> 
>>>>>> Thanks,
>>>>>> Ichiroh Takiguchi
>>>>>> IBM Japan, Ltd.
>>>>>> 
>>>>>> [1] 
>>>>>> https://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html#FT_LOAD_DEFAULT
>>>>>> 
>>>>>> On 2018-11-17 06:04, Phil Race wrote:
>>>>>>> Hi,
>>>>>>> 
>>>>>>> Thanks for spotting this. But I'm not sure the conditions are 
>>>>>>> right.
>>>>>>> Don't we need to unconditionally remove FT_LOAD_RENDER if 
>>>>>>> synthetic styles
>>>>>>> are requested, regardless of the value of that flag ?
>>>>>>> 
>>>>>>> Perhaps we should not set FT_LOAD_RENDER upfront in which case 
>>>>>>> we'll
>>>>>>> just call it once
>>>>>>> we are done, making the logic simpler and avoid all of the error 
>>>>>>> prone "in this
>>>>>>> case we disable it again" logic.
>>>>>>> 
>>>>>>> So instead of your fix just change the initialisation to :
>>>>>>> 
>>>>>>> int renderFlags = FT_LOAD_DEFAULT;
>>>>>>> 
>>>>>>> -phil.
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> On 11/16/18 9:37 AM, Ichiroh Takiguchi wrote:
>>>>>>>> Hello.
>>>>>>>> Could you review the fix ?
>>>>>>>> 
>>>>>>>> Issue:
>>>>>>>> Cannot use italic font style if the font has embeded bitmap.
>>>>>>>> 
>>>>>>>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8214002
>>>>>>>> Change: 
>>>>>>>> https://cr.openjdk.java.net/~itakiguchi/8214002/webrev.00/
>>>>>>>> 
>>>>>>>> It seems it's side-effect for:
>>>>>>>> 8204929: Fonts with embedded bitmaps are not always rotated
>>>>>>>> 
>>>>>>>> Test instruction and screen shot are in JDK-8214002.
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> Ichiroh Takiguchi
>>>>>>>> IBM Japan, Ltd.
>>>>>>>> 
>>>>>>>> On 2018-07-27 20:22, Ichiroh Takiguchi wrote:
>>>>>>>>> Hello.
>>>>>>>>> 
>>>>>>>>> According to my investigation, FT_Render_Glyph() was not called
>>>>>>>>> even if FT_GlyphSlot_Oblique() was called.
>>>>>>>>> 
>>>>>>>>> =========
>>>>>>>>>     if (ftglyph->format == FT_GLYPH_FORMAT_OUTLINE) {
>>>>>>>>>         FT_Render_Glyph(ftglyph, FT_LOAD_TARGET_MODE(target)); 
>>>>>>>>> <<===
>>>>>>>>>     }
>>>>>>>>> =========
>>>>>>>>> 
>>>>>>>>> It seemed FT_Load_Glyph() and renderFlags affected this issue.
>>>>>>>>> 
>>>>>>>>> On my Windows,
>>>>>>>>> For "MS Mincho" with italic, renderFlags was 
>>>>>>>>> "FT_LOAD_TARGET_MONO |
>>>>>>>>> FT_LOAD_NO_BITMAP | FT_LOAD_RENDER".
>>>>>>>>> I also tested "Meiryo" font (it could handle italic style)
>>>>>>>>> For "Meiryo" with italic, renderFlags was "FT_LOAD_TARGET_MONO 
>>>>>>>>> |
>>>>>>>>> FT_LOAD_RENDER".
>>>>>>>>> 
>>>>>>>>> I think, after FT_LOAD_NO_BITMAP is turned on, FT_LOAD_RENDER 
>>>>>>>>> should
>>>>>>>>> be turned off.
>>>>>>>>> So how about following fix ?
>>>>>>>>> 
>>>>>>>>> =========
>>>>>>>>> diff -r 1edcf36fe15f
>>>>>>>>> src/java.desktop/share/native/libfontmanager/freetypeScaler.c
>>>>>>>>> ---
>>>>>>>>> a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c 
>>>>>>>>>    Wed
>>>>>>>>> Jul 18 11:57:51 2018 -0400
>>>>>>>>> +++
>>>>>>>>> b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c 
>>>>>>>>>    Fri
>>>>>>>>> Jul 27 19:44:12 2018 +0900
>>>>>>>>> @@ -700,6 +700,9 @@
>>>>>>>>> 
>>>>>>>>>      if (!context->useSbits) {
>>>>>>>>>          renderFlags |= FT_LOAD_NO_BITMAP;
>>>>>>>>> +        if (context->doBold || context->doItalize) {
>>>>>>>>> +            renderFlags &= ~FT_LOAD_RENDER;
>>>>>>>>> +        }
>>>>>>>>>      }
>>>>>>>>> 
>>>>>>>>>      /* NB: in case of non identity transform
>>>>>>>>> =========
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> On 2018-07-25 19:29, Ichiroh Takiguchi wrote:
>>>>>>>>>> Hello.
>>>>>>>>>> I'm using jdk-11+23 build on Japanese Windows 7.
>>>>>>>>>> 
>>>>>>>>>> I ran Font2DTest Demo, then select like:
>>>>>>>>>> Font: MS Mincho
>>>>>>>>>> Range: Basic Latin
>>>>>>>>>> Method: drawString
>>>>>>>>>> Size: 24
>>>>>>>>>> Style: Italic
>>>>>>>>>> 
>>>>>>>>>> But style was not changed to Italic.
>>>>>>>>>> Antialiasing and Fractional Metrics did not affect this issue.
>>>>>>>>>> 
>>>>>>>>>> I assume it's side-effect for:
>>>>>>>>>> 8204929: Fonts with embedded bitmaps are not always rotated
>>>>>>>>>> 
>>>>>>>>>> Could you check it ?
>>>>>>>>>> 
>>>>>>>>>> Thanks,
>>>>>>>>>> Ichiroh Takiguchi
>>>>>>>>>> IBM Japan, Ltd.
>>>>>>>> 
>>>>>> 
>>>> 
>>