97067 2012-09-18 21:34:52 -0700 [Chromium] SkiaGetGlyphWidthAndExtents() should invert y-axis 2012-09-18 23:19:10 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 bashi bashi tony webkit.review.bot yutak oldest_to_newest 723253 0 bashi 2012-09-18 21:34:52 -0700 Skia is y-grows-down, while harfbuzz is set up with y-grows-up when we set scale positive. We also don't need to call hb_font_set_ppem(). hb-view() don't call it. 723263 1 164658 bashi 2012-09-18 22:09:42 -0700 Created attachment 164658 Patch 723264 2 bashi 2012-09-18 22:10:57 -0700 Yuta-san, could you rubber-stamp? behdad@ did informal review for this patch. 723271 3 164658 yutak 2012-09-18 22:27:02 -0700 Comment on attachment 164658 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164658&action=review Code change looks fine. r- for lack of comments for non-obvious code. > Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzNGFaceSkia.cpp:72 > + extents->y_bearing = SkiaScalarToHarfbuzzPosition(-skBounds.fTop); I would like some comments explaining these nagations. Looks hard to understand. 723287 4 164663 bashi 2012-09-18 22:47:46 -0700 Created attachment 164663 Added comment 723288 5 164658 bashi 2012-09-18 22:48:20 -0700 Comment on attachment 164658 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164658&action=review Thank you for review! >> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzNGFaceSkia.cpp:72 >> + extents->y_bearing = SkiaScalarToHarfbuzzPosition(-skBounds.fTop); > > I would like some comments explaining these nagations. Looks hard to understand. Done. 723289 6 164663 yutak 2012-09-18 22:48:51 -0700 Comment on attachment 164663 Added comment OK. 723290 7 164663 bashi 2012-09-18 22:51:48 -0700 Comment on attachment 164663 Added comment Thanks! 723306 8 164663 webkit.review.bot 2012-09-18 23:19:08 -0700 Comment on attachment 164663 Added comment Clearing flags on attachment: 164663 Committed r128968: <http://trac.webkit.org/changeset/128968> 723307 9 webkit.review.bot 2012-09-18 23:19:10 -0700 All reviewed patches have been landed. Closing bug. 164658 2012-09-18 22:09:42 -0700 2012-09-18 22:48:20 -0700 Patch bug-97067-20120919140908.patch text/plain 2668 bashi U3VidmVyc2lvbiBSZXZpc2lvbjogMTI4OTM3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggODc4MTNjMGM2M2M2NDM3 OGZmNjA3NzY0ZjcxNjM3MjY2Y2RjMmVlZC4uMTQyZjMzNTQ3MjUyNzBiOTZhYTBhMjFiY2E1ZGM3 ZDk0NThmYTM5MiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA5LTE4ICBLZW5p Y2hpIElzaGliYXNoaSAgPGJhc2hpQGNocm9taXVtLm9yZz4KKworICAgICAgICBbQ2hyb21pdW1d IFNraWFHZXRHbHlwaFdpZHRoQW5kRXh0ZW50cygpIHNob3VsZCBpbnZlcnQgeS1heGlzCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05NzA2NworCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEludmVydCBza0JvdW5k cy5mVG9wIGFuZCBza0JvdW5kcy5oZWlnaHQoKS4gRG9uJ3QgY2FsbCBoYl9mb250X3NldF9wcGVt KCkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiBBcmFiaWMgc2hhZGRhIChVKzA2NTEpIHNob3Vs ZCBiZSBwbGFjZWQgbW9yZSBoaWdoZXIgd2hlbiBBcmFiaWMgbGFtIChVKzA2NDQpIGZvbGxvd3Mg aXQuCisgICAgICAgIFRlc3RzIHVuZGVyIHN2Zy9XM0MtSTE4TiBjb250YWluIHN1Y2ggc2VxdWVu Y2VzIHNvIHRoZXNlIHRlc3RzIGNvdmVyIHRoaXMgY2hhbmdlLgorCisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3MvaGFyZmJ1enovbmcvSGFyZkJ1enpOR0ZhY2VTa2lhLmNwcDoKKyAgICAgICAg KFdlYkNvcmU6OlNraWFHZXRHbHlwaFdpZHRoQW5kRXh0ZW50cyk6CisgICAgICAgIChXZWJDb3Jl OjpIYXJmQnV6ek5HRmFjZTo6Y3JlYXRlRm9udCk6CisKIDIwMTItMDktMTggIEpvaG4gTWVsbG9y ICA8am9obm1lQGNocm9taXVtLm9yZz4KIAogICAgICAgICBUZXh0IEF1dG9zaXppbmc6IElnbm9y ZSBjb25zdHJhaW5lZCBoZWlnaHRzIGluIGNlcnRhaW4gY2lyY3Vtc3RhbmNlcy4KZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L25nL0hhcmZCdXp6 TkdGYWNlU2tpYS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9oYXJmYnV6 ei9uZy9IYXJmQnV6ek5HRmFjZVNraWEuY3BwCmluZGV4IGI5YmE3ZDM2MjI2MjJjY2VmNjA5ZWFh NTYzOTE3ZDBhNGIyYjkxN2QuLjQwNzBlZDIxMTFmZmEyM2NjYTUxM2MxNzRkMWZiNzJiODJhZTVk NTAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6 L25nL0hhcmZCdXp6TkdGYWNlU2tpYS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v Z3JhcGhpY3MvaGFyZmJ1enovbmcvSGFyZkJ1enpOR0ZhY2VTa2lhLmNwcApAQCAtNjksMTAgKzY5 LDkgQEAgc3RhdGljIHZvaWQgU2tpYUdldEdseXBoV2lkdGhBbmRFeHRlbnRzKFNrUGFpbnQqIHBh aW50LCBoYl9jb2RlcG9pbnRfdCBjb2RlcG9pbnQKICAgICAgICAgKndpZHRoID0gU2tpYVNjYWxh clRvSGFyZmJ1enpQb3NpdGlvbihza1dpZHRoKTsKICAgICBpZiAoZXh0ZW50cykgewogICAgICAg ICBleHRlbnRzLT54X2JlYXJpbmcgPSBTa2lhU2NhbGFyVG9IYXJmYnV6elBvc2l0aW9uKHNrQm91 bmRzLmZMZWZ0KTsKLSAgICAgICAgLy8gVGhlIGV4dGVudHMgYXJlIHVudXNlZCBpbiBoYXJmYnV6 eiBub3cgc28gd2UncmUgdW5zdXJlIHdoZXRoZXIgYSBuZWdhdGlvbiBpcyBuZWVkZWQgaGVyZS4g UmV2aXNpdCB3aGVuIHdlJ3JlIHN1cmUuCi0gICAgICAgIGV4dGVudHMtPnlfYmVhcmluZyA9IFNr aWFTY2FsYXJUb0hhcmZidXp6UG9zaXRpb24oc2tCb3VuZHMuZlRvcCk7CisgICAgICAgIGV4dGVu dHMtPnlfYmVhcmluZyA9IFNraWFTY2FsYXJUb0hhcmZidXp6UG9zaXRpb24oLXNrQm91bmRzLmZU b3ApOwogICAgICAgICBleHRlbnRzLT53aWR0aCA9IFNraWFTY2FsYXJUb0hhcmZidXp6UG9zaXRp b24oc2tCb3VuZHMud2lkdGgoKSk7Ci0gICAgICAgIGV4dGVudHMtPmhlaWdodCA9IFNraWFTY2Fs YXJUb0hhcmZidXp6UG9zaXRpb24oc2tCb3VuZHMuaGVpZ2h0KCkpOworICAgICAgICBleHRlbnRz LT5oZWlnaHQgPSBTa2lhU2NhbGFyVG9IYXJmYnV6elBvc2l0aW9uKC1za0JvdW5kcy5oZWlnaHQo KSk7CiAgICAgfQogfQogCkBAIC0xNzEsOCArMTcwLDYgQEAgaGJfZm9udF90KiBIYXJmQnV6ek5H RmFjZTo6Y3JlYXRlRm9udCgpCiAgICAgbV9wbGF0Zm9ybURhdGEtPnNldHVwUGFpbnQocGFpbnQp OwogICAgIGhiX2ZvbnRfc2V0X2Z1bmNzKGZvbnQsIGhhcmZidXp6U2tpYUdldEZvbnRGdW5jcygp LCBwYWludCwgZGVzdHJveVBhaW50KTsKICAgICBmbG9hdCBzaXplID0gbV9wbGF0Zm9ybURhdGEt PnNpemUoKTsKLSAgICBpZiAoZmxvb3JmKHNpemUpID09IHNpemUpCi0gICAgICAgIGhiX2ZvbnRf c2V0X3BwZW0oZm9udCwgc2l6ZSwgc2l6ZSk7CiAgICAgaW50IHNjYWxlID0gU2tpYVNjYWxhclRv SGFyZmJ1enpQb3NpdGlvbihzaXplKTsKICAgICBoYl9mb250X3NldF9zY2FsZShmb250LCBzY2Fs ZSwgc2NhbGUpOwogICAgIGhiX2ZvbnRfbWFrZV9pbW11dGFibGUoZm9udCk7Cg== 164663 2012-09-18 22:47:46 -0700 2012-09-18 23:19:08 -0700 Added comment bug-97067-20120919144712.patch text/plain 2781 bashi U3VidmVyc2lvbiBSZXZpc2lvbjogMTI4OTM3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggODc4MTNjMGM2M2M2NDM3 OGZmNjA3NzY0ZjcxNjM3MjY2Y2RjMmVlZC4uMTQyZjMzNTQ3MjUyNzBiOTZhYTBhMjFiY2E1ZGM3 ZDk0NThmYTM5MiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA5LTE4ICBLZW5p Y2hpIElzaGliYXNoaSAgPGJhc2hpQGNocm9taXVtLm9yZz4KKworICAgICAgICBbQ2hyb21pdW1d IFNraWFHZXRHbHlwaFdpZHRoQW5kRXh0ZW50cygpIHNob3VsZCBpbnZlcnQgeS1heGlzCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05NzA2NworCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEludmVydCBza0JvdW5k cy5mVG9wIGFuZCBza0JvdW5kcy5oZWlnaHQoKS4gRG9uJ3QgY2FsbCBoYl9mb250X3NldF9wcGVt KCkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiBBcmFiaWMgc2hhZGRhIChVKzA2NTEpIHNob3Vs ZCBiZSBwbGFjZWQgbW9yZSBoaWdoZXIgd2hlbiBBcmFiaWMgbGFtIChVKzA2NDQpIGZvbGxvd3Mg aXQuCisgICAgICAgIFRlc3RzIHVuZGVyIHN2Zy9XM0MtSTE4TiBjb250YWluIHN1Y2ggc2VxdWVu Y2VzIHNvIHRoZXNlIHRlc3RzIGNvdmVyIHRoaXMgY2hhbmdlLgorCisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3MvaGFyZmJ1enovbmcvSGFyZkJ1enpOR0ZhY2VTa2lhLmNwcDoKKyAgICAgICAg KFdlYkNvcmU6OlNraWFHZXRHbHlwaFdpZHRoQW5kRXh0ZW50cyk6CisgICAgICAgIChXZWJDb3Jl OjpIYXJmQnV6ek5HRmFjZTo6Y3JlYXRlRm9udCk6CisKIDIwMTItMDktMTggIEpvaG4gTWVsbG9y ICA8am9obm1lQGNocm9taXVtLm9yZz4KIAogICAgICAgICBUZXh0IEF1dG9zaXppbmc6IElnbm9y ZSBjb25zdHJhaW5lZCBoZWlnaHRzIGluIGNlcnRhaW4gY2lyY3Vtc3RhbmNlcy4KZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L25nL0hhcmZCdXp6 TkdGYWNlU2tpYS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9oYXJmYnV6 ei9uZy9IYXJmQnV6ek5HRmFjZVNraWEuY3BwCmluZGV4IGI5YmE3ZDM2MjI2MjJjY2VmNjA5ZWFh NTYzOTE3ZDBhNGIyYjkxN2QuLjUzYWNiMjkyY2RiZjA5ODhjNDA0MDQ0ZTExOTA4ZjgyMWJjOTc4 MDQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6 L25nL0hhcmZCdXp6TkdGYWNlU2tpYS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v Z3JhcGhpY3MvaGFyZmJ1enovbmcvSGFyZkJ1enpOR0ZhY2VTa2lhLmNwcApAQCAtNjgsMTEgKzY4 LDExIEBAIHN0YXRpYyB2b2lkIFNraWFHZXRHbHlwaFdpZHRoQW5kRXh0ZW50cyhTa1BhaW50KiBw YWludCwgaGJfY29kZXBvaW50X3QgY29kZXBvaW50CiAgICAgaWYgKHdpZHRoKQogICAgICAgICAq d2lkdGggPSBTa2lhU2NhbGFyVG9IYXJmYnV6elBvc2l0aW9uKHNrV2lkdGgpOwogICAgIGlmIChl eHRlbnRzKSB7CisgICAgICAgIC8vIEludmVydCB5LWF4aXMgYmVjYXVzZSBTa2lhIGlzIHktZ3Jv d3MtZG93biBidXQgd2Ugc2V0IHVwIGhhcmZidXp6IHRvIGJlIHktZ3Jvd3MtdXAuCiAgICAgICAg IGV4dGVudHMtPnhfYmVhcmluZyA9IFNraWFTY2FsYXJUb0hhcmZidXp6UG9zaXRpb24oc2tCb3Vu ZHMuZkxlZnQpOwotICAgICAgICAvLyBUaGUgZXh0ZW50cyBhcmUgdW51c2VkIGluIGhhcmZidXp6 IG5vdyBzbyB3ZSdyZSB1bnN1cmUgd2hldGhlciBhIG5lZ2F0aW9uIGlzIG5lZWRlZCBoZXJlLiBS ZXZpc2l0IHdoZW4gd2UncmUgc3VyZS4KLSAgICAgICAgZXh0ZW50cy0+eV9iZWFyaW5nID0gU2tp YVNjYWxhclRvSGFyZmJ1enpQb3NpdGlvbihza0JvdW5kcy5mVG9wKTsKKyAgICAgICAgZXh0ZW50 cy0+eV9iZWFyaW5nID0gU2tpYVNjYWxhclRvSGFyZmJ1enpQb3NpdGlvbigtc2tCb3VuZHMuZlRv cCk7CiAgICAgICAgIGV4dGVudHMtPndpZHRoID0gU2tpYVNjYWxhclRvSGFyZmJ1enpQb3NpdGlv bihza0JvdW5kcy53aWR0aCgpKTsKLSAgICAgICAgZXh0ZW50cy0+aGVpZ2h0ID0gU2tpYVNjYWxh clRvSGFyZmJ1enpQb3NpdGlvbihza0JvdW5kcy5oZWlnaHQoKSk7CisgICAgICAgIGV4dGVudHMt PmhlaWdodCA9IFNraWFTY2FsYXJUb0hhcmZidXp6UG9zaXRpb24oLXNrQm91bmRzLmhlaWdodCgp KTsKICAgICB9CiB9CiAKQEAgLTE3MSw4ICsxNzEsNiBAQCBoYl9mb250X3QqIEhhcmZCdXp6TkdG YWNlOjpjcmVhdGVGb250KCkKICAgICBtX3BsYXRmb3JtRGF0YS0+c2V0dXBQYWludChwYWludCk7 CiAgICAgaGJfZm9udF9zZXRfZnVuY3MoZm9udCwgaGFyZmJ1enpTa2lhR2V0Rm9udEZ1bmNzKCks IHBhaW50LCBkZXN0cm95UGFpbnQpOwogICAgIGZsb2F0IHNpemUgPSBtX3BsYXRmb3JtRGF0YS0+ c2l6ZSgpOwotICAgIGlmIChmbG9vcmYoc2l6ZSkgPT0gc2l6ZSkKLSAgICAgICAgaGJfZm9udF9z ZXRfcHBlbShmb250LCBzaXplLCBzaXplKTsKICAgICBpbnQgc2NhbGUgPSBTa2lhU2NhbGFyVG9I YXJmYnV6elBvc2l0aW9uKHNpemUpOwogICAgIGhiX2ZvbnRfc2V0X3NjYWxlKGZvbnQsIHNjYWxl LCBzY2FsZSk7CiAgICAgaGJfZm9udF9tYWtlX2ltbXV0YWJsZShmb250KTsK