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

Changeset 284866 in webkit


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

Don't run ImageDiff a second time to generate diff images
​https://bugs.webkit.org/show_bug.cgi?id=232288

Reviewed by Martin Robinson.

Currently, for a ref test failure (which is always run with tolerance=0), we run ImageDiff a
second time in FailureReftestMismatch.write_failure() with the intent of generating a diff
image with zero tolerance.

Fix by storing the ImageDiffResult in FailureReftestMismatch and FailureImageHashMismatch so
we already have the diff image. We only regenerate it when the first diff was run with a
non-zero tolerance (only relevant for pixel tests). To faciliate this, store the tolerance
that was used inside ImageDiffResult too.

  • ImageDiff/ImageDiff.cpp:

(main): Show tolerance in verbose logging.

  • Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:

(SingleTestRunner._compare_image):
(SingleTestRunner._compare_output_with_reference):

  • Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py:

(TestResultWriterTest.test_reftest_diff_image.ImageDiffTestPort.diff_image):
(TestResultWriterTest):
(TestResultWriterTest.test_reftest_diff_image):

  • Scripts/webkitpy/layout_tests/models/test_failures.py:

(FailureImageHashMismatch.init):
(FailureReftestMismatch.init):
(FailureReftestMismatch.write_failure):

  • Scripts/webkitpy/layout_tests/models/test_run_results.py:

(_interpret_test_failures):

  • Scripts/webkitpy/layout_tests/models/test_run_results_unittest.py:

(InterpretTestFailuresTest.test_interpret_test_failures):

  • Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:

(RunTest.test_tolerance.ImageDiffTestPort.diff_image):

  • Scripts/webkitpy/port/image_diff.py:

(ImageDiffResult.init):
(ImageDiffResult.repr):
(ImageDiffer._read):

  • Scripts/webkitpy/port/port_testcase.py:

(PortTestCase.test_diff_image):

