[OpenJDK 2D-Dev] [8] Request for review: 7190349 [macosx] Text (Label) in a JTabbedPane is incorrectly drawn

[Sergey Bylokhov]

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.