21853 2008-10-24 06:21:16 -0700 text drawing on WX port on OSX is a few pixels off 2008-10-24 18:16:05 -0700 1 1 1 Unclassified WebKit WebKit wx 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED P2 Normal --- 0 kevinwatters webkit-unassigned oldest_to_newest 96500 0 kevinwatters 2008-10-24 06:21:16 -0700 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. 96501 1 24639 kevinwatters 2008-10-24 06:23:13 -0700 Created attachment 24639 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. 96512 2 24639 kevino 2008-10-24 09:30:16 -0700 Comment on attachment 24639 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. 96631 3 kevino 2008-10-24 18:16:05 -0700 Landed in r37877, thanks! :-) 24639 2008-10-24 06:23:13 -0700 2008-10-24 09:30:16 -0700 Use CGFont API for better metrics and fix y offset of text wxmac_text_fixes.patch text/plain 4037 kevinwatters SW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vd3gvd3hjb2RlL21hYy9jYXJib24vbm9uLWtlcm5lZC1k cmF3aW5nLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL3d4L3d4Y29kZS9tYWMv Y2FyYm9uL25vbi1rZXJuZWQtZHJhd2luZy5jcHAJKHJldmlzaW9uIDM3ODM5KQorKysgV2ViQ29y ZS9wbGF0Zm9ybS93eC93eGNvZGUvbWFjL2NhcmJvbi9ub24ta2VybmVkLWRyYXdpbmcuY3BwCSh3 b3JraW5nIGNvcHkpCkBAIC01NywxMSArNTcsMTAgQEAKICAgICAgICAgb2Zmc2V0ICs9IGdseXBo QnVmZmVyLmFkdmFuY2VBdChmcm9tICsgaSk7CiAgICAgfQogICAgIAotICAgIC8vIHRoZSB5IHBv aW50IGlzIGFjdHVhbGx5IHRoZSBib3R0b20gcG9pbnQgb2YgdGhlIHRleHQsIHR1cm4gaXQgaW50 byB0aGUgdG9wCi0gICAgZmxvYXQgaGVpZ2h0ID0gZm9udC0+YXNjZW50KCkgLSBmb250LT5kZXNj ZW50KCk7Ci0gICAgd3hDb29yZCB5cG9pbnQgPSAod3hDb29yZCkgKHBvaW50LnkoKSAtIGhlaWdo dCk7Ci0gICAgIAotICAgIGRjLT5EcmF3VGV4dCh0ZXh0LCAod3hDb29yZClwb2ludC54KCksIHlw b2ludCk7CisgICAgLy8gTk9URTogVGhlIHd4IEFQSSBhY3R1YWxseSBhZGRzIHRoZSBhc2NlbnQg dG8gdGhlIHkgdmFsdWUgaW50ZXJuYWxseSwKKyAgICAvLyBzbyB3ZSBoYXZlIHRvIHN1YnRyYWN0 IGl0IGZyb20gdGhlIHkgcG9pbnQgaGVyZSBzbyB0aGF0IHRoZSBhc2NlbnQKKyAgICAvLyBpc24n dCBhZGRlZCB0d2ljZS4gCisgICAgZGMtPkRyYXdUZXh0KHRleHQsICh3eENvb3JkKXBvaW50Lngo KSwgaW50KHBvaW50LnkoKSAtIGZvbnQtPmFzY2VudCgpKSk7CiB9CiAKIH0KSW5kZXg6IFdlYkNv cmUvcGxhdGZvcm0vd3gvd3hjb2RlL21hYy9jYXJib24vZm9udHByb3BzLmNwcAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL3d4L3d4Y29kZS9tYWMvY2FyYm9uL2ZvbnRwcm9wcy5jcHAJ KHJldmlzaW9uIDM3ODM5KQorKysgV2ViQ29yZS9wbGF0Zm9ybS93eC93eGNvZGUvbWFjL2NhcmJv bi9mb250cHJvcHMuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0yMyw5ICsyMyw3IEBACiAgKiBPRiBU SElTIFNPRlRXQVJFLCBFVkVOIElGIEFEVklTRUQgT0YgVEhFIFBPU1NJQklMSVRZIE9GIFNVQ0gg REFNQUdFLgogICovCiAKLSNpZiBfX1dYTUFDX18KICNpbmNsdWRlIDxBcHBsaWNhdGlvblNlcnZp Y2VzL0FwcGxpY2F0aW9uU2VydmljZXMuaD4KLSNlbmRpZgogCiAjaW5jbHVkZSA8d3gvZGVmcy5o PgogI2luY2x1ZGUgPHd4L2dkaWNtbi5oPgpAQCAtMzksMjMgKzM3LDM4IEBACiBtX2FzY2VudCgw KSwgbV9kZXNjZW50KDApLCBtX2xpbmVHYXAoMCksIG1fbGluZVNwYWNpbmcoMCksIG1feEhlaWdo dCgwKQogewogICAgIEFUU0ZvbnRSZWYgZm9udFJlZjsKKyAgICBDR0ZvbnRSZWYgY2dGb250Owog ICAgIAogICAgIGZvbnRSZWYgPSBGTUdldEFUU0ZvbnRSZWZGcm9tRm9udChmb250LT5NYWNHZXRB VFNVRm9udElEKCkpOwotICAgIGlmIChmb250UmVmKXsKLSAgICAgICAgQVRTRm9udE1ldHJpY3Mg dk1ldHJpY3M7Ci0gICAgICAgIE9TU3RhdHVzIGVycjsKKyAgICAKKyAgICBpZiAoZm9udFJlZikK KyAgICAgICAgY2dGb250ID0gQ0dGb250Q3JlYXRlV2l0aFBsYXRmb3JtRm9udCgodm9pZCopJmZv bnRSZWYpOwogICAgICAgICAKLSAgICAgICAgaW50IGhlaWdodCA9IGZvbnQtPkdldFBvaW50U2l6 ZSgpOyAvLy5HZXRIZWlnaHQoKTsKLSAgICAgICAgZXJyID0gQVRTRm9udEdldEhvcml6b250YWxN ZXRyaWNzKGZvbnRSZWYsIDAsICZ2TWV0cmljcyk7Ci0gICAgICAgIGlmIChlcnIgIT0gbm9FcnIp Ci0gICAgICAgICAgICBmcHJpbnRmKHN0ZGVyciwgIkVycm9yIG51bWJlciBpcyAlZFxuIiwgZXJy KTsKLSAgICAgICAgbV9hc2NlbnQgPSBscm91bmRmKHZNZXRyaWNzLmFzY2VudCAqIGhlaWdodCk7 Ci0gICAgICAgIG1fZGVzY2VudCA9IGxyb3VuZGYodk1ldHJpY3MuZGVzY2VudCAqIGhlaWdodCk7 Ci0gICAgICAgIG1fbGluZUdhcCA9IGxyb3VuZGYodk1ldHJpY3MubGVhZGluZyAqIGhlaWdodCk7 CisgICAgaWYgKGNnRm9udCkgeworICAgICAgICBpbnQgaUFzY2VudDsKKyAgICAgICAgaW50IGlE ZXNjZW50OworICAgICAgICBpbnQgaUxpbmVHYXA7CisgICAgICAgIGZsb2F0IHVuaXRzUGVyRW07 CisjaWZkZWYgQlVJTERJTkdfT05fVElHRVIKKyAgICAgICAgd2tHZXRGb250TWV0cmljcyhjZ0Zv bnQsICZpQXNjZW50LCAmaURlc2NlbnQsICZpTGluZUdhcCwgJnVuaXRzUGVyRW0pOworI2Vsc2UK KyAgICAgICAgaUFzY2VudCA9IENHRm9udEdldEFzY2VudChjZ0ZvbnQpOworICAgICAgICBpRGVz Y2VudCA9IENHRm9udEdldERlc2NlbnQoY2dGb250KTsKKyAgICAgICAgaUxpbmVHYXAgPSBDR0Zv bnRHZXRMZWFkaW5nKGNnRm9udCk7CisgICAgICAgIHVuaXRzUGVyRW0gPSBDR0ZvbnRHZXRVbml0 c1BlckVtKGNnRm9udCk7CisjZW5kaWYKKyAgICAgICAgZmxvYXQgcG9pbnRTaXplID0gZm9udC0+ R2V0UG9pbnRTaXplKCk7CisgICAgICAgIGZsb2F0IGZBc2NlbnQgPSBzY2FsZUVtVG9Vbml0cyhp QXNjZW50LCB1bml0c1BlckVtKSAqIHBvaW50U2l6ZTsKKyAgICAgICAgZmxvYXQgZkRlc2NlbnQg PSAtc2NhbGVFbVRvVW5pdHMoaURlc2NlbnQsIHVuaXRzUGVyRW0pICogcG9pbnRTaXplOworICAg ICAgICBmbG9hdCBmTGluZUdhcCA9IHNjYWxlRW1Ub1VuaXRzKGlMaW5lR2FwLCB1bml0c1BlckVt KSAqIHBvaW50U2l6ZTsKKworICAgICAgICBtX2FzY2VudCA9IGxyb3VuZGYoZkFzY2VudCk7Cisg ICAgICAgIG1fZGVzY2VudCA9IGxyb3VuZGYoZkRlc2NlbnQpOworICAgICAgICBtX2xpbmVHYXAg PSBscm91bmRmKGZMaW5lR2FwKTsKICAgICAgICAgd3hDb29yZCB4SGVpZ2h0ID0gMDsKICAgICAg ICAgR2V0VGV4dEV4dGVudCgqZm9udCwgd3hUKCJ4IiksIE5VTEwsICZ4SGVpZ2h0LCBOVUxMLCBO VUxMKTsKICAgICAgICAgbV94SGVpZ2h0ID0gbHJvdW5kZih4SGVpZ2h0KTsKLSAgICAgICAgbV9s aW5lU3BhY2luZyA9IG1fYXNjZW50IC0gbV9kZXNjZW50ICsgbV9saW5lR2FwOworICAgICAgICBt X2xpbmVTcGFjaW5nID0gbV9hc2NlbnQgKyBtX2Rlc2NlbnQgKyBtX2xpbmVHYXA7CiAKICAgICB9 CiAKSW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdl TG9nCShyZXZpc2lvbiAzNzg0MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkp CkBAIC0xLDMgKzEsMTYgQEAKKzIwMDgtMTAtMjMgIEtldmluIFdhdHRlcnMgIDxrZXZpbndhdHRl cnNAZ21haWwuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFVzZSB0aGUgQ0dGb250IG1ldHJpY3MgQVBJcyBmb3IgbW9yZSBhY2N1cmF0ZSBtZWFz dXJlbWVudHMsIGFuZCB0d2VhayB0aGUgeQorICAgICAgICB2YWx1ZSB0aGUgdGV4dCBpcyBkcmF3 biBhdCAoaXQgd2FzIGEgY291cGxlIHBpeGVscyBvZmYgYmVmb3JlIGJlY2F1c2Ugd3gKKyAgICAg ICAgaW50ZXJuYWxseSBhZGRzIHRvIHRoZSB5IHZhbHVlLgorCisgICAgICAgICogcGxhdGZvcm0v d3gvd3hjb2RlL21hYy9jYXJib24vZm9udHByb3BzLmNwcDoKKyAgICAgICAgKHd4Rm9udFByb3Bl cnRpZXM6Ond4Rm9udFByb3BlcnRpZXMpOgorICAgICAgICAqIHBsYXRmb3JtL3d4L3d4Y29kZS9t YWMvY2FyYm9uL25vbi1rZXJuZWQtZHJhd2luZy5jcHA6CisgICAgICAgIChXZWJDb3JlOjpkcmF3 VGV4dFdpdGhTcGFjaW5nKToKKwogMjAwOC0xMC0yMyAgR3JlZyBCb2xzaW5nYSAgPGJvbHNpbmdh QGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBTYW0gV2VpbmlnLgo=