Bug 26120 – [Cairo] Honor Desired Weight and Italic State for Fonts in Windows Cairo Build

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebKit Misc.

[Brent Fulgham]

The attached patch from Joonghoon Kim modifes FontCacheWin.cpp so that the HFONT is created with the desired weight and italic state.

The "matchImprovingEnumProc" method is also modified to take weight and italic state into account.

[Brent Fulgham]

Created attachment 30837 [details]
Honor weight/italic in HFONT structure

Proposed change from Joonghoon Kim to handle font weight and italic state under the Cairo build of Windows.

[Brent Fulgham]

The patch seems valid, but I think Dave Hyatt should review and comment on this.

I'm not sure if the changes to matchImprovingEnumProc are valid/desirable.  I'm also not sure why the standard CG font handling doesn't make use of the weight and italic members of the HFONT structure.  It seems like this should not have to be conditionalized for PLATFORM(CAIRO) at all.

[Eric Seidel (no email)]

Hyatt is really your man here.

[Dave Hyatt]

Comment on attachment 30837 [details]
Honor weight/italic in HFONT structure

Hard for me to plus this one without being sure about what it's changing.  Can this maybe be elaborated upon?

[Brent Fulgham]

The changes in this bug were proposed by a team using WebKit on some kind of mobile computing device.  They apparently ran into problems with bold and italic fonts (Korean fonts IIRC) not displaying properly.  They found that the following change (around line 440) provided proper font display:

#if PLATFORM(CAIRO)
     matchData.m_chosen.lfWeight = desiredWeight;
     matchData.m_chosen.lfItalic = desiredItalic;
#endif 

The underlying problem may be completely resolved by Bug 21492, which is already in the tree.

Mitz separately wrote me:
> In the CG port, we need the LOGFONT and resulting HFONT to correspond to an actual font,
> not a synthesized one, so that we can apply synthetic oblique and synthetic boldface in the
> engine when rendering with CG (see createFontPlatformData()).

The original reporter indicated that the changes to the matchImprovingEnumProc were done to help get exact matching once they added the BOLD/ITALIC state to the HFONT, since otherwise it iterates over every font structure proposed by GDI.

I see two possible states for this issue:
(1) If the fix in Bug 21492 provides correct rendering, then this issue is unneeded and we can close it out.
(2) If Bug 21492 did NOT correct the problem, then we probably want to make sure that the HFONT settings are included with the guards, so the bold/italic state is properly synthesized in the CG implementation.

(In reply to comment #5)
> The original reporter indicated that the changes to the matchImprovingEnumProc
> were done to help get exact matching once they added the BOLD/ITALIC state to
> the HFONT, since otherwise it iterates over every font structure proposed by
> GDI.

That’s the part of the change that I didn’t understand. While it’s true that the current code iterates over all candidates, it still prefers the closest match, including an exact match if there is one. The change looks like it may be an optimization attempt (stop iterating if the best match is an exact match), but I am not sure it’s warranted.

[Joonghoon Kim]

r44268 from Bug 21492 corrected the problem of Korean bold font rendering. 
Thank you. 

The changes to the matchImprovingEnumProc() is just a little optimization. 

Currently, matchImprovingEnumProc enumerates all fonts proposed by GDI.
If it don't need to improve anything other than italic & weight, it can
be stopped when it's proposed same weight & italic by GDI. 

Sorry for the bugs in the changes. First if block need to be modified 
to return 1 when firstMatch is true to keep original logic. 
(candidate is always chosen in first match) And, in second if block, 
the change removes all '!' operators comparing lfitalic fields, 
but it is my mistake. The original code implies much more complex 
(and hard to understand) logic. 

I think it's better to keep current matchImprovingEnumProc() if there's
no clear evidence it slows down font creation. 

But, about the other changes to creatGDIFont(), I still wonder why we
have to embolden manually' during font rendering. Cairo looks to be able
to handle synthetic bold rendering by itself.  

[Joonghoon Kim]

r44268 from Bug 21492 corrected the problem of Korean bold font rendering. 
Since most of Korean fonts (bundled with MS Windows) don't have bold data,   
they should be rendered as synthetic-bold. 

The changes to the matchImprovingEnumProc() is just a little optimization. 

Currently, matchImprovingEnumProc enumerates all fonts proposed by GDI.
If it don't need to improve anything other than italic & weight, it can
be stopped when it's proposed same weight & italic by GDI. 

Sorry for the bugs in the changes. First if block need to be modified 
to return 1 when firstMatch is true to keep original logic. 
(candidate is always chosen in first match) And, in second if block, 
the change removes all '!' operators comparing lfitalic fields, 
but it is my mistake. The original code implies much more complex 
(and hard to understand) logic. 

I think it's better to keep current matchImprovingEnumProc() if there's
no clear evidence it slows down font creation. 

But, about the other changes to creatGDIFont(), 
I'm still wonder why we have to embolden fonts manually. 
I changed the createGDIFont() before r44268 landed, and saw it works. 
That means cairo can draw synthtic-bold fonts by itself. 
Is there any reason that I don't know yet? 

Anyway, This issue can be closed. Thank you.  

[Brent Fulgham]

Per original reporters request, closing this issue as resolved by Bug 21492.

*** This bug has been marked as a duplicate of bug 21492 ***

[Eric Seidel (no email)]

Comment on attachment 30837 [details]
Honor weight/italic in HFONT structure

Since this is marked fix, clearing review flag.