130848 2014-03-27 10:55:11 -0700 Remove comment from Filter.h 2014-03-27 13:08:17 -0700 1 1 1 Unclassified WebKit SVG 528+ (Nightly build) All All RESOLVED FIXED P3 Normal --- 1 savagobr webkit-unassigned commit-queue dino d-r fmalita gyuyoung.kim kondapallykalyan krit pdr schenney sergio simon.fraser zimmermann oldest_to_newest 994974 0 savagobr 2014-03-27 10:55:11 -0700 As noticed in the TODO in #130779, it was possible to make the parameters in applyHorizontalScale()/VerticalScale() const refs if small changes were made on the derived classes (e.g. SVGFilter). 994978 1 227960 savagobr 2014-03-27 11:02:40 -0700 Created attachment 227960 Patch 994992 2 227960 krit 2014-03-27 11:51:32 -0700 Comment on attachment 227960 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227960&action=review Not sure why you are doing that. passing a reference to a float is not more or less expecive than passing the value itself. All the float references should be turned back to normal float arguments. > Source/WebCore/platform/graphics/filters/Filter.h:50 > + virtual float applyHorizontalScale(const float& value) const { return value * m_filterResolution.width(); } > + virtual float applyVerticalScale(const float& value) const { return value * m_filterResolution.height(); } there is no need to make the float a reference, it doesn't save anything. > Source/WebCore/svg/graphics/filters/SVGFilter.cpp:39 > +float SVGFilter::applyHorizontalScale(const float& value) const Ditto. 994996 3 savagobr 2014-03-27 11:56:17 -0700 Dirk Thanks for the review. I have one question: const float would be ok? 995011 4 227960 simon.fraser 2014-03-27 12:26:59 -0700 Comment on attachment 227960 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227960&action=review >> Source/WebCore/platform/graphics/filters/Filter.h:50 >> + virtual float applyVerticalScale(const float& value) const { return value * m_filterResolution.height(); } > > there is no need to make the float a reference, it doesn't save anything. Nor a const float. The const doesn't add anything. 995012 5 227964 savagobr 2014-03-27 12:27:51 -0700 Created attachment 227964 Patch 995013 6 savagobr 2014-03-27 12:35:33 -0700 Gentlemen Thanks for the review. I agree that it won't fly, so I'm just renaming the bug title and simply removing the TODO comment. 995024 7 227964 commit-queue 2014-03-27 13:08:09 -0700 Comment on attachment 227964 Patch Clearing flags on attachment: 227964 Committed r166368: <http://trac.webkit.org/changeset/166368> 995025 8 commit-queue 2014-03-27 13:08:17 -0700 All reviewed patches have been landed. Closing bug. 227960 2014-03-27 11:02:40 -0700 2014-03-27 12:27:45 -0700 Patch bug-130848-20140327110220.patch text/plain 4568 savagobr U3VidmVyc2lvbiBSZXZpc2lvbjogMTY2MzU4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYjI0MTIxNzU2NTEzMTA3 ZDczZjk3N2QwZGY3ZmE1NjI3YzFlMzEzNS4uMTY0MmNkZjI3NzE5NGQ2MzFjZWEyZmJjMGMxZjZh ODFkNmZmMTkwZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDE0LTAzLTI3ICBBZGVu aWxzb24gQ2F2YWxjYW50aSAgPGNhdmFsY2FudGlpQGdtYWlsLmNvbT4KKworICAgICAgICBGaWx0 ZXI6IHBhc3MgYnkgcmVmIGNvbnN0IG9uIGFwcGx5U2NhbGUgbWV0aG9kcworICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMwODQ4CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSXQgaXMgcG9zc2libGUgdG8gbWFr ZSBwYXJhbWV0ZXJzIGNvbnN0IHJlZnMgd2l0aCBzbWFsbCBjaGFuZ2VzIG9uCisgICAgICAgIGRl cml2ZWQgY2xhc3Nlcy4KKworICAgICAgICBObyBuZXcgdGVzdHMsIG5vIGNoYW5nZSBvbiBiZWhh dmlvci4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2ZpbHRlcnMvRmlsdGVyLmg6Cisg ICAgICAgIChXZWJDb3JlOjpGaWx0ZXI6OmFwcGx5SG9yaXpvbnRhbFNjYWxlKToKKyAgICAgICAg KFdlYkNvcmU6OkZpbHRlcjo6YXBwbHlWZXJ0aWNhbFNjYWxlKToKKyAgICAgICAgKiBzdmcvZ3Jh cGhpY3MvZmlsdGVycy9TVkdGaWx0ZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6U1ZHRmlsdGVy OjphcHBseUhvcml6b250YWxTY2FsZSk6CisgICAgICAgIChXZWJDb3JlOjpTVkdGaWx0ZXI6OmFw cGx5VmVydGljYWxTY2FsZSk6CisgICAgICAgICogc3ZnL2dyYXBoaWNzL2ZpbHRlcnMvU1ZHRmls dGVyLmg6CisKIDIwMTQtMDMtMjcgIEtyenlzenRvZiBDemVjaCAgPGsuY3plY2hAc2Ftc3VuZy5j b20+CiAKICAgICAgICAgQVg6IFJldHVybnMgY29uc3QgcmVmZXJlbmNlIHRvIHN0YXRpYyBzdHJp bmcuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9maWx0ZXJz L0ZpbHRlci5oIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZmlsdGVycy9GaWx0 ZXIuaAppbmRleCA1NjQzZGVmNDA1NzkwMzgzMjFjMDc4M2UwYmY1YzEwNzU0OWI4NjQ5Li5kOWRi NDc5NTA3MTQ1OGMzMjkyMzEzYjkwNjcyZDU2M2NhOTliZTc0IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9maWx0ZXJzL0ZpbHRlci5oCisrKyBiL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2ZpbHRlcnMvRmlsdGVyLmgKQEAgLTQ2LDkgKzQ2LDgg QEAgcHVibGljOgogCiAgICAgdmlydHVhbCBib29sIGlzU1ZHRmlsdGVyKCkgY29uc3QgeyByZXR1 cm4gZmFsc2U7IH0KIAotICAgIC8vIFRPRE86IHZlcmlmeSBkZXNjZW5kYW50cyB0aGF0IG92ZXJs b2FkIHRoaXMgbWV0aG9kIGFuZCB0dXJuIHRoZW0gdG8gY29uc3QuCi0gICAgdmlydHVhbCBmbG9h dCBhcHBseUhvcml6b250YWxTY2FsZShmbG9hdCB2YWx1ZSkgY29uc3QgeyByZXR1cm4gdmFsdWUg KiBtX2ZpbHRlclJlc29sdXRpb24ud2lkdGgoKTsgfQotICAgIHZpcnR1YWwgZmxvYXQgYXBwbHlW ZXJ0aWNhbFNjYWxlKGZsb2F0IHZhbHVlKSBjb25zdCB7IHJldHVybiB2YWx1ZSAqIG1fZmlsdGVy UmVzb2x1dGlvbi5oZWlnaHQoKTsgfQorICAgIHZpcnR1YWwgZmxvYXQgYXBwbHlIb3Jpem9udGFs U2NhbGUoY29uc3QgZmxvYXQmIHZhbHVlKSBjb25zdCB7IHJldHVybiB2YWx1ZSAqIG1fZmlsdGVy UmVzb2x1dGlvbi53aWR0aCgpOyB9CisgICAgdmlydHVhbCBmbG9hdCBhcHBseVZlcnRpY2FsU2Nh bGUoY29uc3QgZmxvYXQmIHZhbHVlKSBjb25zdCB7IHJldHVybiB2YWx1ZSAqIG1fZmlsdGVyUmVz b2x1dGlvbi5oZWlnaHQoKTsgfQogICAgIAogICAgIHZpcnR1YWwgRmxvYXRSZWN0IHNvdXJjZUlt YWdlUmVjdCgpIGNvbnN0ID0gMDsKICAgICB2aXJ0dWFsIEZsb2F0UmVjdCBmaWx0ZXJSZWdpb24o KSBjb25zdCA9IDA7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvZmls dGVycy9TVkdGaWx0ZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvc3ZnL2dyYXBoaWNzL2ZpbHRlcnMv U1ZHRmlsdGVyLmNwcAppbmRleCAyNDc1N2QxOWFkY2JmOGIxYjkyNDFiNmFlNzY0ZDViZDlkMDRm MGNhLi42ZDg3NzhkMDYxYWUxOGU4N2YzZDQ0YWZjOGU3ZmZiODIxODc3MmQ3IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvZmlsdGVycy9TVkdGaWx0ZXIuY3BwCisrKyBi L1NvdXJjZS9XZWJDb3JlL3N2Zy9ncmFwaGljcy9maWx0ZXJzL1NWR0ZpbHRlci5jcHAKQEAgLTM2 LDE3ICszNiwxOSBAQCBTVkdGaWx0ZXI6OlNWR0ZpbHRlcihjb25zdCBBZmZpbmVUcmFuc2Zvcm0m IGFic29sdXRlVHJhbnNmb3JtLCBjb25zdCBGbG9hdFJlY3QmCiAgICAgbV9hYnNvbHV0ZUZpbHRl clJlZ2lvbiA9IGFic29sdXRlVHJhbnNmb3JtLm1hcFJlY3QoZmlsdGVyUmVnaW9uKTsKIH0KIAot ZmxvYXQgU1ZHRmlsdGVyOjphcHBseUhvcml6b250YWxTY2FsZShmbG9hdCB2YWx1ZSkgY29uc3QK K2Zsb2F0IFNWR0ZpbHRlcjo6YXBwbHlIb3Jpem9udGFsU2NhbGUoY29uc3QgZmxvYXQmIHZhbHVl KSBjb25zdAogewogICAgIGlmIChtX2VmZmVjdEJCb3hNb2RlKQotICAgICAgICB2YWx1ZSAqPSBt X3RhcmdldEJvdW5kaW5nQm94LndpZHRoKCk7CisgICAgICAgIHJldHVybiBGaWx0ZXI6OmFwcGx5 SG9yaXpvbnRhbFNjYWxlKHZhbHVlICogbV90YXJnZXRCb3VuZGluZ0JveC53aWR0aCgpKSAqIG1f YWJzb2x1dGVGaWx0ZXJSZWdpb24ud2lkdGgoKSAvIG1fZmlsdGVyUmVnaW9uLndpZHRoKCk7CisK ICAgICByZXR1cm4gRmlsdGVyOjphcHBseUhvcml6b250YWxTY2FsZSh2YWx1ZSkgKiBtX2Fic29s dXRlRmlsdGVyUmVnaW9uLndpZHRoKCkgLyBtX2ZpbHRlclJlZ2lvbi53aWR0aCgpOwogfQogCi1m bG9hdCBTVkdGaWx0ZXI6OmFwcGx5VmVydGljYWxTY2FsZShmbG9hdCB2YWx1ZSkgY29uc3QKK2Zs b2F0IFNWR0ZpbHRlcjo6YXBwbHlWZXJ0aWNhbFNjYWxlKGNvbnN0IGZsb2F0JiB2YWx1ZSkgY29u c3QKIHsKICAgICBpZiAobV9lZmZlY3RCQm94TW9kZSkKLSAgICAgICAgdmFsdWUgKj0gbV90YXJn ZXRCb3VuZGluZ0JveC5oZWlnaHQoKTsKKyAgICAgICAgcmV0dXJuIEZpbHRlcjo6YXBwbHlWZXJ0 aWNhbFNjYWxlKHZhbHVlICogbV90YXJnZXRCb3VuZGluZ0JveC5oZWlnaHQoKSkgKiBtX2Fic29s dXRlRmlsdGVyUmVnaW9uLmhlaWdodCgpIC8gbV9maWx0ZXJSZWdpb24uaGVpZ2h0KCk7CisKICAg ICByZXR1cm4gRmlsdGVyOjphcHBseVZlcnRpY2FsU2NhbGUodmFsdWUpICogbV9hYnNvbHV0ZUZp bHRlclJlZ2lvbi5oZWlnaHQoKSAvIG1fZmlsdGVyUmVnaW9uLmhlaWdodCgpOwogfQogCmRpZmYg LS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9zdmcvZ3JhcGhpY3MvZmlsdGVycy9TVkdGaWx0ZXIuaCBi L1NvdXJjZS9XZWJDb3JlL3N2Zy9ncmFwaGljcy9maWx0ZXJzL1NWR0ZpbHRlci5oCmluZGV4IDRm MWU3ZDUwY2NlNjBiMmQ0NTk4NDFlMDY0YjNiYzc0N2IwMWZjYzEuLjgwMjgyZmY5ZDAzNTQwYWM2 MGNmNzAwNGMxZGRhOWNhYzg4ODgxMzcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3N2Zy9n cmFwaGljcy9maWx0ZXJzL1NWR0ZpbHRlci5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3N2Zy9ncmFw aGljcy9maWx0ZXJzL1NWR0ZpbHRlci5oCkBAIC00Myw4ICs0Myw4IEBAIHB1YmxpYzoKICAgICB2 aXJ0dWFsIEZsb2F0UG9pbnQgbWFwQWJzb2x1dGVQb2ludFRvTG9jYWxQb2ludChjb25zdCBGbG9h dFBvaW50JiBwb2ludCkgY29uc3Qgb3ZlcnJpZGUgeyByZXR1cm4gbV9hYnNvbHV0ZVRyYW5zZm9y bS5pbnZlcnNlKCkubWFwUG9pbnQocG9pbnQpOyB9CiAgICAgY29uc3QgQWZmaW5lVHJhbnNmb3Jt JiBhYnNvbHV0ZVRyYW5zZm9ybSgpIGNvbnN0IHsgcmV0dXJuIG1fYWJzb2x1dGVUcmFuc2Zvcm07 IH0KIAotICAgIHZpcnR1YWwgZmxvYXQgYXBwbHlIb3Jpem9udGFsU2NhbGUoZmxvYXQgdmFsdWUp IGNvbnN0IG92ZXJyaWRlOwotICAgIHZpcnR1YWwgZmxvYXQgYXBwbHlWZXJ0aWNhbFNjYWxlKGZs b2F0IHZhbHVlKSBjb25zdCBvdmVycmlkZTsKKyAgICB2aXJ0dWFsIGZsb2F0IGFwcGx5SG9yaXpv bnRhbFNjYWxlKGNvbnN0IGZsb2F0JiB2YWx1ZSkgY29uc3Qgb3ZlcnJpZGU7CisgICAgdmlydHVh bCBmbG9hdCBhcHBseVZlcnRpY2FsU2NhbGUoY29uc3QgZmxvYXQmIHZhbHVlKSBjb25zdCBvdmVy cmlkZTsKIAogICAgIHZpcnR1YWwgRmxvYXRSZWN0IHNvdXJjZUltYWdlUmVjdCgpIGNvbnN0IG92 ZXJyaWRlIHsgcmV0dXJuIG1fYWJzb2x1dGVTb3VyY2VEcmF3aW5nUmVnaW9uOyB9CiAgICAgRmxv YXRSZWN0IHRhcmdldEJvdW5kaW5nQm94KCkgY29uc3QgeyByZXR1cm4gbV90YXJnZXRCb3VuZGlu Z0JveDsgfQo= 227964 2014-03-27 12:27:51 -0700 2014-03-27 13:08:06 -0700 Patch bug-130848-20140327122731.patch text/plain 1478 savagobr U3VidmVyc2lvbiBSZXZpc2lvbjogMTY2MzU4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYjI0MTIxNzU2NTEzMTA3 ZDczZjk3N2QwZGY3ZmE1NjI3YzFlMzEzNS4uYzdlNjllZWY0ZTNkM2FiYWE5M2FkYjJjYjM4YmI0 YjQ5MjZmZDA0NCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE0LTAzLTI3ICBBZGVu aWxzb24gQ2F2YWxjYW50aSAgPGNhdmFsY2FudGlpQGdtYWlsLmNvbT4KKworICAgICAgICBSZW1v dmUgY29tbWVudCBmcm9tIEZpbHRlci5oCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMzA4NDgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBFeHBsb3JpbmcgdGhlIHVzZSBvZiBjb25zdHMgb24gYXBwbHlTY2Fs ZSgpIG1ldGhvZHMgZGlkbid0IHlpZWxkCisgICAgICAgIGdvb2QgcmVzdWx0cy4KKworICAgICAg ICBObyBuZXcgdGVzdHMsIG5vIGNoYW5nZSBvbiBiZWhhdmlvci4KKworICAgICAgICAqIHBsYXRm b3JtL2dyYXBoaWNzL2ZpbHRlcnMvRmlsdGVyLmg6CisKIDIwMTQtMDMtMjcgIEtyenlzenRvZiBD emVjaCAgPGsuY3plY2hAc2Ftc3VuZy5jb20+CiAKICAgICAgICAgQVg6IFJldHVybnMgY29uc3Qg cmVmZXJlbmNlIHRvIHN0YXRpYyBzdHJpbmcuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9maWx0ZXJzL0ZpbHRlci5oIGIvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3MvZmlsdGVycy9GaWx0ZXIuaAppbmRleCA1NjQzZGVmNDA1NzkwMzgzMjFjMDc4 M2UwYmY1YzEwNzU0OWI4NjQ5Li4wNzRiMzBjMmEzM2E3NWU5NTU0MjYxZTQyNTY5MTJiZDE2M2Rh MGI1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9maWx0ZXJz L0ZpbHRlci5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2ZpbHRlcnMv RmlsdGVyLmgKQEAgLTQ2LDcgKzQ2LDYgQEAgcHVibGljOgogCiAgICAgdmlydHVhbCBib29sIGlz U1ZHRmlsdGVyKCkgY29uc3QgeyByZXR1cm4gZmFsc2U7IH0KIAotICAgIC8vIFRPRE86IHZlcmlm eSBkZXNjZW5kYW50cyB0aGF0IG92ZXJsb2FkIHRoaXMgbWV0aG9kIGFuZCB0dXJuIHRoZW0gdG8g Y29uc3QuCiAgICAgdmlydHVhbCBmbG9hdCBhcHBseUhvcml6b250YWxTY2FsZShmbG9hdCB2YWx1 ZSkgY29uc3QgeyByZXR1cm4gdmFsdWUgKiBtX2ZpbHRlclJlc29sdXRpb24ud2lkdGgoKTsgfQog ICAgIHZpcnR1YWwgZmxvYXQgYXBwbHlWZXJ0aWNhbFNjYWxlKGZsb2F0IHZhbHVlKSBjb25zdCB7 IHJldHVybiB2YWx1ZSAqIG1fZmlsdGVyUmVzb2x1dGlvbi5oZWlnaHQoKTsgfQogICAgIAo=