23861 2009-02-09 18:08:00 -0800 Stroke font outlines on chromium linux 2009-02-18 14:07:29 -0800 1 1 1 Unclassified WebKit Text 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 estade estade oldest_to_newest 109076 0 estade 2009-02-09 18:08:00 -0800 See http://codereview.chromium.org/18176 109077 1 27512 estade 2009-02-09 18:10:32 -0800 Created attachment 27512 patch I'll make an entry on the ChangeLog later. The text of that entry will be as follows: Stroke font outlines on chromium in Linux. TEST=LayoutTests/svg/custom/pointer-events-text.svg 109735 2 27512 eric 2009-02-13 14:29:30 -0800 Comment on attachment 27512 patch Since you can't yet land these yourself, it makes sense to include the ChangeLog in your posted patch. It would help if this could have a bit more context: 98 if (textMode & cTextFill) { 99 // See comment in FontChromiumWin.cpp::paintSkiaText() 100 paint.setLooper(0)->safeUnref(); Some idea as to why we unref there. For more context sure, point them to the original comment, but a small local explanation I think would help. .rbg isn't needed here: paint.setColor(gc->strokeColor().rgb()); It seems the shared code from the fill/stroke path could be abstracted into a small static function, where you pass a color. Something like: setupContentForTextPainting(gc, font, paint, color); Maybe it's not worth it, but I think it would be nice to save the 6 lines from the file. Is Paint creation expensive? Seems we might just want to have a local SkPaint per block instead of using .reset()? r- mostly for the lack of changelog. 109742 3 estade 2009-02-13 14:44:53 -0800 I actually only count 2 lines from each code path that could go into your proposed helper function. So probably not worth it :( The reuse of |paint| doesn't save any computation. I will change that. Also I will add more commentary, and post the changelog. 109747 4 27670 estade 2009-02-13 15:24:45 -0800 Created attachment 27670 try2 updated comment, made |paint| local to if blocks, added changelog 110108 5 27670 eric 2009-02-18 12:09:24 -0800 Comment on attachment 27670 try2 LGTM. 110120 6 fishd 2009-02-18 14:07:29 -0800 Landed as http://trac.webkit.org/changeset/41065 27512 2009-02-09 18:10:32 -0800 2009-02-13 15:24:45 -0800 patch fontlinux.diff text/plain 2435 estade SW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vRm9udExpbnV4LmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL0ZvbnRMaW51 eC5jcHAJKHJldmlzaW9uIDQwNzgyKQorKysgV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJv bWl1bS9Gb250TGludXguY3BwCSh3b3JraW5nIGNvcHkpCkBAIC00OSwxNCArNDksNiBAQCBuYW1l c3BhY2UgV2ViQ29yZSB7CiB2b2lkIEZvbnQ6OmRyYXdHbHlwaHMoR3JhcGhpY3NDb250ZXh0KiBn YywgY29uc3QgU2ltcGxlRm9udERhdGEqIGZvbnQsCiAgICAgICAgICAgICAgICAgICAgICAgY29u c3QgR2x5cGhCdWZmZXImIGdseXBoQnVmZmVyLCAgaW50IGZyb20sIGludCBudW1HbHlwaHMsCiAg ICAgICAgICAgICAgICAgICAgICAgY29uc3QgRmxvYXRQb2ludCYgcG9pbnQpIGNvbnN0IHsKLSAg ICBTa0NhbnZhcyogY2FudmFzID0gZ2MtPnBsYXRmb3JtQ29udGV4dCgpLT5jYW52YXMoKTsKLSAg ICBTa1BhaW50IHBhaW50OwotCi0gICAgZ2MtPnBsYXRmb3JtQ29udGV4dCgpLT5zZXR1cFBhaW50 Q29tbW9uKCZwYWludCk7Ci0gICAgZm9udC0+cGxhdGZvcm1EYXRhKCkuc2V0dXBQYWludCgmcGFp bnQpOwotICAgIHBhaW50LnNldFRleHRFbmNvZGluZyhTa1BhaW50OjprR2x5cGhJRF9UZXh0RW5j b2RpbmcpOwotICAgIHBhaW50LnNldENvbG9yKGdjLT5maWxsQ29sb3IoKS5yZ2IoKSk7Ci0KICAg ICBTa0FTU0VSVChzaXplb2YoR2x5cGhCdWZmZXJHbHlwaCkgPT0gc2l6ZW9mKHVpbnQxNl90KSk7 ICAvLyBjb21waWxlLXRpbWUgYXNzZXJ0CiAKICAgICBjb25zdCBHbHlwaEJ1ZmZlckdseXBoKiBn bHlwaHMgPSBnbHlwaEJ1ZmZlci5nbHlwaHMoZnJvbSk7CkBAIC03OCw3ICs3MCwzOCBAQCB2b2lk IEZvbnQ6OmRyYXdHbHlwaHMoR3JhcGhpY3NDb250ZXh0KiBnCiAgICAgICAgIHggKz0gU2tGbG9h dFRvU2NhbGFyKGFkdltpXS53aWR0aCgpKTsKICAgICAgICAgeSArPSBTa0Zsb2F0VG9TY2FsYXIo YWR2W2ldLmhlaWdodCgpKTsKICAgICB9Ci0gICAgY2FudmFzLT5kcmF3UG9zVGV4dChnbHlwaHMs IG51bUdseXBocyA8PCAxLCBwb3MsIHBhaW50KTsKKworICAgIFNrQ2FudmFzKiBjYW52YXMgPSBn Yy0+cGxhdGZvcm1Db250ZXh0KCktPmNhbnZhcygpOworICAgIFNrUGFpbnQgcGFpbnQ7CisgICAg aW50IHRleHRNb2RlID0gZ2MtPnBsYXRmb3JtQ29udGV4dCgpLT5nZXRUZXh0RHJhd2luZ01vZGUo KTsKKworICAgIC8vIFdlIGRyYXcgdGV4dCB1cCB0byB0d28gdGltZXMgKG9uY2UgZm9yIGZpbGws IG9uY2UgZm9yIHN0cm9rZSkuCisgICAgaWYgKHRleHRNb2RlICYgY1RleHRGaWxsKSB7CisgICAg ICAgIGdjLT5wbGF0Zm9ybUNvbnRleHQoKS0+c2V0dXBQYWludEZvckZpbGxpbmcoJnBhaW50KTsK KyAgICAgICAgZm9udC0+cGxhdGZvcm1EYXRhKCkuc2V0dXBQYWludCgmcGFpbnQpOworICAgICAg ICBwYWludC5zZXRUZXh0RW5jb2RpbmcoU2tQYWludDo6a0dseXBoSURfVGV4dEVuY29kaW5nKTsK KyAgICAgICAgcGFpbnQuc2V0Q29sb3IoZ2MtPmZpbGxDb2xvcigpLnJnYigpKTsKKyAgICAgICAg Y2FudmFzLT5kcmF3UG9zVGV4dChnbHlwaHMsIG51bUdseXBocyA8PCAxLCBwb3MsIHBhaW50KTsK KyAgICAgICAgcGFpbnQucmVzZXQoKTsKKyAgICB9CisKKyAgICBpZiAoKHRleHRNb2RlICYgY1Rl eHRTdHJva2UpCisgICAgICAgICYmIGdjLT5wbGF0Zm9ybUNvbnRleHQoKS0+Z2V0U3Ryb2tlU3R5 bGUoKSAhPSBOb1N0cm9rZQorICAgICAgICAmJiBnYy0+cGxhdGZvcm1Db250ZXh0KCktPmdldFN0 cm9rZVRoaWNrbmVzcygpID4gMCkgeworCisgICAgICAgIGdjLT5wbGF0Zm9ybUNvbnRleHQoKS0+ c2V0dXBQYWludEZvclN0cm9raW5nKCZwYWludCwgMCwgMCk7CisgICAgICAgIGZvbnQtPnBsYXRm b3JtRGF0YSgpLnNldHVwUGFpbnQoJnBhaW50KTsKKyAgICAgICAgcGFpbnQuc2V0RmxhZ3MoU2tQ YWludDo6a0FudGlBbGlhc19GbGFnKTsKKyAgICAgICAgcGFpbnQuc2V0VGV4dEVuY29kaW5nKFNr UGFpbnQ6OmtHbHlwaElEX1RleHRFbmNvZGluZyk7CisgICAgICAgIHBhaW50LnNldENvbG9yKGdj LT5zdHJva2VDb2xvcigpLnJnYigpKTsKKworICAgICAgICBpZiAodGV4dE1vZGUgJiBjVGV4dEZp bGwpIHsKKyAgICAgICAgICAgIC8vIFNlZSBjb21tZW50IGluIEZvbnRDaHJvbWl1bVdpbi5jcHA6 OnBhaW50U2tpYVRleHQoKQorICAgICAgICAgICAgcGFpbnQuc2V0TG9vcGVyKDApLT5zYWZlVW5y ZWYoKTsKKyAgICAgICAgfQorCisgICAgICAgIGNhbnZhcy0+ZHJhd1Bvc1RleHQoZ2x5cGhzLCBu dW1HbHlwaHMgPDwgMSwgcG9zLCBwYWludCk7CisgICAgfQogfQogCgo= 27670 2009-02-13 15:24:45 -0800 2009-02-18 12:09:24 -0800 try2 fontlinux2.diff text/plain 3213 estade SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0MDk5MikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMDktMDItMTMgIEV2YW4gU3RhZGUgIDxlc3RhZGVAY2hyb21pdW0u b3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMzg2MQorICAgICAgICBTdHJv a2UgZm9udCBvdXRsaW5lcyBvbiBjaHJvbWl1bSBsaW51eAorCisgICAgICAgIFRFU1Q9TGF5b3V0 VGVzdHMvc3ZnL2N1c3RvbS9wb2ludGVyLWV2ZW50cy10ZXh0LnN2ZworCisgICAgICAgICogcGxh dGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vRm9udExpbnV4LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkZvbnQ6OmRyYXdHbHlwaHMpOgorCiAyMDA5LTAyLTEzICBBZGFtIFRyZWF0ICA8YWRhbS50cmVh dEB0b3JjaG1vYmlsZS5jb20+CiAKICAgICAgICAgQ2F1Z2h0IGJ5IERhcmluIEFkbGVyLgpJbmRl eDogV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9Gb250TGludXguY3BwCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vRm9udExpbnV4LmNw cAkocmV2aXNpb24gNDA5NTcpCisrKyBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVt L0ZvbnRMaW51eC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTQ5LDE0ICs0OSw2IEBAIG5hbWVzcGFj ZSBXZWJDb3JlIHsKIHZvaWQgRm9udDo6ZHJhd0dseXBocyhHcmFwaGljc0NvbnRleHQqIGdjLCBj b25zdCBTaW1wbGVGb250RGF0YSogZm9udCwKICAgICAgICAgICAgICAgICAgICAgICBjb25zdCBH bHlwaEJ1ZmZlciYgZ2x5cGhCdWZmZXIsICBpbnQgZnJvbSwgaW50IG51bUdseXBocywKICAgICAg ICAgICAgICAgICAgICAgICBjb25zdCBGbG9hdFBvaW50JiBwb2ludCkgY29uc3QgewotICAgIFNr Q2FudmFzKiBjYW52YXMgPSBnYy0+cGxhdGZvcm1Db250ZXh0KCktPmNhbnZhcygpOwotICAgIFNr UGFpbnQgcGFpbnQ7Ci0KLSAgICBnYy0+cGxhdGZvcm1Db250ZXh0KCktPnNldHVwUGFpbnRDb21t b24oJnBhaW50KTsKLSAgICBmb250LT5wbGF0Zm9ybURhdGEoKS5zZXR1cFBhaW50KCZwYWludCk7 Ci0gICAgcGFpbnQuc2V0VGV4dEVuY29kaW5nKFNrUGFpbnQ6OmtHbHlwaElEX1RleHRFbmNvZGlu Zyk7Ci0gICAgcGFpbnQuc2V0Q29sb3IoZ2MtPmZpbGxDb2xvcigpLnJnYigpKTsKLQogICAgIFNr QVNTRVJUKHNpemVvZihHbHlwaEJ1ZmZlckdseXBoKSA9PSBzaXplb2YodWludDE2X3QpKTsgIC8v IGNvbXBpbGUtdGltZSBhc3NlcnQKIAogICAgIGNvbnN0IEdseXBoQnVmZmVyR2x5cGgqIGdseXBo cyA9IGdseXBoQnVmZmVyLmdseXBocyhmcm9tKTsKQEAgLTc4LDcgKzcwLDM5IEBAIHZvaWQgRm9u dDo6ZHJhd0dseXBocyhHcmFwaGljc0NvbnRleHQqIGcKICAgICAgICAgeCArPSBTa0Zsb2F0VG9T Y2FsYXIoYWR2W2ldLndpZHRoKCkpOwogICAgICAgICB5ICs9IFNrRmxvYXRUb1NjYWxhcihhZHZb aV0uaGVpZ2h0KCkpOwogICAgIH0KLSAgICBjYW52YXMtPmRyYXdQb3NUZXh0KGdseXBocywgbnVt R2x5cGhzIDw8IDEsIHBvcywgcGFpbnQpOworCisgICAgU2tDYW52YXMqIGNhbnZhcyA9IGdjLT5w bGF0Zm9ybUNvbnRleHQoKS0+Y2FudmFzKCk7CisgICAgaW50IHRleHRNb2RlID0gZ2MtPnBsYXRm b3JtQ29udGV4dCgpLT5nZXRUZXh0RHJhd2luZ01vZGUoKTsKKworICAgIC8vIFdlIGRyYXcgdGV4 dCB1cCB0byB0d28gdGltZXMgKG9uY2UgZm9yIGZpbGwsIG9uY2UgZm9yIHN0cm9rZSkuCisgICAg aWYgKHRleHRNb2RlICYgY1RleHRGaWxsKSB7CisgICAgICAgIFNrUGFpbnQgcGFpbnQ7CisgICAg ICAgIGdjLT5wbGF0Zm9ybUNvbnRleHQoKS0+c2V0dXBQYWludEZvckZpbGxpbmcoJnBhaW50KTsK KyAgICAgICAgZm9udC0+cGxhdGZvcm1EYXRhKCkuc2V0dXBQYWludCgmcGFpbnQpOworICAgICAg ICBwYWludC5zZXRUZXh0RW5jb2RpbmcoU2tQYWludDo6a0dseXBoSURfVGV4dEVuY29kaW5nKTsK KyAgICAgICAgcGFpbnQuc2V0Q29sb3IoZ2MtPmZpbGxDb2xvcigpKTsKKyAgICAgICAgY2FudmFz LT5kcmF3UG9zVGV4dChnbHlwaHMsIG51bUdseXBocyA8PCAxLCBwb3MsIHBhaW50KTsKKyAgICB9 CisKKyAgICBpZiAoKHRleHRNb2RlICYgY1RleHRTdHJva2UpCisgICAgICAgICYmIGdjLT5wbGF0 Zm9ybUNvbnRleHQoKS0+Z2V0U3Ryb2tlU3R5bGUoKSAhPSBOb1N0cm9rZQorICAgICAgICAmJiBn Yy0+cGxhdGZvcm1Db250ZXh0KCktPmdldFN0cm9rZVRoaWNrbmVzcygpID4gMCkgeworCisgICAg ICAgIFNrUGFpbnQgcGFpbnQ7CisgICAgICAgIGdjLT5wbGF0Zm9ybUNvbnRleHQoKS0+c2V0dXBQ YWludEZvclN0cm9raW5nKCZwYWludCwgMCwgMCk7CisgICAgICAgIGZvbnQtPnBsYXRmb3JtRGF0 YSgpLnNldHVwUGFpbnQoJnBhaW50KTsKKyAgICAgICAgcGFpbnQuc2V0RmxhZ3MoU2tQYWludDo6 a0FudGlBbGlhc19GbGFnKTsKKyAgICAgICAgcGFpbnQuc2V0VGV4dEVuY29kaW5nKFNrUGFpbnQ6 OmtHbHlwaElEX1RleHRFbmNvZGluZyk7CisgICAgICAgIHBhaW50LnNldENvbG9yKGdjLT5zdHJv a2VDb2xvcigpKTsKKworICAgICAgICBpZiAodGV4dE1vZGUgJiBjVGV4dEZpbGwpIHsKKyAgICAg ICAgICAgIC8vIElmIHdlIGFsc28gZmlsbGVkLCB3ZSBkb24ndCB3YW50IHRvIGRyYXcgc2hhZG93 cyB0d2ljZS4KKyAgICAgICAgICAgIC8vIFNlZSBjb21tZW50IGluIEZvbnRDaHJvbWl1bVdpbi5j cHA6OnBhaW50U2tpYVRleHQoKSBmb3IgbW9yZSBkZXRhaWxzLgorICAgICAgICAgICAgcGFpbnQu c2V0TG9vcGVyKDApLT5zYWZlVW5yZWYoKTsKKyAgICAgICAgfQorCisgICAgICAgIGNhbnZhcy0+ ZHJhd1Bvc1RleHQoZ2x5cGhzLCBudW1HbHlwaHMgPDwgMSwgcG9zLCBwYWludCk7CisgICAgfQog fQogCiB2b2lkIEZvbnQ6OmRyYXdDb21wbGV4VGV4dChHcmFwaGljc0NvbnRleHQqIGNvbnRleHQs IGNvbnN0IFRleHRSdW4mIHJ1biwK