[8] Request for review: 7190349 [macosx] Text (Label) in a JTabbedPane is incorrectly drawn

[Phil Race]

Sergey,

Mike is the one with picture about what's going on inside that API so
what we do will depend on what he thinks can be done there. My take
is that a fix in JRSFontGetAdvancesForGlyphsAndStyle() might be harder 
to deliver,
and I'm not sure how we would would know (at runtime) when its fixed.
In the interim we'd not be able to fix this bug. So lets workaround it and
note the inconsistencies in the code.

I don't know what underlying SPI is being used but that is where the changes
would be needed for JDK 6 which won't benefit from a fix in
JRSFontGetAdvancesForGlyphsAndStyle since that was invented for JDK 7.
And I doubt Apple want to fix JDK 6 now.

-phil.

On 5/22/2013 10:42 AM, Sergey Bylokhov wrote:
> Hi, Phil, Mike.
> Looks like I found the root cause of this bug. It is caused by the 
> different behavior of JRSFontGetAdvancesForGlyphsAndStyle when 
> anti-aliasing is on/off.
> Initial bug exists from the beginning of macosx-port: 
> https://java.net/jira/browse/MACOSX_PORT-47. The root cause of this 
> bug was not fixed(as a fix, all aqua rendering was changed to use 
> antialiasing-on by default).
> So all our components looks good, and custom components looks bad 
> because in jdk6 anti-aliasing is on by default and in jdk 7 is off by 
> default.
>
> Here is a test which fail on jdk6 and jdk7:
> http://cr.openjdk.java.net/~serb/7190349/webrev.00/raw_files/new/test/java/awt/Graphics2D/DrawString/DrawRotatedString.java 
>
> If you add "bg.setRenderingHint(KEY_TEXT_ANTIALIASING, 
> VALUE_TEXT_ANTIALIAS_ON);" to the createBufferedImage() it is passed.
>
> Some technical details:
>  - In both cases we call  CGGlyphImages.m.CGGI_CreateGlyphInfos, which 
> calls JRSFontGetAdvancesForGlyphsAndStyle
>  - JRSFontGetAdvancesForGlyphsAndStyle is called with the same 
> transform, len, glyphs, CTFontRef. Only JRSFontRenderingStyle is 
> different.
>  - JRSFontGetAdvancesForGlyphsAndStyle return inverted values when 
> anti-aliasing is off.
>
> Of course I can invert received AdvancesY, when anti-aliasing is off, 
> but probably it is better to fix JRSFontGetAdvancesForGlyphsAndStyle 
> and cover jdk6 and jdk7 in the same time?
>
> Thanks.
>
> On 22.05.2013 3:47, Phil Race wrote:
>> Sergey,
>>
>> This needs another look.  Although it fixed some clearly broken cases,
>> there is a rotated tab pane label option in SwingSet2, and it turns
>> out that at least for me that was working *before* the fix and now
>> its broken. So I suppose that there are 2 pieces of code interpreting
>> the sign of the advance with different expectations.
>>
>> Since only Aqua does that rotation for the tab pane label, I think the
>> special case is there .. where it likely expected the original CT
>> coordinate system.
>>
>> The bug report in the test case uses custom rendering so
>> won't fall into that case as had seemed likely from the synopsis.
>>
>> -phil.
>>
>> On 5/21/2013 2:27 PM, Phil Race wrote:
>>> I think this fix is fine. In the other rasteriser cases we use
>>> (T2K, freetype) we also  flip the sign of the y advance as it is stored
>>> as the image y advance. It looks like this was just missing here.
>>> This change should then only affect the final glyph rendering.
>>>
>>> -phil.
>>>
>>> On 5/21/2013 12:40 PM, Sergey Bylokhov wrote:
>>>> Hello,
>>>> Please review the fix for jdk 8.
>>>> On OSX advanceY in the glyphInfo is inverted.
>>>> It is used to increment position when the glyph is drawn as part of 
>>>> a string of text:
>>>> @see DrawGlyphList.c.setupBlitVector():
>>>>     ....
>>>>     for (g=0; g<len; g++) {
>>>>         ginfo = (GlyphInfo*)imagePtrs[g];
>>>>         .....
>>>>         FLOOR_ASSIGN(gbv->glyphs[g].y, y + ginfo->topLeftY);
>>>>         .....
>>>>         y += ginfo->advanceY;
>>>>     }
>>>>
>>>> advances are initialized in the CGGlyphImages.m via 
>>>> JRSFontGetAdvancesForGlyphsAndStyle().
>>>> Note that JRSFontGetAdvancesForGlyphsAndStyle use "strike->fTx" and 
>>>> the origin of this transform is relative to the bottom-left corner 
>>>> baseline. So we should pass "strike->fAltTx" to the 
>>>> JRSFontGetAdvancesForGlyphsAndStyle(Not sure is it ok or not) OR to 
>>>> invert the received advance.height.
>>>>
>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7190349
>>>> Webrev can be found at: 
>>>> http://cr.openjdk.java.net/~serb/7190349/webrev.00
>>>>
>>>> -- 
>>>> Best regards, Sergey.
>>>>
>>>
>>
>
>