Update pixel/corpus tests to handle expected results for Skia. With pdf_use_skia enabled, the rendering results can be correct but slightly different compared to AGG's results. This CL allows run_pixel_tests.py and run_corpus_tests.py to also compared to acceptable rendering results generated by Skia, named in the format of "NAME_expected_skia(_(win|mac|linux))?.pdf.#.png". So that if the rendering result of a PDF is not completely the same as its AGG's expectation, as long as it finds a match with a Skia expected rendering result, it won't cause a failure in pixel/corpus test. This CL also provides a PNG file of Skia-based expected result of pixel test bug_842.in to test the script change. This pixel test contains a gradient transition that Skia uses a different algorithm to render compared to AGG. In case that SkiaPaths, Skia and AGG all generate correct results and these results are different from each other, we will need to add expected results for SkiaPaths in a new format of "NAME_expected_skiapaths(_(win|mac|linux))?.pdf.#.png", besides the expected results of Skia and AGG. Currently we don't have an existing test case that can be rendered differently and correctly among using all three rendering methods. Therefore there is no PNG file uploaded for testing this SkiaPaths specific change. Bug: pdfium:1501 Change-Id: I237c70bdec2640995f9cb8602d0b32addc0598e0 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/67871 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Hui Yingst <nigi@chromium.org>
diff --git a/testing/resources/pixel/bug_842_expected_skia.pdf.0.png b/testing/resources/pixel/bug_842_expected_skia.pdf.0.png new file mode 100644 index 0000000..b454aa5 --- /dev/null +++ b/testing/resources/pixel/bug_842_expected_skia.pdf.0.png Binary files differ
diff --git a/testing/tools/pngdiffer.py b/testing/tools/pngdiffer.py index 85179f8..63315c9 100755 --- a/testing/tools/pngdiffer.py +++ b/testing/tools/pngdiffer.py
@@ -13,6 +13,31 @@ import common +class PathMode: + """PathMode indicates the available expected results' paths. + + Attributes: + DEFAULT: Used for default expected paths in the format of + 'NAME_expected(_OSNAME)?.pdf.#.png'. For a test, this path always + exists. + SKIA: Used when Skia or SkiaPaths is enabled, for paths in the format + of 'NAME_expected_skia(_OSNAME)?.pdf.#.png'. + Such paths only exist when the expected results of Skia or + SkiaPaths are different from those of AGG. + SKIAPATHS: Used when SkiaPaths is enabled, for path in the format of + 'NAME_expected_skiapaths(_OSNAME)?.pdf.#.png'. + Such paths only exist when the expected results from using AGG, + Skia and SkiaPaths are all different from each other. + + Always check PathMode in an incrementing order as the modes are listed in + order of its matching paths' precedence. + """ + + DEFAULT = 0 + SKIA = 1 + SKIAPATHS = 2 + + class NotFoundError(Exception): """Raised when file doesn't exist""" pass @@ -20,10 +45,16 @@ class PNGDiffer(): - def __init__(self, finder, reverse_byte_order): + def __init__(self, finder, features, reverse_byte_order): self.pdfium_diff_path = finder.ExecutablePath('pdfium_diff') self.os_name = finder.os_name self.reverse_byte_order = reverse_byte_order + if 'SKIAPATHS' in features: + self.max_path_mode = PathMode.SKIAPATHS + elif 'SKIA' in features: + self.max_path_mode = PathMode.SKIA + else: + self.max_path_mode = PathMode.DEFAULT def CheckMissingTools(self, regenerate_expected): if (regenerate_expected and self.os_name == 'linux' and @@ -33,19 +64,16 @@ def GetActualFiles(self, input_filename, source_dir, working_dir): actual_paths = [] - path_templates = PathTemplates(input_filename, source_dir, working_dir) + path_templates = PathTemplates(input_filename, source_dir, working_dir, + self.os_name, self.max_path_mode) for page in itertools.count(): actual_path = path_templates.GetActualPath(page) - expected_path = path_templates.GetExpectedPath(page) - platform_expected_path = path_templates.GetPlatformExpectedPath( - self.os_name, page) - if os.path.exists(platform_expected_path): - expected_path = platform_expected_path - elif not os.path.exists(expected_path): + expected_paths = path_templates.GetExpectedPaths(page) + if any(itertools.imap(os.path.exists, expected_paths)): + actual_paths.append(actual_path) + else: break - actual_paths.append(actual_path) - return actual_paths def _RunImageDiffCommand(self, expected_path, actual_path): @@ -59,17 +87,12 @@ return common.RunCommand(cmd) def HasDifferences(self, input_filename, source_dir, working_dir): - path_templates = PathTemplates(input_filename, source_dir, working_dir) - + path_templates = PathTemplates(input_filename, source_dir, working_dir, + self.os_name, self.max_path_mode) for page in itertools.count(): actual_path = path_templates.GetActualPath(page) - expected_path = path_templates.GetExpectedPath(page) - # PDFium tests should be platform independent. Platform based results are - # used to capture platform dependent implementations. - platform_expected_path = path_templates.GetPlatformExpectedPath( - self.os_name, page) - if (not os.path.exists(expected_path) and - not os.path.exists(platform_expected_path)): + expected_paths = path_templates.GetExpectedPaths(page) + if not any(itertools.imap(os.path.exists, expected_paths)): if page == 0: print "WARNING: no expected results files for " + input_filename if os.path.exists(actual_path): @@ -80,21 +103,28 @@ print "Checking " + actual_path sys.stdout.flush() - error = self._RunImageDiffCommand(expected_path, actual_path) + error = None + for path in expected_paths: + new_error = self._RunImageDiffCommand(path, actual_path) + # Update error code. No need to overwrite the previous error code if + # |path| doesn't exist. + if not isinstance(new_error, NotFoundError): + error = new_error + # Found a match and proceed to next page + if not error: + break + if error: - # When failed, we check against platform based results. - platform_error = self._RunImageDiffCommand(platform_expected_path, - actual_path) - if platform_error: - if not isinstance(platform_error, NotFoundError): - error = platform_error - print "FAILURE: " + input_filename + "; " + str(error) - return True + print "FAILURE: " + input_filename + "; " + str(error) + return True return False + # TODO(crbug.com/pdfium/1508): Add support to automatically generate + # Skia/SkiaPaths specific expected results. def Regenerate(self, input_filename, source_dir, working_dir, platform_only): - path_templates = PathTemplates(input_filename, source_dir, working_dir) + path_templates = PathTemplates(input_filename, source_dir, working_dir, + self.os_name, self.max_path_mode) for page in itertools.count(): # Loop through the generated page images. Stop when there is a page @@ -103,8 +133,8 @@ if not os.path.isfile(actual_path): break - platform_expected_path = path_templates.GetPlatformExpectedPath( - self.os_name, page) + platform_expected_path = path_templates.GetExpectedPathByPathMode( + page, PathMode.DEFAULT, self.os_name) # If there is a platform expected png, we will overwrite it. Otherwise, # overwrite the generic png in "all" mode, or do nothing in "platform" @@ -112,7 +142,8 @@ if os.path.exists(platform_expected_path): expected_path = platform_expected_path elif not platform_only: - expected_path = path_templates.GetExpectedPath(page) + expected_path = path_templates.GetExpectedPathByPathMode( + page, PathMode.DEFAULT) else: continue @@ -121,26 +152,51 @@ ACTUAL_TEMPLATE = '.pdf.%d.png' -EXPECTED_TEMPLATE = '_expected' + ACTUAL_TEMPLATE -PLATFORM_EXPECTED_TEMPLATE = '_expected_%s' + ACTUAL_TEMPLATE - class PathTemplates(object): - def __init__(self, input_filename, source_dir, working_dir): + def __init__(self, input_filename, source_dir, working_dir, os_name, + max_path_mode): + assert PathMode.DEFAULT <= max_path_mode <= PathMode.SKIAPATHS, ( + 'Unexpected Maximum PathMode: %d.' % max_path_mode) + input_root, _ = os.path.splitext(input_filename) self.actual_path_template = os.path.join(working_dir, input_root + ACTUAL_TEMPLATE) - self.expected_path = os.path.join(source_dir, - input_root + EXPECTED_TEMPLATE) - self.platform_expected_path = os.path.join( - source_dir, input_root + PLATFORM_EXPECTED_TEMPLATE) + self.source_dir = source_dir + self.input_root = input_root + self.max_path_mode = max_path_mode + self.os_name = os_name + + # Pre-create the available templates depending on |max_path_mode|. + self.expected_templates = [] + for mode in range(PathMode.DEFAULT, max_path_mode + 1): + self.expected_templates.extend([ + self._GetExpectedTemplateByPathMode(mode), + self._GetExpectedTemplateByPathMode(mode, os_name), + ]) def GetActualPath(self, page): return self.actual_path_template % page - def GetExpectedPath(self, page): - return self.expected_path % page + def _GetExpectedTemplateByPathMode(self, mode, os_name=None): + expected_str = '_expected' + if mode == PathMode.DEFAULT: + pass + elif mode == PathMode.SKIA: + expected_str += '_skia' + elif mode == PathMode.SKIAPATHS: + expected_str += '_skiapaths' + else: + assert False, 'Unexpected PathMode: %d.' % mode - def GetPlatformExpectedPath(self, platform, page): - return self.platform_expected_path % (platform, page) + if os_name: + expected_str += '_' + self.os_name + return os.path.join(self.source_dir, + self.input_root + expected_str + ACTUAL_TEMPLATE) + + def GetExpectedPathByPathMode(self, page, mode, os_name=None): + return self._GetExpectedTemplateByPathMode(mode, os_name) % page + + def GetExpectedPaths(self, page): + return [template % page for template in self.expected_templates]
diff --git a/testing/tools/test_runner.py b/testing/tools/test_runner.py index 363faf3..f4b45ac 100644 --- a/testing/tools/test_runner.py +++ b/testing/tools/test_runner.py
@@ -119,6 +119,8 @@ DeleteFiles(actual_images) return True, results + # TODO(crbug.com/pdfium/1508): Add support for an option to automatically + # generate Skia/SkiaPaths specific expected results. def RegenerateIfNeeded_(self, input_filename, source_dir): if (not self.options.regenerate_expected or self.test_suppressor.IsResultSuppressed(input_filename) or @@ -360,7 +362,7 @@ self.test_suppressor = suppressor.Suppressor( finder, self.features, self.options.disable_javascript, self.options.disable_xfa) - self.image_differ = pngdiffer.PNGDiffer(finder, + self.image_differ = pngdiffer.PNGDiffer(finder, self.features, self.options.reverse_byte_order) error_message = self.image_differ.CheckMissingTools( self.options.regenerate_expected)
