Bug 89253 - Makes ImageDiff reflect size of pixel delta in the alpha channel of the resulting red pixel.
Summary: Makes ImageDiff reflect size of pixel delta in the alpha channel of the resul...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-15 15:52 PDT by Tony Payne
Modified: 2012-06-15 16:17 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.80 KB, patch)
2012-06-15 15:54 PDT, Tony Payne
jamesr: review-
Details | Formatted Diff | Diff
Sample output (48.58 KB, image/png)
2012-06-15 16:05 PDT, Tony Payne
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Payne 2012-06-15 15:52:53 PDT
Makes ImageDiff reflect size of pixel delta in the alpha channel of the resulting red pixel.
Comment 1 Tony Payne 2012-06-15 15:54:16 PDT
Created attachment 147916 [details]
Patch
Comment 2 James Robinson 2012-06-15 16:02:50 PDT
Comment on attachment 147916 [details]
Patch

Could we make this a mode rather than something that always happens?  Bright red is something that I think most people who look at image diffs are already pretty familiar with - looking for a subtle failure by looking for a every so slightly red pixel sounds like it could be a pain.

Mind posting some pictures of what this does?
Comment 3 Tony Payne 2012-06-15 16:05:58 PDT
Created attachment 147919 [details]
Sample output
Comment 4 Tony Payne 2012-06-15 16:07:14 PDT
(In reply to comment #2)
> (From update of attachment 147916 [details])
> Could we make this a mode rather than something that always happens?  Bright red is something that I think most people who look at image diffs are already pretty familiar with - looking for a subtle failure by looking for a every so slightly red pixel sounds like it could be a pain.
> 
> Mind posting some pictures of what this does?

Sample output attached. I don't think making it optional is really necessary. I set the lower bound to 20% opacity, so even minute changes are still readily noticeable. Notice how much more useful this output is than a square block of red. You can see that the text is missing *and* the background is slightly off.
Comment 5 James Robinson 2012-06-15 16:07:17 PDT
Comment on attachment 147916 [details]
Patch

Yeah, that's pretty but it's too hard to tell from a non-failure for gardening use IMO.  I also don't want to have different behaviors for chromium ImageDiff vs "normal" ImageDiff.

Some sort of flag or option, perhaps?
Comment 6 Tony Payne 2012-06-15 16:11:18 PDT
(In reply to comment #5)
> (From update of attachment 147916 [details])
> Yeah, that's pretty but it's too hard to tell from a non-failure for gardening use IMO.  I also don't want to have different behaviors for chromium ImageDiff vs "normal" ImageDiff.
> 
> Some sort of flag or option, perhaps?

Define "normal" ImageDiff. Looking at the gtk and win version, it looks like both already do something very similar.
Comment 7 James Robinson 2012-06-15 16:17:01 PDT
I was just thinking of Apple and Chromium's.  What do the gtk/win ImageDiffs look like?

I think if you want to propose changing overall behavior it's worth mailing webkit-dev@ and soliciting opinions from more people who look at these diffs all day.