Location:
trunk/Tools
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r284855 r284866  
     12021-10-26  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Don't run ImageDiff a second time to generate diff images
     4        https://bugs.webkit.org/show_bug.cgi?id=232288
     5
     6        Reviewed by Martin Robinson.
     7
     8        Currently, for a ref test failure (which is always run with tolerance=0), we run ImageDiff a
     9        second time in FailureReftestMismatch.write_failure() with the intent of generating a diff
     10        image with zero tolerance.
     11
     12        Fix by storing the ImageDiffResult in FailureReftestMismatch and FailureImageHashMismatch so
     13        we already have the diff image. We only regenerate it when the first diff was run with a
     14        non-zero tolerance (only relevant for pixel tests). To faciliate this, store the tolerance
     15        that was used inside ImageDiffResult too.
     16
     17        * ImageDiff/ImageDiff.cpp:
     18        (main): Show tolerance in verbose logging.
     19        * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
     20        (SingleTestRunner._compare_image):
     21        (SingleTestRunner._compare_output_with_reference):
     22        * Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py:
     23        (TestResultWriterTest.test_reftest_diff_image.ImageDiffTestPort.diff_image):
     24        (TestResultWriterTest):
     25        (TestResultWriterTest.test_reftest_diff_image):
     26        * Scripts/webkitpy/layout_tests/models/test_failures.py:
     27        (FailureImageHashMismatch.__init__):
     28        (FailureReftestMismatch.__init__):
     29        (FailureReftestMismatch.write_failure):
     30        * Scripts/webkitpy/layout_tests/models/test_run_results.py:
     31        (_interpret_test_failures):
     32        * Scripts/webkitpy/layout_tests/models/test_run_results_unittest.py:
     33        (InterpretTestFailuresTest.test_interpret_test_failures):
     34        * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
     35        (RunTest.test_tolerance.ImageDiffTestPort.diff_image):
     36        * Scripts/webkitpy/port/image_diff.py:
     37        (ImageDiffResult.__init__):
     38        (ImageDiffResult.__repr__):
     39        (ImageDiffer._read):
     40        * Scripts/webkitpy/port/port_testcase.py:
     41        (PortTestCase.test_diff_image):
     42
    1432021-10-25  Ryan Haddad  <ryanhaddad@apple.com>
    244

  • trunk/Tools/ImageDiff/ImageDiff.cpp

    r284764 r284866  
    218218        if (actualImage && baselineImage) {
    219219            if (verbose)
    220                 fprintf(stderr, "ImageDiff: processing images\n");
     220                fprintf(stderr, "ImageDiff: processing images with tolerance %01.2f%%\n", tolerance);
    221221            auto result = processImages(std::exchange(actualImage, { }), std::exchange(baselineImage, { }), tolerance, printDifference);
    222222            if (result != EXIT_SUCCESS)

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py

    r284845 r284866  
    296296            diff_result = self._port.diff_image(expected_driver_output.image, driver_output.image)
    297297            if not diff_result.passed:
    298                 failures.append(test_failures.FailureImageHashMismatch(diff_result.diff_percent))
     298                failures.append(test_failures.FailureImageHashMismatch(diff_result))
    299299                if diff_result.error_string:
    300300                    _log.warning('  %s : %s' % (self._test_name, diff_result.error_string))
    … …  
    357357            diff_result = self._port.diff_image(reference_driver_output.image, actual_driver_output.image, tolerance=0)
    358358            if not diff_result.passed:
    359                 failures.append(test_failures.FailureReftestMismatch(reference_filename))
     359                failures.append(test_failures.FailureReftestMismatch(reference_filename, diff_result))
    360360                if diff_result.error_string:
    361361                    _log.warning('  %s : %s' % (self._test_name, diff_result.error_string))

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py

    r284845 r284866  
    4141
    4242        class ImageDiffTestPort(TestPort):
    43             def diff_image(self, expected_contents, actual_contents, tolerance=None):
     43            def diff_image(self, expected_contents, actual_contents, tolerance):
    4444                used_tolerance_values.append(tolerance)
    45                 return ImageDiffResult(passed=False, diff_image=b'', difference=1)
     45                return ImageDiffResult(passed=False, diff_image=b'', difference=1, tolerance=tolerance)
    4646
    4747        host = MockHost()
    … …  
    5151        driver_output1 = DriverOutput('text1', 'image1', 'imagehash1', 'audio1')
    5252        driver_output2 = DriverOutput('text2', 'image2', 'imagehash2', 'audio2')
    53         failures = [test_failures.FailureReftestMismatch(test_reference_file)]
     53        failures = [test_failures.FailureReftestMismatch(test_reference_file, ImageDiffResult(passed=False, diff_image=b'', difference=1, tolerance=1))]
    5454        test_result_writer.write_test_result(host.filesystem, ImageDiffTestPort(host), port.results_directory(), test_name,
    5555                                             driver_output1, driver_output2, failures)

  • trunk/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py

    r284785 r284866  
    202202
    203203class FailureImageHashMismatch(TestFailure):
    204     def __init__(self, diff_percent=0):
     204    def __init__(self, image_diff_result=None):
    205205        super(FailureImageHashMismatch, self).__init__()
    206         self.diff_percent = diff_percent
     206        self.image_diff_result = image_diff_result
    207207
    208208    def message(self):
    … …  
    220220
    221221class FailureReftestMismatch(TestFailure):
    222     def __init__(self, reference_filename=None):
     222    def __init__(self, reference_filename=None, image_diff_result=None):
    223223        super(FailureReftestMismatch, self).__init__()
    224224        self.reference_filename = reference_filename
    225         self.diff_percent = None
     225        self.image_diff_result = image_diff_result
    226226
    227227    def message(self):
    … …  
    230230    def write_failure(self, writer, driver_output, expected_driver_output, port):
    231231        writer.write_image_files(driver_output.image, expected_driver_output.image)
    232         # FIXME: This work should be done earlier in the pipeline (e.g., when we compare images for non-ref tests).
    233         # FIXME: We should always have 2 images here.
    234         if driver_output.image and expected_driver_output.image:
    235             diff_result = port.diff_image(expected_driver_output.image, driver_output.image, tolerance=0)
    236             if diff_result.diff_image:
    237                 writer.write_image_diff_files(diff_result.diff_image)
    238                 self.diff_percent = diff_result.diff_percent
     232        if self.image_diff_result:
     233            # If the ref test was run with non-zero tolerance, generate the image diff again with zero tolerance.
     234            if self.image_diff_result.tolerance != 0:
     235                diff_image = port.diff_image(expected_driver_output.image, driver_output.image, tolerance=0).diff_image
    239236            else:
    240                 _log.warn('ref test mismatch did not produce an image diff.')
     237                diff_image = self.image_diff_result.diff_image
     238
     239            writer.write_image_diff_files(diff_image)
     240        else:
     241            _log.warn('ref test mismatch did not produce an image diff.')
    241242        writer.write_reftest(self.reference_filename)
    242243

  • trunk/Tools/Scripts/webkitpy/layout_tests/models/test_run_results.py

    r284784 r284866  
    214214        for failure in failures:
    215215            if isinstance(failure, test_failures.FailureImageHashMismatch) or isinstance(failure, test_failures.FailureReftestMismatch):
    216                 test_dict['image_diff_percent'] = failure.diff_percent
     216                test_dict['image_diff_percent'] = failure.image_diff_result.diff_percent
    217217
    218218    return test_dict

  • trunk/Tools/Scripts/webkitpy/layout_tests/models/test_run_results_unittest.py

    r284775 r284866  
    3434from webkitpy.layout_tests.models import test_results
    3535from webkitpy.layout_tests.models import test_run_results
     36from webkitpy.port.image_diff import ImageDiffResult
    3637from webkitpy.tool.mocktool import MockOptions
    3738
    … …  
    125126
    126127    def test_interpret_test_failures(self):
    127         test_dict = test_run_results._interpret_test_failures([test_failures.FailureImageHashMismatch(diff_percent=0.42)])
     128        test_dict = test_run_results._interpret_test_failures([test_failures.FailureImageHashMismatch(ImageDiffResult(passed=False, diff_image=b'', difference=0.42))])
    128129        self.assertEqual(test_dict['image_diff_percent'], 0.42)
    129130
    130         test_dict = test_run_results._interpret_test_failures([test_failures.FailureReftestMismatch(self.port.abspath_for_test('foo/reftest-expected.html'))])
     131        test_dict = test_run_results._interpret_test_failures([test_failures.FailureReftestMismatch(self.port.abspath_for_test('foo/reftest-expected.html'), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))])
    131132        self.assertIn('image_diff_percent', test_dict)
    132133

  • trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py

    r284855 r284866  
    753753            def diff_image(self, expected_contents, actual_contents, tolerance=None):
    754754                self.tolerance_used_for_diff_image = self._options.tolerance
    755                 return ImageDiffResult(passed=False, diff_image=b'', difference=1)
     755                return ImageDiffResult(passed=False, diff_image=b'', difference=1, tolerance=self._options.tolerance or 0)
    756756
    757757        def get_port_for_run(args):

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

    r284845 r284866  
    4141
    4242class ImageDiffResult(object):
    43     def __init__(self, passed, diff_image, difference, error_string=None):
     43    def __init__(self, passed, diff_image, difference, tolerance=0, error_string=None):
    4444        self.passed = passed
    4545        self.diff_image = diff_image
    4646        self.diff_percent = difference
     47        self.tolerance = tolerance
    4748        self.error_string = error_string
    4849
    … …  
    5253                    self.diff_image == other.diff_image and
    5354                    self.diff_percent == other.diff_percent and
     55                    self.tolerance == other.tolerance and
    5456                    self.error_string == other.error_string)
    5557
    … …  
    6062
    6163    def __repr__(self):
    62         return 'ImageDiffResult(Passed {} {} {} {})'.format(self.passed, self.diff_image, self.diff_percent, self.error_string)
     64        return 'ImageDiffResult(Passed {} {} diff {} tolerance {} {})'.format(self.passed, self.diff_image, self.diff_percent, self.tolerance, self.error_string)
    6365
    6466class ImageDiffer(object):
    … …  
    134136            diff_percent = float(string_utils.decode(m.group(1), target_type=str))
    135137
    136         return ImageDiffResult(passed=False, diff_image=output_image, difference=diff_percent, error_string=err_str or None)
     138        return ImageDiffResult(passed=False, diff_image=output_image, difference=diff_percent, tolerance=self._tolerance, error_string=err_str or None)
    137139
    138140    def stop(self):

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

    r284845 r284866  
    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))
     300        self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
    301301        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0.1"])
    302         self.assertEqual(port.diff_image(b'foo', b'bar', None), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
     302
     303        self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=None), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
    303304        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0.1"])
    304         self.assertEqual(port.diff_image(b'foo', b'bar', 0), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
     305
     306        self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=0), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
    305307        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0"])
    306308
    … …  
    309311        self.assertTrue(port._should_use_jhbuild())
    310312
    311         self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
     313        self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
    312314        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0.1"])
    313         self.assertEqual(port.diff_image(b'foo', b'bar', None), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
     315
     316        self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=None), ImageDiffResult(passed=False, diff_image=b'', difference=100.0, tolerance=0.1))
    314317        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0.1"])
    315         self.assertEqual(port.diff_image(b'foo', b'bar', 0), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
     318
     319        self.assertEqual(port.diff_image(b'foo', b'bar', tolerance=0), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
    316320        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0"])
    317321
    … …  
    324328        port._server_process_constructor = lambda port, nm, cmd, env, crash_message=None: MockServerProcess(lines=['diff: 0% passed\n'])
    325329        image_differ = ImageDiffer(port)
    326         self.assertEqual(image_differ.diff_image(b'foo', b'bar', 0.1), ImageDiffResult(passed=True, diff_image=None, difference=0))
     330        self.assertEqual(image_differ.diff_image(b'foo', b'bar', tolerance=0.1), ImageDiffResult(passed=True, diff_image=None, difference=0))
    327331
    328332    def test_diff_image_failed(self):
    … …  
    330334        port._server_process_constructor = lambda port, nm, cmd, env, crash_message=None: MockServerProcess(lines=['diff: 100% failed\n'])
    331335        image_differ = ImageDiffer(port)
    332         self.assertEqual(image_differ.diff_image(b'foo', b'bar', 0.1), ImageDiffResult(passed=False, diff_image=b'', difference=100.0))
     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))
    333337
    334338    def test_diff_image_crashed(self):
    … …  
    346350        port._server_process_constructor = make_proc
    347351        port.setup_test_run()
    348         self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(passed=False, diff_image=b'', difference=0, error_string='ImageDiff crashed\n'))
     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'))
    349353        port.clean_up_test_run()
    350354

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

    r284845 r284866  
    409409        diffed = actual_contents != expected_contents
    410410        if not actual_contents and not expected_contents:
    411             return ImageDiffResult(passed=True, diff_image=None, difference=0)
     411            return ImageDiffResult(passed=True, diff_image=None, difference=0, tolerance=tolerance or 0)
    412412
    413413        if not actual_contents or not expected_contents:
    414             return ImageDiffResult(passed=False, diff_image=b'', difference=0)
     414            return ImageDiffResult(passed=False, diff_image=b'', difference=0, tolerance=tolerance or 0)
    415415
    416416        if b'ref' in expected_contents:
    … …  
    423423                    string_utils.decode(actual_contents, target_type=str),
    424424                ),
    425                 difference=1)
    426 
    427         return ImageDiffResult(passed=True, diff_image=None, difference=0)
     425                difference=1,
     426                tolerance=tolerance or 0)
     427
     428        return ImageDiffResult(passed=True, diff_image=None, difference=0, tolerance=tolerance or 0)
    428429
    429430    def layout_tests_dir(self):
Note: See TracChangeset for help on using the changeset viewer.