RESOLVED FIXED 21853
text drawing on WX port on OSX is a few pixels off
https://bugs.webkit.org/show_bug.cgi?id=21853
Summary text drawing on WX port on OSX is a few pixels off
Kevin Watters
Reported 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.
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+
Kevin Watters
Comment 1 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.
Kevin Ollivier
Comment 2 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.
Kevin Ollivier
Comment 3 2008-10-24 18:16:05 PDT
Landed in r37877, thanks! :-)
Note You need to log in before you can comment on or make changes to this bug.