[OpenJDK 2D-Dev] [8] Request for review: 7190349 and 8013569.

[Mike Swingler]

On Jul 2, 2013, at 4:29 PM, Phil Race <philip.race at oracle.com> wrote:

> On 7/1/2013 7:57 AM, Sergey Bylokhov wrote:
> 
>> Hello.
>> Can you take a look to the fix for:
>> 7190349 : [macosx] Text (Label) is incorrectly drawn with a rotated g2d
>> 8013569 : [macosx] JLabel preferred size incorrect on retina displays with non-default font size
>> 
>> 1 JRS function from Java Runtime Support is replaced by the CTFontGetAdvancesForGlyphs. See http://mail.openjdk.java.net/pipermail/2d-dev/2013-May/003360.html
> 
> This is fine with me, although we have an odd mixture of JRS and non JRS functions here and
> as you and others have asked or implied, I am not sure why we need any JRS
> functions in this part of the code. What do they do that we can't find from
> public API ? However doing more here is for another day.

The public API has no concept of integral metrics. the CTFontGetTransformedAdvancesForGlyphsAndStyle() will return glyph advances that are aligned to the closest integral boundary, given the right style mask, but what this means in Retina is...odd...which is probably why this never became public API.

>> 2 The all methods, which try to get advances in the CStrile.java now is implemented via getGlyphAdvance()
>> 3 Now we apply font transformation on the native lvl, not on java lvl.
> 
>> fDevTx = CGAffineTransformInvert(invDevTx);
> 
> Maybe you explained this before, but why don't
> we have the dev tx anyway? Seems odd to recreate
> it from an inverse tx.
> 
> 
>> 4 When the Fractional metrics is off, we use round in the user's space. Before the fix, we used round in user's space in font metrics calculation, and we used round in device space in GlyphInfo.
> 
> Being consistent is the right thing do to, although we'll have to see
> if anything else was relying on the difference.
> 
> -phil.
>> 
>> Bugs:
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7190349
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8013569
>> 
>> Webrev can be found at:
>> http://cr.openjdk.java.net/~serb/7190349/webrev.04
>> Any suggestions and testing are welcome.

Regards,
Mike Swingler
Apple Inc.