Bug 66299 - [Qt] ImageDiff does not consider --tolerance
Summary: [Qt] ImageDiff does not consider --tolerance
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Balazs Kelemen
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2011-08-16 07:05 PDT by Balazs Kelemen
Modified: 2011-08-22 08:19 PDT (History)
1 user (show)

See Also:


Attachments
Patch (1.43 KB, patch)
2011-08-16 07:10 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>