Apache OpenOffice (AOO) Bugzilla – Issue 89545
Too large mnDescent for some fonts on Mac OS X
Last modified: 2008-07-22 09:45:41 UTC
Some fonts on Mac OS X are rendered with large descent size, and it causes these fonts aligned upward. For example "Poplar Std", "ROSEWOOD STD REGULAR" in the attached figures (fontmenu-non-patched.png and fontmenu-patched.png). This symptom applies for Hiragino fonts, which is the default UI fonts for ja version, and this causes text rendered in UI dialog a bit funny (ja-ui-non-patched.png and ja-ui-patched.png). @hdu: Is there any particular reason that aMetrics.leading is currently added for pMetric->mnDescent? The figures with patched one are created with following diff. Index: salgdi.cxx =================================================================== RCS file: /cvs/gsl/vcl/aqua/source/gdi/salgdi.cxx,v retrieving revision 1.68.32.6 diff -u -r1.68.32.6 salgdi.cxx --- salgdi.cxx 8 May 2008 16:13:47 -0000 1.68.32.6 +++ salgdi.cxx 16 May 2008 06:29:25 -0000 @@ -1412,8 +1412,8 @@ // please see the comment in AquaSalGraphics::SetFont() for details const double fPixelSize = (mfFontScale * mfFakeDPIScale * fPointSize); pMetric->mnAscent = static_cast<long>(+aMetrics.ascent * fPixelSize + 0.5); - pMetric->mnDescent = static_cast<long>((-aMetrics.descent + aMetrics.leading) * fPixelSize + 0.5); - pMetric->mnIntLeading = 0; + pMetric->mnDescent = static_cast<long>(-aMetrics.descent * fPixelSize + 0.5); + pMetric->mnIntLeading = static_cast<long>(aMetrics.leading * fPixelSize + 0.5); pMetric->mnExtLeading = 0; // ATSFontMetrics.avgAdvanceWidth is obsolete, so it is usually set to zero // since ImplFontMetricData::mnWidth is only used for stretching/squeezing fonts
Created attachment 53699 [details] font menu (non-patched)
Created attachment 53700 [details] font menu (patched)
Created attachment 53701 [details] UI fonts (ja version, non-patched)
Created attachment 53702 [details] UI fonts (ja version, patched)
I found that vcl/aqua/source/gdi/salgdi.cxx revision 1.66.6.4 changed the behavior. This revision disabled CJK heuristics, and you seemed to added aMetrics.leading to mnDescent to get adequate line height. However, adding leading to mnDescent doesn't seem to be a good idea for fonts like Hiragino. So, how about adding half of aMetrics.leading to mnAscent and rest half to mnDescent like this? Index: salgdi.cxx =================================================================== RCS file: /cvs/gsl/vcl/aqua/source/gdi/salgdi.cxx,v retrieving revision 1.68.32.6 diff -u -r1.68.32.6 salgdi.cxx --- salgdi.cxx 8 May 2008 16:13:47 -0000 1.68.32.6 +++ salgdi.cxx 16 May 2008 10:02:28 -0000 @@ -1411,8 +1411,8 @@ // convert quartz units to pixel units // please see the comment in AquaSalGraphics::SetFont() for details const double fPixelSize = (mfFontScale * mfFakeDPIScale * fPointSize); - pMetric->mnAscent = static_cast<long>(+aMetrics.ascent * fPixelSize + 0.5); - pMetric->mnDescent = static_cast<long>((-aMetrics.descent + aMetrics.leading) * fPixelSize + 0.5); + pMetric->mnAscent = static_cast<long>((+aMetrics.ascent + aMetrics.leading / 2) * fPixelSize + 0.5); + pMetric->mnDescent = static_cast<long>((-aMetrics.descent + aMetrics.leading / 2) * fPixelSize + 0.5); pMetric->mnIntLeading = 0; pMetric->mnExtLeading = 0; // ATSFontMetrics.avgAdvanceWidth is obsolete, so it is usually set to zero
Thanks for looking into this! I forgot all about that change. AFAIR it improved the QuartzFixed- >VCLPixel rounding quite a bit. What do you think of this change below? - pMetric->mnDescent = static_cast<long>((-aMetrics.descent + aMetrics.leading) * fPixelSize + 0.5); + pMetric->mnDescent = static_cast<long>(-aMetrics.descent * fPixelSize + 0.5); pMetric->mnIntLeading = 0; - pMetric->mnExtLeading = 0; + const long nFullDescent = static_cast<long>((-aMetrics.descent + aMetrics.leading ) * fPixelSize + 0.5); + pMetric->mnExtLeading = nFullDescent - pMetric->mnDescent;
I'm not sure whether we need to add mnExtLeading. Personally, mnExtLeading = 0 is fine for me even with Hiragino fonts. The problem I can think of using mnAscent and mnDescent with the value pMetric->mnAscent = static_cast<long>(+aMetrics.ascent * fPixelSize + 0.5); pMetric->mnDescent = static_cast<long>(-aMetrics.descent * fPixelSize + 0.5); for Hiragino fonts is that the messages in dialogs looks a little bit condensed for ja version (see attached option-dialog-ja.png; it uses Hiragino Kaku Gothic).
Created attachment 53711 [details] dialog with ja version
> What do you think of this change below? After some trial of editing documents and spreadsheets with your change last weekend, I become think of the change is working fine. Adding mnExtLeading seems good compromise in regard of adding appropriate line spaces in the documents with Hiragino fonts. It would be great if you commit your change for 3.0. Also it would be fine if the UI dialog had additional one pixel or so around the text as I noted in the last comment. But this maybe another problem to address. Thanks.
Applied in CWS aquavcl08. Thanks for analyzing the problem and testing!
Verified by ekato. The related issue 89844 in CWS hb09 is expected in the master workspace soon.
Got into DEV300_m23 and BEB300_m3 => closing
now closing for real