⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 284870 in webkit


Ignore:
Timestamp:
Oct 26, 2021, 8:55:48 AM (5 years ago)
Author:
Simon Fraser
Message:

Have ImageDiff print the diff image when any pixel is different
​https://bugs.webkit.org/show_bug.cgi?id=232294

Reviewed by Martin Robinson.

ImageDiff currently only outputs the diff image when any pixel exceeds its built-in
tolerance.

To prepare for moving the "pass/fail" decision to script, have ImageDiff output the diff
image when any pixel is different. Also have it write "#EOF" so that we're not reliant on
the "diff:" line to terminate reading the output.

Fix up webkitpy unit tests for #EOF parsing, presence of image when the test passes via
tolerance, and to actually test which image data is present in the ImageDiffResult.

  • ImageDiff/ImageDiff.cpp:

(processImages):
(main):

  • ImageDiff/PlatformImage.cpp:

(ImageDiff::PlatformImage::difference): Track legacyDistanceMax if any pixel diff is non-zero,
since it's needed to scale the diff image.

  • Scripts/webkitpy/port/image_diff.py:

(ImageDiffer._read): Look for "#EOF" to terminate the output. Save the diff image, even
if the test passed.

  • Scripts/webkitpy/port/port_testcase.py:

(PortTestCase.test_diff_image.make_proc):
(PortTestCase.test_diff_image):
(PortTestCase.test_diff_image_passed):
(PortTestCase.test_diff_image_failed):
(PortTestCase.test_diff_image_crashed):

