Bug 21853 - text drawing on WX port on OSX is a few pixels off
Summary: text drawing on WX port on OSX is a few pixels off
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit wx (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-24 06:21 PDT by Kevin Watters
Modified: 2008-10-24 18:16 PDT (History)
0 users

See Also:


Attachments
Use CGFont API for better metrics and fix y offset of text (3.94 KB, patch)
2008-10-24 06:23 PDT, Kevin Watters
kevino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Watters 2008-10-24 06:21:16 PDT
Font rendering on OS X in the wx port doesn't account for adjustments made internally in WX, resulting in text that's a few pixels off.
Comment 1 Kevin Watters 2008-10-24 06:23:13 PDT
Created attachment 24639 [details]
Use CGFont API for better metrics and fix y offset of text

This patch uses the CGFont API to obtain better measurements for text rendering on OS X. It also tweaks the y value text is drawn at.
Comment 2 Kevin Ollivier 2008-10-24 09:30:16 PDT
Comment on attachment 24639 [details]
Use CGFont API for better metrics and fix y offset of text

> Index: WebCore/platform/wx/wxcode/mac/carbon/non-kerned-drawing.cpp
> ===================================================================
> --- WebCore/platform/wx/wxcode/mac/carbon/non-kerned-drawing.cpp	(revision 37839)
> +++ WebCore/platform/wx/wxcode/mac/carbon/non-kerned-drawing.cpp	(working copy)
> @@ -57,11 +57,10 @@
>          offset += glyphBuffer.advanceAt(from + i);
>      }
>      
> -    // the y point is actually the bottom point of the text, turn it into the top
> -    float height = font->ascent() - font->descent();
> -    wxCoord ypoint = (wxCoord) (point.y() - height);
> -     
> -    dc->DrawText(text, (wxCoord)point.x(), ypoint);
> +    // NOTE: The wx API actually adds the ascent to the y value internally,
> +    // so we have to subtract it from the y point here so that the ascent
> +    // isn't added twice. 
> +    dc->DrawText(text, (wxCoord)point.x(), int(point.y() - font->ascent()));
>  }
>  
>  }
> Index: WebCore/platform/wx/wxcode/mac/carbon/fontprops.cpp
> ===================================================================
> --- WebCore/platform/wx/wxcode/mac/carbon/fontprops.cpp	(revision 37839)
> +++ WebCore/platform/wx/wxcode/mac/carbon/fontprops.cpp	(working copy)
> @@ -23,9 +23,7 @@
>   * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> -#if __WXMAC__
>  #include <ApplicationServices/ApplicationServices.h>
> -#endif
>  
>  #include <wx/defs.h>
>  #include <wx/gdicmn.h>
> @@ -39,23 +37,38 @@
>  m_ascent(0), m_descent(0), m_lineGap(0), m_lineSpacing(0), m_xHeight(0)
>  {
>      ATSFontRef fontRef;
> +    CGFontRef cgFont;
>      
>      fontRef = FMGetATSFontRefFromFont(font->MacGetATSUFontID());
> -    if (fontRef){
> -        ATSFontMetrics vMetrics;
> -        OSStatus err;
> +    
> +    if (fontRef)
> +        cgFont = CGFontCreateWithPlatformFont((void*)&fontRef);
>          
> -        int height = font->GetPointSize(); //.GetHeight();
> -        err = ATSFontGetHorizontalMetrics(fontRef, 0, &vMetrics);
> -        if (err != noErr)
> -            fprintf(stderr, "Error number is %d\n", err);
> -        m_ascent = lroundf(vMetrics.ascent * height);
> -        m_descent = lroundf(vMetrics.descent * height);
> -        m_lineGap = lroundf(vMetrics.leading * height);
> +    if (cgFont) {
> +        int iAscent;
> +        int iDescent;
> +        int iLineGap;
> +        float unitsPerEm;
> +#ifdef BUILDING_ON_TIGER
> +        wkGetFontMetrics(cgFont, &iAscent, &iDescent, &iLineGap, &unitsPerEm);
> +#else
> +        iAscent = CGFontGetAscent(cgFont);
> +        iDescent = CGFontGetDescent(cgFont);
> +        iLineGap = CGFontGetLeading(cgFont);
> +        unitsPerEm = CGFontGetUnitsPerEm(cgFont);
> +#endif
> +        float pointSize = font->GetPointSize();
> +        float fAscent = scaleEmToUnits(iAscent, unitsPerEm) * pointSize;
> +        float fDescent = -scaleEmToUnits(iDescent, unitsPerEm) * pointSize;
> +        float fLineGap = scaleEmToUnits(iLineGap, unitsPerEm) * pointSize;
> +
> +        m_ascent = lroundf(fAscent);
> +        m_descent = lroundf(fDescent);
> +        m_lineGap = lroundf(fLineGap);
>          wxCoord xHeight = 0;
>          GetTextExtent(*font, wxT("x"), NULL, &xHeight, NULL, NULL);
>          m_xHeight = lroundf(xHeight);
> -        m_lineSpacing = m_ascent - m_descent + m_lineGap;
> +        m_lineSpacing = m_ascent + m_descent + m_lineGap;
>  
>      }
>  
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 37840)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,16 @@
> +2008-10-23  Kevin Watters  <kevinwatters@gmail.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Use the CGFont metrics APIs for more accurate measurements, and tweak the y
> +        value the text is drawn at (it was a couple pixels off before because wx
> +        internally adds to the y value.
> +
> +        * platform/wx/wxcode/mac/carbon/fontprops.cpp:
> +        (wxFontProperties::wxFontProperties):
> +        * platform/wx/wxcode/mac/carbon/non-kerned-drawing.cpp:
> +        (WebCore::drawTextWithSpacing):
> +
>  2008-10-23  Greg Bolsinga  <bolsinga@apple.com>
>  
>          Reviewed by Sam Weinig.
Comment 3 Kevin Ollivier 2008-10-24 18:16:05 PDT
Landed in r37877, thanks! :-)