Bug 66299 – [Qt] ImageDiff does not consider --tolerance

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Balazs Kelemen]

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

[Balazs Kelemen]

Created attachment 104035 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 104035 [details]
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.

[Balazs Kelemen]

(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.

[Csaba Osztrogonác]

(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.

[Balazs Kelemen]

Committed r93509: <http://trac.webkit.org/changeset/93509>