Location:
trunk/Tools
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r284866 r284870  
     12021-10-26  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Have ImageDiff print the diff image when any pixel is different
     4        https://bugs.webkit.org/show_bug.cgi?id=232294
     5
     6        Reviewed by Martin Robinson.
     7       
     8        ImageDiff currently only outputs the diff image when any pixel exceeds its built-in
     9        tolerance.
     10
     11        To prepare for moving the "pass/fail" decision to script, have ImageDiff output the diff
     12        image when any pixel is different. Also have it write "#EOF" so that we're not reliant on
     13        the "diff:" line to terminate reading the output.
     14
     15        Fix up webkitpy unit tests for #EOF parsing, presence of image when the test passes via
     16        tolerance, and to actually test which image data is present in the ImageDiffResult.
     17
     18        * ImageDiff/ImageDiff.cpp:
     19        (processImages):
     20        (main):
     21        * ImageDiff/PlatformImage.cpp:
     22        (ImageDiff::PlatformImage::difference): Track legacyDistanceMax if any pixel diff is non-zero,
     23        since it's needed to scale the diff image.
     24        * Scripts/webkitpy/port/image_diff.py:
     25        (ImageDiffer._read): Look for "#EOF" to terminate the output. Save the diff image, even
     26        if the test passed.
     27        * Scripts/webkitpy/port/port_testcase.py:
     28        (PortTestCase.test_diff_image.make_proc):
     29        (PortTestCase.test_diff_image):
     30        (PortTestCase.test_diff_image_passed):
     31        (PortTestCase.test_diff_image_failed):
     32        (PortTestCase.test_diff_image_crashed):
     33
    1342021-10-26  Simon Fraser  <simon.fraser@apple.com>
    235

  • trunk/Tools/ImageDiff/ImageDiff.cpp

    r284866 r284870  
    7474    }
    7575
     76    if (diffImage)
     77        diffImage->writeAsPNGToStdout();
     78
    7679    if (legacyDifference > 0.0f) {
    77         if (diffImage)
    78             diffImage->writeAsPNGToStdout();
    7980        fprintf(stdout, "diff: %01.2f%% failed\n", legacyDifference);
    8081    } else
    … …  
    8384    if (printDifference)
    8485        fprintf(stdout, "maxDifference=%u; totalPixels=%lu\n", differenceData.maxDifference, differenceData.totalPixels);
     86
     87    fprintf(stdout, "#EOF\n");
     88    fflush(stdout);
    8589
    8690    return EXIT_SUCCESS;
    … …  
    223227                return result;
    224228        }
    225         fflush(stdout);
    226229    }
    227230

  • trunk/Tools/ImageDiff/PlatformImage.cpp

    r284764 r284870  
    7474                unsigned maxDiff = std::max({ redDiff, greenDiff, blueDiff });
    7575                difference.maxDifference = std::max(difference.maxDifference, maxDiff);
     76
     77                legacyDistanceMax = std::max(legacyDistanceMax, legacyDistance);
    7678            }
    7779
    … …  
    8082                ++pixelCountWithSignificantDifference;
    8183                legacyDistanceSum += legacyDistance;
    82                 legacyDistanceMax = std::max(legacyDistanceMax, legacyDistance);
    8384            }
    8485
    … …  
    9495        difference.percentageDifference = 0.0f;
    9596
    96     if (!pixelCountWithSignificantDifference) {
    97         free(diffBuffer);
    98         return nullptr;
    99     }
    100 
    101     // Generate a normalized diff image if there is any difference.
    102     if (pixelCountWithSignificantDifference) {
     97    if (difference.totalPixels) {
    10398        diffPixel = reinterpret_cast<unsigned char*>(diffBuffer);
    10499        for (size_t p = 0; p < height * width; ++p)
    105100            diffPixel[p] /= legacyDistanceMax;
     101
     102        return PlatformImage::createFromDiffData(diffBuffer, width, height);
    106103    }
    107104
    108     return PlatformImage::createFromDiffData(diffBuffer, width, height);
     105    free(diffBuffer);
     106    return nullptr;
    109107}
    110108

  • trunk/Tools/Scripts/webkitpy/port/image_diff.py

    r284866 r284870  
    102102    def _read(self):
    103103        deadline = time.time() + 2.0
    104         output = None
    105         output_image = b''
     104        output_image = None
     105        diff_output = None
    106106
    107107        while not self._process.timed_out and not self._process.has_crashed():
    … …  
    110110                break
    111111
    112             if output.startswith(b'diff'):  # This is the last line ImageDiff prints.
     112            if output.startswith(b'#EOF'):
    113113                break
     114
     115            if output.startswith(b'diff:'):
     116                diff_output = output
    114117
    115118            if output.startswith(b'Content-Length'):
    … …  
    117120                content_length = int(string_utils.decode(m.group(1), target_type=str))
    118121                output_image = self._process.read_stdout(deadline, content_length)
    119                 output = self._process.read_stdout_line(deadline)
    120                 break
    121122
    122123        stderr = string_utils.decode(self._process.pop_all_buffered_stderr(), target_type=str)
    … …  
    130131
    131132        diff_percent = 0
    132         if output and output.startswith(b'diff'):
    133             m = re.match(b'diff: (.+)% (passed|failed)', output)
     133        if diff_output:
     134            m = re.match(b'diff: (.+)% (passed|failed)', diff_output)
    134135            if m.group(2) == b'passed':
    135                 return ImageDiffResult(passed=True, diff_image=None, difference=0)
     136                return ImageDiffResult(passed=True, diff_image=output_image, difference=0)
    136137            diff_percent = float(string_utils.decode(m.group(1), target_type=str))
    137138

  • trunk/Tools/Scripts/webkitpy/port/port_testcase.py

    r284866 r284870  
    285285
    286286        def make_proc(port, nm, cmd, env, crash_message=None):
    287             self.proc = MockServerProcess(port, nm, cmd, env, lines=['diff: 100% failed\n', 'diff: 100% failed\n'])
     287            self.proc = MockServerProcess(port, nm, cmd, env, lines=['Content-Length: 6\n', 'image1', 'diff: 90% failed\n', '#EOF\n', 'Content-Length: 6\n', 'image2', 'diff: 100% failed\n', '#EOF\n'])
    288288            return self.proc
    289289
    … …  
    298298        self.assertFalse(port._should_use_jhbuild())
    299299
    300         self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
     300        self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'image1', difference=90.0, tolerance=0.1))
    301301        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0.1"])
    302302
    303         self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=None), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
     303        self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=None), ImageDiffResult(passed=False, diff_image=b'image1', difference=90.0, tolerance=0.1))
    304304        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0.1"])
    305305
    306         self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=0), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
     306        self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=0), ImageDiffResult(passed=False, diff_image=b'image1', difference=90.0))
    307307        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0"])
    308308
    … …  
    311311        self.assertTrue(port._should_use_jhbuild())
    312312
    313         self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
     313        self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'image1', difference=90.0, tolerance=0.1))
    314314        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0.1"])
    315315
    316         self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=None), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
     316        self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=None), ImageDiffResult(passed=False, diff_image=b'image1', difference=90.0, tolerance=0.1))
    317317        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0.1"])
    318318
    319         self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=0), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
     319        self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=0), ImageDiffResult(passed=False, diff_image=b'image1', difference=90.0))
    320320        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0"])
    321321
    … …  
    326326    def test_diff_image_passed(self):
    327327        port = self.make_port()
    328         port._server_process_constructor = lambda port, nm, cmd, env, crash_message=None: MockServerProcess(lines=['diff: 0% passed\n'])
     328        port._server_process_constructor = lambda port, nm, cmd, env, crash_message=None: MockServerProcess(lines=['diff: 0% passed\n', '#EOF\n'])
    329329        image_differ = ImageDiffer(port)
    330330        self.assertEqual(image_differ.diff_image(b'foo', b'bar', tolerance=0.1), ImageDiffResult(passed=True, diff_image=None, difference=0))
    … …  
    332332    def test_diff_image_failed(self):
    333333        port = self.make_port()
    334         port._server_process_constructor = lambda port, nm, cmd, env, crash_message=None: MockServerProcess(lines=['diff: 100% failed\n'])
     334        port._server_process_constructor = lambda port, nm, cmd, env, crash_message=None: MockServerProcess(lines=['Content-Length: 4\n', 'test', 'diff: 100% failed\n', '#EOF\n'])
    335335        image_differ = ImageDiffer(port)
    336         self.assertEqual(image_differ.diff_image(b'foo', b'bar', tolerance=0.1), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
     336        self.assertEqual(image_differ.diff_image(b'foo', b'bar', tolerance=0.1), ImageDiffResult(passed=False, diff_image=b'test', difference=100.0, tolerance=0.1))
    337337
    338338    def test_diff_image_crashed(self):
    … …  
    350350        port._server_process_constructor = make_proc
    351351        port.setup_test_run()
    352         self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'', difference=0, tolerance=0.1, error_string='ImageDiff crashed\n'))
     352        self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=None, difference=0, tolerance=0.1, error_string='ImageDiff crashed\n'))
    353353        port.clean_up_test_run()
    354354
Note: See TracChangeset for help on using the changeset viewer.