[Webkit-unassigned] [Bug 16768] Position and thickness of underline as text size changes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=16768


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #38787|review+                     |review-
               Flag|                            |




--- Comment #31 from mitz at webkit.org  2009-10-06 16:53:48 PDT ---
(From update of attachment 38787)
> +#ifndef BUILDING_ON_TIGER
> +    float fUnderlinePosition = fAscent - CTFontGetUnderlinePosition(toCTFontRef(m_platformData.font()));
> +    float fUnderlineThickness = CTFontGetUnderlineThickness(toCTFontRef(m_platformData.font()));
> +#endif

I think you need to null-check m_platformData.font() here. But it is also
unnecessary to go through Core Text (and thus exclude Tiger), because -[NSFont
underlinePosition] and -[NSFont underlineThickness] are available in Mac OS X
v10.0 and later.

> +#ifndef BUILDING_ON_TIGER
> +    m_underlinePosition = std::max(lroundf(fUnderlinePosition), m_ascent + 1L);
> +    m_underlineThickness = std::max(lroundf(fUnderlineThickness), 1L);
> +    // Note that we use the default (hard coded) position/thickness on Tiger.
> +    // FIXME: whould be better to update WebKitLibraries/ so that we can get proper metrics on Tiger?
> +#endif

If you use the aforementioned NSFont methods you won’t need to make any changes
for Tiger.

Instead of using 'std::max' you should add 'using namespace std;' near the
beginning of the file and just use 'max' here. I don’t think you need the
#import <algorithm> statement in this file.

There is no need to for the "1L" literal form. You can use roundf() and
max<int>().

I am curious how you arrived at the formulas for m_underlinePosition and
m_underlineThickness. Do they match what AppKit does on Mac OS X? In particular
I don’t think AppKit derives the underline position from the font’s
-underlinePosition.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.