I was checking the diff image for the following ref test that is failing by 1-pixel test: https://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20%28Tests%29/r192415%20%2812243%29/imported/blink/fast/canvas/canvas-clip-stack-persistence-diffs.html As you can check, on the diff image is hard to see what is different (everything looks black). So I tried to manually to create a diff image using imagemagick as follows: $ wget https://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20%28Tests%29/r192415%20%2812243%29/imported/blink/fast/canvas/canvas-clip-stack-persistence-actual.png $ wget https://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20%28Tests%29/r192415%20%2812243%29/imported/blink/fast/canvas/canvas-clip-stack-persistence-expected.png $ composite canvas-clip-stack-persistence-expected.png canvas-clip-stack-persistence-actual.png -compose difference diff.png The diff.png image generated was like the current we produce. Hard to tell if there is any difference or where. The I normalized the diff image as follows: $ convert diff.png -auto-level diff_norm.png Now the diff_norm.png shows clearly where the problem is. So I think we can teach ImageDiff to auto normalize the diff image. I see that each ports seems to have its own version of ImageDiff. I only tested the GTK one, and opening the bug for this one. A quick grep suggests that other ports are already normalizing the diff image: $ find Tools/ -type f -iname "imagediff*.*cpp"|xargs grep -i normalize
(In reply to comment #0) > The I normalized the diff image as follows: > > $ convert diff.png -auto-level diff_norm.png > > Now the diff_norm.png shows clearly where the problem is. > This is the result: http://people.igalia.com/clopez/wkbug/151261/canvas-clip-stack-persistence-diff_normalized.png
We should definitely do that! There are a lot of 1-pixel diff test cases that fail from time to time.
Created attachment 265855 [details] Patch
Comment on attachment 265855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265855&action=review Can't we immediately unskip a bunch of tests? > Tools/ImageDiff/gtk/ImageDiff.cpp:85 > + bufferPixel /= maxDistance; What guarantees that maxDistance is not 0?
(In reply to comment #4) > Comment on attachment 265855 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=265855&action=review > > Can't we immediately unskip a bunch of tests? No, this doesn't fix any test, this only makes it easier to see the differences when they are minimum, see the examples Carlos posted here, without the patch you see a diff that looks completely black, while after the patch you easily see the pixel that is different. > > Tools/ImageDiff/gtk/ImageDiff.cpp:85 > > + bufferPixel /= maxDistance; > > What guarantees that maxDistance is not 0? I think differenceImageFromDifferenceBuffer wouldn't be called in that case, I could ensure it just in case
Committed r192728: <http://trac.webkit.org/changeset/192728>