Summary: | [Qt] Make ImageDiff similar to Chromium's ImageDiff | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||||||||
Component: | New Bugs | Assignee: | Csaba Osztrogonác <ossy> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | allan.jensen, jturcotte | ||||||||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 94800 | ||||||||||||||
Attachments: |
|
Description
Csaba Osztrogonác
2012-08-22 23:42:06 PDT
Created attachment 160095 [details]
WIP patch
This sounds like the compare util from ImageMagics, see http://www.imagemagick.org/Usage/compare/ Can we possibly use a standard tool like that? Created attachment 176739 [details]
Patch
A variant of the original idea, combining the measures.
Comment on attachment 176739 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176739&action=review > Tools/ChangeLog:1 > +2012-11-29 Allan Sandfeld Jensen <allan.jensen@digia.com> I think you can give a bit of credit to Csaba for this one. I can rubber-stamp the patch if he wants to review it. (In reply to comment #4) > I think you can give a bit of credit to Csaba for this one. > I can rubber-stamp the patch if he wants to review it. Unless the two patches are totally different, but from my quick look it seems like an iteration over the previous patch. Created attachment 176750 [details]
Patch
Created attachment 182102 [details]
Patch
rebased after imagediff moved
Comment on attachment 182102 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182102&action=review > Tools/ImageDiff/qt/ImageDiff.cpp:41 > + if (args[i] == "-f" || args[i] == "--fuzz") fuzz itself doesn't tell much about what it does. A comment here would be nice to explain its effects, until we get a "--help" (if ever). > Tools/ImageDiff/qt/ImageDiff.cpp:115 > count++; It seems like count had to be increased each time sum is. At this point though I think that we could remove count completely, seeing the change in bug #66299. > Tools/ImageDiff/qt/ImageDiff.cpp:119 > sum += distance; The fuzz then only applies to the visual result, it doesn't affect the difference ratio. Is this intended? If yes a note somewhere clarifying this could help. (In reply to comment #8) > (From update of attachment 182102 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=182102&action=review > > > Tools/ImageDiff/qt/ImageDiff.cpp:41 > > + if (args[i] == "-f" || args[i] == "--fuzz") > > fuzz itself doesn't tell much about what it does. > A comment here would be nice to explain its effects, until we get a "--help" (if ever). > I will remove it for now. It is dead code until we actually use it. > > Tools/ImageDiff/qt/ImageDiff.cpp:115 > > count++; > > It seems like count had to be increased each time sum is. At this point though I think that we could remove count completely, seeing the change in bug #66299. > It is an error count. > > Tools/ImageDiff/qt/ImageDiff.cpp:119 > > sum += distance; > > The fuzz then only applies to the visual result, it doesn't affect the difference ratio. > Is this intended? If yes a note somewhere clarifying this could help. Yeah. This is part of the hybrid approach. Fuzz indicated when 0.5 was added on top of a pixel difference. Created attachment 193831 [details]
Patch
Comment on attachment 193831 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193831&action=review > Tools/ImageDiff/qt/ImageDiff.cpp:35 > + qreal fuzz = 0; // Tolerated difference in color to indicate a pixel error. Rests of a previous patch? > Tools/ImageDiff/qt/ImageDiff.cpp:41 > - for (int i = 0; i < argc; ++i) > + for (int i = 0; i < argc; ++i) { > if (args[i] == "-t" || args[i] == "--tolerance") > tolerance = args[i + 1].toDouble(); > + } ditto > Tools/ImageDiff/qt/ImageDiff.cpp:109 > + if (distance > fuzz) { ditto Comment on attachment 193831 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193831&action=review >> Tools/ImageDiff/qt/ImageDiff.cpp:109 >> + if (distance > fuzz) { > > ditto r=me if you replace this with distance > 0 and remove the fuzz variable. Committed r146206: <http://trac.webkit.org/changeset/146206> |