26120 2009-06-01 11:47:12 -0700 [Cairo] Honor Desired Weight and Italic State for Fonts in Windows Cairo Build 2009-07-20 15:04:34 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) PC Windows XP RESOLVED DUPLICATE 21492 P2 Normal --- 1 bfulgham webkit-unassigned hyatt lunaris mitz oldest_to_newest 123859 0 bfulgham 2009-06-01 11:47:12 -0700 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. 123862 1 30837 bfulgham 2009-06-01 11:56:10 -0700 Created attachment 30837 Honor weight/italic in HFONT structure Proposed change from Joonghoon Kim to handle font weight and italic state under the Cairo build of Windows. 123863 2 bfulgham 2009-06-01 11:57:47 -0700 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. 126835 3 eric 2009-06-18 17:42:32 -0700 Hyatt is really your man here. 126909 4 30837 hyatt 2009-06-19 00:56:06 -0700 Comment on attachment 30837 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? 126954 5 bfulgham 2009-06-19 10:10:07 -0700 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. 126955 6 mitz 2009-06-19 10:15:51 -0700 (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. 127211 7 lunaris 2009-06-21 03:59:06 -0700 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. 127212 8 lunaris 2009-06-21 04:30:15 -0700 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. 132272 9 bfulgham 2009-07-16 09:07:58 -0700 Per original reporters request, closing this issue as resolved by Bug 21492. *** This bug has been marked as a duplicate of bug 21492 *** 133246 10 30837 eric 2009-07-20 15:04:34 -0700 Comment on attachment 30837 Honor weight/italic in HFONT structure Since this is marked fix, clearing review flag. 30837 2009-06-01 11:56:10 -0700 2009-07-20 15:04:34 -0700 Honor weight/italic in HFONT structure font_cache_cairo.patch text/plain 3261 bfulgham SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NDMyNSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDktMDYtMDEgIEpvb25naG9vbiBLaW0gPGx1bmFyaXNAdG1heC5j by5rcj4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBC dWcgMjYxMjA6IEhvbm9yIGRlc2lyZWQgd2VpZ2h0IGFuZCBpdGFsaWMgc3RhdGUgZm9yIEZvbnRz CisgICAgICAgIGluIFdpbmRvd3MgQ2Fpcm8gYnVpbGQuCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yNjEyMC4KKworICAgICAgICAqIHBsYXRmb3JtL2dy YXBoaWNzL3dpbi9Gb250Q2FjaGVXaW4uY3BwOiBCdWlsZCBIRk9OVCBzdHJ1Y3R1cmUKKyAgICAg ICAgICB3aXRoIHdlaWdodCBhbmQgaXRhbGljIHN0YXRlIHNwZWNpZmllZC4gIEV4dGVuZCBsb2dp YyBpbiB0aGUKKyAgICAgICAgICBtYXRjaEltcHJvdmluZ0VudW1Qcm9jIHRvIHRha2Ugd2VpZ2h0 IGFuZCBpdGFsaWMgc3RhdGUgaW50bworICAgICAgICAgIGFjY291bnQgZm9yIG1hdGNoLgorCiAy MDA5LTA2LTAxICBEYXZpZCBMZXZpbiAgPGxldmluQGNocm9taXVtLm9yZz4KIAogICAgICAgICBS ZXZpZXdlZCBieSBEYXJpbiBBbGRlciBhbmQgTWFjaWVqIFN0YWNob3dpYWsuCkluZGV4OiBXZWJD b3JlL3BsYXRmb3JtL2dyYXBoaWNzL3dpbi9Gb250Q2FjaGVXaW4uY3BwCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvd2luL0ZvbnRDYWNoZVdpbi5jcHAJKHJldmlzaW9u IDQ0MzI0KQorKysgV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy93aW4vRm9udENhY2hlV2luLmNw cAkod29ya2luZyBjb3B5KQpAQCAtMzU5LDIyICszNTksMzcgQEAgc3RhdGljIGludCBDQUxMQkFD SyBtYXRjaEltcHJvdmluZ0VudW1QcgogewogICAgIE1hdGNoSW1wcm92aW5nUHJvY0RhdGEqIG1h dGNoRGF0YSA9IHJlaW50ZXJwcmV0X2Nhc3Q8TWF0Y2hJbXByb3ZpbmdQcm9jRGF0YSo+KGxQYXJh bSk7CiAKKyAgICBib29sIGZpcnN0TWF0Y2ggPSBmYWxzZTsKICAgICBpZiAoIW1hdGNoRGF0YS0+ bV9oYXNNYXRjaGVkKSB7CiAgICAgICAgIG1hdGNoRGF0YS0+bV9oYXNNYXRjaGVkID0gdHJ1ZTsK ICAgICAgICAgbWF0Y2hEYXRhLT5tX2Nob3NlbiA9ICpjYW5kaWRhdGU7Ci0gICAgICAgIHJldHVy biAxOworICAgICAgICBmaXJzdE1hdGNoID0gdHJ1ZTsKICAgICB9CiAKLSAgICBpZiAoIWNhbmRp ZGF0ZS0+bGZJdGFsaWMgIT0gIW1hdGNoRGF0YS0+bV9jaG9zZW4ubGZJdGFsaWMpIHsKLSAgICAg ICAgaWYgKCFjYW5kaWRhdGUtPmxmSXRhbGljID09ICFtYXRjaERhdGEtPm1fZGVzaXJlZEl0YWxp YykKLSAgICAgICAgICAgIG1hdGNoRGF0YS0+bV9jaG9zZW4gPSAqY2FuZGlkYXRlOwotCi0gICAg ICAgIHJldHVybiAxOwotICAgIH0KKyAgICBib29sIGRlc2lyZWRJdGFsaWMgPSBtYXRjaERhdGEt Pm1fZGVzaXJlZEl0YWxpYzsKKyAgICBib29sIGNhbmRpZGF0ZUl0YWxpYyA9IGNhbmRpZGF0ZS0+ bGZJdGFsaWM7CisgICAgYm9vbCBjaG9zZW5JdGFsaWMgPSBtYXRjaERhdGEtPm1fY2hvc2VuLmxm SXRhbGljOyAKIAogICAgIHVuc2lnbmVkIGNob3NlbldlaWdodERlbHRhTWFnbml0dWRlID0gYWJz KG1hdGNoRGF0YS0+bV9jaG9zZW4ubGZXZWlnaHQgLSBtYXRjaERhdGEtPm1fZGVzaXJlZFdlaWdo dCk7CiAgICAgdW5zaWduZWQgY2FuZGlkYXRlV2VpZ2h0RGVsdGFNYWduaXR1ZGUgPSBhYnMoY2Fu ZGlkYXRlLT5sZldlaWdodCAtIG1hdGNoRGF0YS0+bV9kZXNpcmVkV2VpZ2h0KTsKIAorICAgIC8v IFdlIENoZWNrICdFeGFjdCBNYXRjaGluZycgZm9yIGJvdGggb2YgY2FzZS4KKyAgICBpZiAoZmly c3RNYXRjaCkgeworICAgICAgICBpZiAoIWNob3NlbldlaWdodERlbHRhTWFnbml0dWRlICYmIGRl c2lyZWRJdGFsaWMgPT0gY2hvc2VuSXRhbGljKQorICAgICAgICAgICAgcmV0dXJuIDA7IAorICAg IH0gZWxzZSB7CisgICAgICAgIGlmICghY2FuZGlkYXRlV2VpZ2h0RGVsdGFNYWduaXR1ZGUgJiYg ZGVzaXJlZEl0YWxpYyA9PSBjYW5kaWRhdGVJdGFsaWMpIHsKKyAgICAgICAgICAgIG1hdGNoRGF0 YS0+bV9jaG9zZW4gPSAqY2FuZGlkYXRlOworICAgICAgICAgICAgcmV0dXJuIDA7IAorICAgICAg ICB9CisgICAgfQorIAorICAgIC8vIHByZWZlciBpdGFsaWMgdGhhbiB3ZWlnaHQuICAKKyAgICBp ZiAoZGVzaXJlZEl0YWxpYyAhPSBjaG9zZW5JdGFsaWMgJiYgZGVzaXJlZEl0YWxpYyA9PSBjYW5k aWRhdGVJdGFsaWMpIHsKKyAgICAgICAgbWF0Y2hEYXRhLT5tX2Nob3NlbiA9ICpjYW5kaWRhdGU7 CisgICAgICAgIHJldHVybiAxOworICAgIH0KKwogICAgIC8vIElmIGJvdGggYXJlIHRoZSBzYW1l IGRpc3RhbmNlIGZyb20gdGhlIGRlc2lyZWQgd2VpZ2h0LCBwcmVmZXIgdGhlIGNhbmRpZGF0ZSBp ZiBpdCBpcyBmdXJ0aGVyIGZyb20gcmVndWxhci4KICAgICBpZiAoY2hvc2VuV2VpZ2h0RGVsdGFN YWduaXR1ZGUgPT0gY2FuZGlkYXRlV2VpZ2h0RGVsdGFNYWduaXR1ZGUgJiYgYWJzKGNhbmRpZGF0 ZS0+bGZXZWlnaHQgLSBGV19OT1JNQUwpID4gYWJzKG1hdGNoRGF0YS0+bV9jaG9zZW4ubGZXZWln aHQgLSBGV19OT1JNQUwpKSB7CiAgICAgICAgIG1hdGNoRGF0YS0+bV9jaG9zZW4gPSAqY2FuZGlk YXRlOwpAQCAtNDIxLDYgKzQzNiwxMCBAQCBzdGF0aWMgSEZPTlQgY3JlYXRlR0RJRm9udChjb25z dCBBdG9taWNTCiAjZW5kaWYKICAgICBtYXRjaERhdGEubV9jaG9zZW4ubGZRdWFsaXR5ID0gREVG QVVMVF9RVUFMSVRZOwogICAgIG1hdGNoRGF0YS5tX2Nob3Nlbi5sZlBpdGNoQW5kRmFtaWx5ID0g REVGQVVMVF9QSVRDSCB8IEZGX0RPTlRDQVJFOworI2lmIFBMQVRGT1JNKENBSVJPKQorICAgIG1h dGNoRGF0YS5tX2Nob3Nlbi5sZldlaWdodCA9IGRlc2lyZWRXZWlnaHQ7CisgICAgbWF0Y2hEYXRh Lm1fY2hvc2VuLmxmSXRhbGljID0gZGVzaXJlZEl0YWxpYzsKKyNlbmRpZiAKIAogICAgIEhGT05U IHJlc3VsdCA9IENyZWF0ZUZvbnRJbmRpcmVjdCgmbWF0Y2hEYXRhLm1fY2hvc2VuKTsKICAgICBp ZiAoIXJlc3VsdCkK