91499 2012-07-17 06:18:40 -0700 double data type shoud be changed to float in ColorMatrix 2012-07-20 23:07:39 -0700 1 1 1 Unclassified WebKit SVG 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 tczene webkit-unassigned ap thorton webkit.review.bot zherczeg zimmermann oldest_to_newest 671223 0 tczene 2012-07-17 06:18:40 -0700 According to tests double type is unnecessary. SVG Filters use float type everywhere so we shoud change double to float. 671228 1 152751 tczene 2012-07-17 06:24:03 -0700 Created attachment 152751 proposed patch 672620 2 ap 2012-07-18 12:27:43 -0700 What is the expected benefit of this change? Is it matching other code, or something else? 673299 3 tczene 2012-07-19 01:31:41 -0700 Opengl ES 2.0, ARM NEON, OpenCL don't support double type. 673541 4 ap 2012-07-19 08:52:29 -0700 Please elaborate. This patch changes C++ code, not OpenGL or OpenCL. 674529 5 tczene 2012-07-20 07:01:36 -0700 For this filter doesn't need double precision and it confused me when I implemented the opencl/gl version. The goal is to uniform the codes. 674629 6 ap 2012-07-20 09:21:38 -0700 Thank you for the explanation. I'll leave final review to someone better familiar with this code, and with performance characteristics of double vs. float on modern processors. 675174 7 152751 zimmermann 2012-07-20 22:50:28 -0700 Comment on attachment 152751 proposed patch Looks okay to me - all tests still pass, so we're not affected by precision issues, so rs=me. 675178 8 152751 webkit.review.bot 2012-07-20 23:07:34 -0700 Comment on attachment 152751 proposed patch Clearing flags on attachment: 152751 Committed r123288: <http://trac.webkit.org/changeset/123288> 675179 9 webkit.review.bot 2012-07-20 23:07:39 -0700 All reviewed patches have been landed. Closing bug. 152751 2012-07-17 06:24:03 -0700 2012-07-20 23:07:34 -0700 proposed patch float.patch text/plain 5609 tczene ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA4MTc3NDJmLi5iNDI0YTgyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcg QEAKKzIwMTItMDctMTcgIEN6ZW5lIFRhbcOhcyAgPHRjemVuZUBpbmYudS1zemVnZWQuaHU+CisK KyAgICAgICAgZG91YmxlIGRhdGEgdHlwZSBzaG91ZCBiZSBjaGFuZ2VkIHRvIGZsb2F0IGluIENv bG9yTWF0cml4CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD05MTQ5OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogcGxhdGZvcm0vZ3JhcGhpY3MvZmlsdGVycy9GRUNvbG9yTWF0cml4LmNwcDoKKyAgICAgICAg KFdlYkNvcmU6Om1hdHJpeCk6CisgICAgICAgIChXZWJDb3JlOjpzYXR1cmF0ZSk6CisgICAgICAg IChXZWJDb3JlOjpodWVyb3RhdGUpOgorICAgICAgICAoV2ViQ29yZTo6bHVtaW5hbmNlKToKKyAg ICAgICAgKFdlYkNvcmU6OmVmZmVjdFR5cGUpOgorCiAyMDEyLTA3LTE3ICBWaXZlayBHYWxhdGFn ZSAgPHZpdmVrZ2FsYXRhZ2VAZ21haWwuY29tPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IHJl ZmFjdG9yIEluc3BlY3RvckNvbnRyb2xsZXI6OmNvbm5lY3RGcm9udGVuZCgpIHRvIGFjY2VwdCBJ bnNwZWN0b3JGcm9udGVuZENoYW5uZWwuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9maWx0ZXJzL0ZFQ29sb3JNYXRyaXguY3BwIGIvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vZ3JhcGhpY3MvZmlsdGVycy9GRUNvbG9yTWF0cml4LmNwcAppbmRleCBiZDQyMTJk Li4yNWIwODdlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9m aWx0ZXJzL0ZFQ29sb3JNYXRyaXguY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dy YXBoaWNzL2ZpbHRlcnMvRkVDb2xvck1hdHJpeC5jcHAKQEAgLTczLDEyICs3MywxMiBAQCBib29s IEZFQ29sb3JNYXRyaXg6OnNldFZhbHVlcyhjb25zdCBWZWN0b3I8ZmxvYXQ+ICZ2YWx1ZXMpCiAg ICAgcmV0dXJuIHRydWU7CiB9CiAKLWlubGluZSB2b2lkIG1hdHJpeChkb3VibGUmIHJlZCwgZG91 YmxlJiBncmVlbiwgZG91YmxlJiBibHVlLCBkb3VibGUmIGFscGhhLCBjb25zdCBWZWN0b3I8Zmxv YXQ+JiB2YWx1ZXMpCitpbmxpbmUgdm9pZCBtYXRyaXgoZmxvYXQmIHJlZCwgZmxvYXQmIGdyZWVu LCBmbG9hdCYgYmx1ZSwgZmxvYXQmIGFscGhhLCBjb25zdCBWZWN0b3I8ZmxvYXQ+JiB2YWx1ZXMp CiB7Ci0gICAgZG91YmxlIHIgPSB2YWx1ZXNbMF0gICogcmVkICsgdmFsdWVzWzFdICAqIGdyZWVu ICsgdmFsdWVzWzJdICAqIGJsdWUgKyB2YWx1ZXNbM10gICogYWxwaGEgKyB2YWx1ZXNbNF0gKiAy NTU7Ci0gICAgZG91YmxlIGcgPSB2YWx1ZXNbNV0gICogcmVkICsgdmFsdWVzWzZdICAqIGdyZWVu ICsgdmFsdWVzWzddICAqIGJsdWUgKyB2YWx1ZXNbOF0gICogYWxwaGEgKyB2YWx1ZXNbOV0gKiAy NTU7Ci0gICAgZG91YmxlIGIgPSB2YWx1ZXNbMTBdICAqIHJlZCArIHZhbHVlc1sxMV0gICogZ3Jl ZW4gKyB2YWx1ZXNbMTJdICogYmx1ZSArIHZhbHVlc1sxM10gKiBhbHBoYSArIHZhbHVlc1sxNF0g KiAyNTU7Ci0gICAgZG91YmxlIGEgPSB2YWx1ZXNbMTVdICogcmVkICsgdmFsdWVzWzE2XSAqIGdy ZWVuICsgdmFsdWVzWzE3XSAqIGJsdWUgKyB2YWx1ZXNbMThdICogYWxwaGEgKyB2YWx1ZXNbMTld ICogMjU1OworICAgIGZsb2F0IHIgPSB2YWx1ZXNbMF0gICogcmVkICsgdmFsdWVzWzFdICAqIGdy ZWVuICsgdmFsdWVzWzJdICAqIGJsdWUgKyB2YWx1ZXNbM10gICogYWxwaGEgKyB2YWx1ZXNbNF0g KiAyNTU7CisgICAgZmxvYXQgZyA9IHZhbHVlc1s1XSAgKiByZWQgKyB2YWx1ZXNbNl0gICogZ3Jl ZW4gKyB2YWx1ZXNbN10gICogYmx1ZSArIHZhbHVlc1s4XSAgKiBhbHBoYSArIHZhbHVlc1s5XSAq IDI1NTsKKyAgICBmbG9hdCBiID0gdmFsdWVzWzEwXSAgKiByZWQgKyB2YWx1ZXNbMTFdICAqIGdy ZWVuICsgdmFsdWVzWzEyXSAqIGJsdWUgKyB2YWx1ZXNbMTNdICogYWxwaGEgKyB2YWx1ZXNbMTRd ICogMjU1OworICAgIGZsb2F0IGEgPSB2YWx1ZXNbMTVdICogcmVkICsgdmFsdWVzWzE2XSAqIGdy ZWVuICsgdmFsdWVzWzE3XSAqIGJsdWUgKyB2YWx1ZXNbMThdICogYWxwaGEgKyB2YWx1ZXNbMTld ICogMjU1OwogCiAgICAgcmVkID0gcjsKICAgICBncmVlbiA9IGc7CkBAIC04NiwyOCArODYsMjgg QEAgaW5saW5lIHZvaWQgbWF0cml4KGRvdWJsZSYgcmVkLCBkb3VibGUmIGdyZWVuLCBkb3VibGUm IGJsdWUsIGRvdWJsZSYgYWxwaGEsIGNvbnMKICAgICBhbHBoYSA9IGE7CiB9CiAKLWlubGluZSB2 b2lkIHNhdHVyYXRlKGRvdWJsZSYgcmVkLCBkb3VibGUmIGdyZWVuLCBkb3VibGUmIGJsdWUsIGNv bnN0IGZsb2F0JiBzKQoraW5saW5lIHZvaWQgc2F0dXJhdGUoZmxvYXQmIHJlZCwgZmxvYXQmIGdy ZWVuLCBmbG9hdCYgYmx1ZSwgY29uc3QgZmxvYXQmIHMpCiB7Ci0gICAgZG91YmxlIHIgPSAoMC4y MTMgKyAwLjc4NyAqIHMpICogcmVkICsgKDAuNzE1IC0gMC43MTUgKiBzKSAqIGdyZWVuICsgKDAu MDcyIC0gMC4wNzIgKiBzKSAqIGJsdWU7Ci0gICAgZG91YmxlIGcgPSAoMC4yMTMgLSAwLjIxMyAq IHMpICogcmVkICsgKDAuNzE1ICsgMC4yODUgKiBzKSAqIGdyZWVuICsgKDAuMDcyIC0gMC4wNzIg KiBzKSAqIGJsdWU7Ci0gICAgZG91YmxlIGIgPSAoMC4yMTMgLSAwLjIxMyAqIHMpICogcmVkICsg KDAuNzE1IC0gMC43MTUgKiBzKSAqIGdyZWVuICsgKDAuMDcyICsgMC45MjggKiBzKSAqIGJsdWU7 CisgICAgZmxvYXQgciA9ICgwLjIxMyArIDAuNzg3ICogcykgKiByZWQgKyAoMC43MTUgLSAwLjcx NSAqIHMpICogZ3JlZW4gKyAoMC4wNzIgLSAwLjA3MiAqIHMpICogYmx1ZTsKKyAgICBmbG9hdCBn ID0gKDAuMjEzIC0gMC4yMTMgKiBzKSAqIHJlZCArICgwLjcxNSArIDAuMjg1ICogcykgKiBncmVl biArICgwLjA3MiAtIDAuMDcyICogcykgKiBibHVlOworICAgIGZsb2F0IGIgPSAoMC4yMTMgLSAw LjIxMyAqIHMpICogcmVkICsgKDAuNzE1IC0gMC43MTUgKiBzKSAqIGdyZWVuICsgKDAuMDcyICsg MC45MjggKiBzKSAqIGJsdWU7CiAKICAgICByZWQgPSByOwogICAgIGdyZWVuID0gZzsKICAgICBi bHVlID0gYjsKIH0KIAotaW5saW5lIHZvaWQgaHVlcm90YXRlKGRvdWJsZSYgcmVkLCBkb3VibGUm IGdyZWVuLCBkb3VibGUmIGJsdWUsIGNvbnN0IGZsb2F0JiBodWUpCitpbmxpbmUgdm9pZCBodWVy b3RhdGUoZmxvYXQmIHJlZCwgZmxvYXQmIGdyZWVuLCBmbG9hdCYgYmx1ZSwgY29uc3QgZmxvYXQm IGh1ZSkKIHsKLSAgICBkb3VibGUgY29zSHVlID0gY29zKGh1ZSAqIHBpRG91YmxlIC8gMTgwKTsg Ci0gICAgZG91YmxlIHNpbkh1ZSA9IHNpbihodWUgKiBwaURvdWJsZSAvIDE4MCk7IAotICAgIGRv dWJsZSByID0gcmVkICAgKiAoMC4yMTMgKyBjb3NIdWUgKiAwLjc4NyAtIHNpbkh1ZSAqIDAuMjEz KSArCisgICAgZmxvYXQgY29zSHVlID0gY29zKGh1ZSAqIHBpRG91YmxlIC8gMTgwKTsgCisgICAg ZmxvYXQgc2luSHVlID0gc2luKGh1ZSAqIHBpRG91YmxlIC8gMTgwKTsgCisgICAgZmxvYXQgciA9 IHJlZCAgICogKDAuMjEzICsgY29zSHVlICogMC43ODcgLSBzaW5IdWUgKiAwLjIxMykgKwogICAg ICAgICAgICAgICAgZ3JlZW4gKiAoMC43MTUgLSBjb3NIdWUgKiAwLjcxNSAtIHNpbkh1ZSAqIDAu NzE1KSArCiAgICAgICAgICAgICAgICBibHVlICAqICgwLjA3MiAtIGNvc0h1ZSAqIDAuMDcyICsg c2luSHVlICogMC45MjgpOwotICAgIGRvdWJsZSBnID0gcmVkICAgKiAoMC4yMTMgLSBjb3NIdWUg KiAwLjIxMyArIHNpbkh1ZSAqIDAuMTQzKSArCisgICAgZmxvYXQgZyA9IHJlZCAgICogKDAuMjEz IC0gY29zSHVlICogMC4yMTMgKyBzaW5IdWUgKiAwLjE0MykgKwogICAgICAgICAgICAgICAgZ3Jl ZW4gKiAoMC43MTUgKyBjb3NIdWUgKiAwLjI4NSArIHNpbkh1ZSAqIDAuMTQwKSArCiAgICAgICAg ICAgICAgICBibHVlICAqICgwLjA3MiAtIGNvc0h1ZSAqIDAuMDcyIC0gc2luSHVlICogMC4yODMp OwotICAgIGRvdWJsZSBiID0gcmVkICAgKiAoMC4yMTMgLSBjb3NIdWUgKiAwLjIxMyAtIHNpbkh1 ZSAqIDAuNzg3KSArCisgICAgZmxvYXQgYiA9IHJlZCAgICogKDAuMjEzIC0gY29zSHVlICogMC4y MTMgLSBzaW5IdWUgKiAwLjc4NykgKwogICAgICAgICAgICAgICAgZ3JlZW4gKiAoMC43MTUgLSBj b3NIdWUgKiAwLjcxNSArIHNpbkh1ZSAqIDAuNzE1KSArCiAgICAgICAgICAgICAgICBibHVlICAq ICgwLjA3MiArIGNvc0h1ZSAqIDAuOTI4ICsgc2luSHVlICogMC4wNzIpOwogCkBAIC0xMTYsNyAr MTE2LDcgQEAgaW5saW5lIHZvaWQgaHVlcm90YXRlKGRvdWJsZSYgcmVkLCBkb3VibGUmIGdyZWVu LCBkb3VibGUmIGJsdWUsIGNvbnN0IGZsb2F0JiBodWUKICAgICBibHVlID0gYjsKIH0KIAotaW5s aW5lIHZvaWQgbHVtaW5hbmNlKGRvdWJsZSYgcmVkLCBkb3VibGUmIGdyZWVuLCBkb3VibGUmIGJs dWUsIGRvdWJsZSYgYWxwaGEpCitpbmxpbmUgdm9pZCBsdW1pbmFuY2UoZmxvYXQmIHJlZCwgZmxv YXQmIGdyZWVuLCBmbG9hdCYgYmx1ZSwgZmxvYXQmIGFscGhhKQogewogICAgIGFscGhhID0gMC4y MTI1ICogcmVkICsgMC43MTU0ICogZ3JlZW4gKyAwLjA3MjEgKiBibHVlOwogICAgIHJlZCA9IDA7 CkBAIC0xMjksMTAgKzEyOSwxMCBAQCB2b2lkIGVmZmVjdFR5cGUoVWludDhDbGFtcGVkQXJyYXkq IHBpeGVsQXJyYXksIGNvbnN0IFZlY3RvcjxmbG9hdD4mIHZhbHVlcykKIHsKICAgICB1bnNpZ25l ZCBwaXhlbEFycmF5TGVuZ3RoID0gcGl4ZWxBcnJheS0+bGVuZ3RoKCk7CiAgICAgZm9yICh1bnNp Z25lZCBwaXhlbEJ5dGVPZmZzZXQgPSAwOyBwaXhlbEJ5dGVPZmZzZXQgPCBwaXhlbEFycmF5TGVu Z3RoOyBwaXhlbEJ5dGVPZmZzZXQgKz0gNCkgewotICAgICAgICBkb3VibGUgcmVkID0gcGl4ZWxB cnJheS0+aXRlbShwaXhlbEJ5dGVPZmZzZXQpOwotICAgICAgICBkb3VibGUgZ3JlZW4gPSBwaXhl bEFycmF5LT5pdGVtKHBpeGVsQnl0ZU9mZnNldCArIDEpOwotICAgICAgICBkb3VibGUgYmx1ZSA9 IHBpeGVsQXJyYXktPml0ZW0ocGl4ZWxCeXRlT2Zmc2V0ICsgMik7Ci0gICAgICAgIGRvdWJsZSBh bHBoYSA9IHBpeGVsQXJyYXktPml0ZW0ocGl4ZWxCeXRlT2Zmc2V0ICsgMyk7CisgICAgICAgIGZs b2F0IHJlZCA9IHBpeGVsQXJyYXktPml0ZW0ocGl4ZWxCeXRlT2Zmc2V0KTsKKyAgICAgICAgZmxv YXQgZ3JlZW4gPSBwaXhlbEFycmF5LT5pdGVtKHBpeGVsQnl0ZU9mZnNldCArIDEpOworICAgICAg ICBmbG9hdCBibHVlID0gcGl4ZWxBcnJheS0+aXRlbShwaXhlbEJ5dGVPZmZzZXQgKyAyKTsKKyAg ICAgICAgZmxvYXQgYWxwaGEgPSBwaXhlbEFycmF5LT5pdGVtKHBpeGVsQnl0ZU9mZnNldCArIDMp OwogCiAgICAgICAgIHN3aXRjaCAoZmlsdGVyVHlwZSkgewogICAgICAgICAgICAgY2FzZSBGRUNP TE9STUFUUklYX1RZUEVfTUFUUklYOgo=