66299 2011-08-16 07:05:36 -0700 [Qt] ImageDiff does not consider --tolerance 2011-08-22 08:19:34 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Qt, QtTriaged P2 Normal --- 1 kbalazs kbalazs ossy oldest_to_newest 451590 0 kbalazs 2011-08-16 07:05:36 -0700 The way I propose in the following to reproduce this is depend on the patch in #66283. Run "Tools/Scripts/run-webkit-tests -p --tolerance 2 -2 LayoutTests/animations/missing-values-first-keyframe.html" Result: expectation failure with a diff image that looks like full black. Expected result: pass 451592 1 104035 kbalazs 2011-08-16 07:10:27 -0700 Created attachment 104035 Patch 454555 2 104035 ossy 2011-08-22 06:14:05 -0700 Comment on attachment 104035 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104035&action=review > Tools/DumpRenderTree/qt/ImageDiff.cpp:128 > + if (!difference) > + fprintf(stdout, "diff: %01.2f%% passed\n", 0.0); > + else { Good catch, thanks for the fix. - I think (difference==0) would be more talkative. - Modifying difference to 0.0 is unnecessary. All ports use this copy/paste code. :) r=me with these changes. 454582 3 kbalazs 2011-08-22 07:17:55 -0700 (In reply to comment #2) > (From update of attachment 104035 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104035&action=review > > > Tools/DumpRenderTree/qt/ImageDiff.cpp:128 > > + if (!difference) > > + fprintf(stdout, "diff: %01.2f%% passed\n", 0.0); > > + else { > > Good catch, thanks for the fix. > > - I think (difference==0) would be more talkative. Personally I agree but '== 0' is not WebKit style. > - Modifying difference to 0.0 is unnecessary. All ports use this copy/paste code. :) But than how could I test it in the if? > > r=me with these changes. 454595 4 ossy 2011-08-22 07:35:16 -0700 (In reply to comment #3) >> - I think (difference==0) would be more talkative. > Personally I agree but '== 0' is not WebKit style. Ooops, you're right. Negating a double value is strange for me, but it works. :) So use !difference to make style bot happy. ;) > But than how could I test it in the if? I meant the printf line. 454608 5 kbalazs 2011-08-22 08:17:29 -0700 Committed r93509: <http://trac.webkit.org/changeset/93509> 104035 2011-08-16 07:10:27 -0700 2011-08-22 08:19:34 -0700 Patch bug-66299-20110816141025.patch text/plain 1467 kbalazs U3VidmVyc2lvbiBSZXZpc2lvbjogOTMwNDQKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBi L1Rvb2xzL0NoYW5nZUxvZwppbmRleCA2NjgwNmE4Mjg4YzIxNmZiMjQ4NDZlOTZiN2QxZTcyODA2 ZDJiZWQ2Li4yN2I0MWYyOTA2MzIxMGQ0YTY0NzkyOWY3ZGQxMTMxMjIyOTZkNGIyIDEwMDY0NAot LS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMTUg QEAKIDIwMTEtMDgtMTYgIEJhbGF6cyBLZWxlbWVuICA8a2JhbGF6c0B3ZWJraXQub3JnPgogCisg ICAgICAgIFtRdF0gSW1hZ2VEaWZmIGRvZXMgbm90IGNvbnNpZGVyIC0tdG9sZXJhbmNlCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02NjI5OQorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogRHVtcFJlbmRlclRy ZWUvcXQvSW1hZ2VEaWZmLmNwcDoKKyAgICAgICAgKG1haW4pOgorCisyMDExLTA4LTE2ICBCYWxh enMgS2VsZW1lbiAgPGtiYWxhenNAd2Via2l0Lm9yZz4KKwogICAgICAgICBbUXRdW1dLMl0gQWRk IHBpeGVsIHRlc3Qgc3VwcG9ydAogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9NjYyODMKIApkaWZmIC0tZ2l0IGEvVG9vbHMvRHVtcFJlbmRlclRyZWUvcXQv SW1hZ2VEaWZmLmNwcCBiL1Rvb2xzL0R1bXBSZW5kZXJUcmVlL3F0L0ltYWdlRGlmZi5jcHAKaW5k ZXggZGVmMTI5NDlkOWY5MmMwMjFlMjBhZWQ1MmFhYTk2M2M2MzcxMGUyNi4uNDQ3MDFiZTU4NzVh Y2JmMmRlYjgzNGQzNzE4M2UzYTY1MGIzNmE1ZiAxMDA2NDQKLS0tIGEvVG9vbHMvRHVtcFJlbmRl clRyZWUvcXQvSW1hZ2VEaWZmLmNwcAorKysgYi9Ub29scy9EdW1wUmVuZGVyVHJlZS9xdC9JbWFn ZURpZmYuY3BwCkBAIC0xMjMsOSArMTIzLDkgQEAgaW50IG1haW4oaW50IGFyZ2MsIGNoYXIqIGFy Z3ZbXSkKICAgICAgICAgICAgICAgICAgICAgZGlmZmVyZW5jZSA9IHFNYXgoZGlmZmVyZW5jZSwg cXJlYWwoMC4wMSkpOwogICAgICAgICAgICAgICAgIH0KIAotICAgICAgICAgICAgICAgIGlmICgh Y291bnQpIHsKLSAgICAgICAgICAgICAgICAgICAgZnByaW50ZihzdGRvdXQsICJkaWZmOiAlMDEu MmYlJSBwYXNzZWRcbiIsIGRpZmZlcmVuY2UpOwotICAgICAgICAgICAgICAgIH0gZWxzZSB7Cisg ICAgICAgICAgICAgICAgaWYgKCFkaWZmZXJlbmNlKQorICAgICAgICAgICAgICAgICAgICBmcHJp bnRmKHN0ZG91dCwgImRpZmY6ICUwMS4yZiUlIHBhc3NlZFxuIiwgMC4wKTsKKyAgICAgICAgICAg ICAgICBlbHNlIHsKICAgICAgICAgICAgICAgICAgICAgUUJ1ZmZlciBidWZmZXI7CiAgICAgICAg ICAgICAgICAgICAgIGJ1ZmZlci5vcGVuKFFCdWZmZXI6OldyaXRlT25seSk7CiAgICAgICAgICAg ICAgICAgICAgIGRpZmZJbWFnZS5zYXZlKCZidWZmZXIsICJQTkciKTsK