Bug 66299

Summary: [Qt] ImageDiff does not consider --tolerance
Product: WebKit Reporter: Balazs Kelemen <kbalazs>
Component: Layout and RenderingAssignee: Balazs Kelemen <kbalazs>
Status: RESOLVED FIXED    
Severity: Normal CC: ossy
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Balazs Kelemen 2011-08-16 07:05:36 PDT
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
Comment 1 Balazs Kelemen 2011-08-16 07:10:27 PDT
Created attachment 104035 [details]
Patch
Comment 2 Csaba Osztrogon√°c 2011-08-22 06:14:05 PDT
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.
Comment 3 Balazs Kelemen 2011-08-22 07:17:55 PDT
(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.
Comment 4 Csaba Osztrogon√°c 2011-08-22 07:35:16 PDT
(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.
Comment 5 Balazs Kelemen 2011-08-22 08:17:29 PDT
Committed r93509: <http://trac.webkit.org/changeset/93509>