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

[Sergey Bylokhov]

Hi, Phil.
I was under impression that jdk6 use the same java runtime support, 
because it has the same issue and because of this e-mail.
http://mail.openjdk.java.net/pipermail/macosx-port-dev/2012-November/005156.html
We can use workaround and catch possible future fix by the new test. I 
have just wanted to point to the problem.

On 23.05.2013 1:51, Phil Race wrote:
> 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.
>>>>>
>>>>
>>>
>>
>>
>


-- 
Best regards, Sergey.