From d6caa9dc0b03eed0eb4d0e4780aef5bfa13d376f Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 10 Feb 2026 14:42:30 -0800 Subject: [PATCH] diffimg: binary tool for comparing images (#9668) This tool uses existing libraries: image, imageio, imageio-lite, imagediff to perform difference comparison for on-disk images. We refactor renderdiff to use this tool instead of using python dependencies. Co-authored-by: Ben Doherty --- .github/workflows/postsubmit-main.yml | 4 +- .github/workflows/presubmit.yml | 8 +- CMakeLists.txt | 1 + test/renderdiff/generate.sh | 2 +- test/renderdiff/local_test.sh | 2 + test/renderdiff/src/compare.py | 145 +++++------- test/renderdiff/src/image_diff.py | 206 ----------------- test/renderdiff/src/render.py | 1 - .../renderdiff/src/rendering_requirements.txt | 2 - test/renderdiff/src/test_config.py | 76 +++---- test/renderdiff/src/update_golden.py | 74 +++++- test/renderdiff/tests/presubmit.json | 4 +- tools/diffimg/CMakeLists.txt | 30 +++ tools/diffimg/README.md | 54 +++++ tools/diffimg/main.cpp | 212 ++++++++++++++++++ 15 files changed, 469 insertions(+), 352 deletions(-) delete mode 100644 test/renderdiff/src/image_diff.py create mode 100644 tools/diffimg/CMakeLists.txt create mode 100644 tools/diffimg/README.md create mode 100644 tools/diffimg/main.cpp diff --git a/.github/workflows/postsubmit-main.yml b/.github/workflows/postsubmit-main.yml index 809c56b0cd..f84f4acc57 100644 --- a/.github/workflows/postsubmit-main.yml +++ b/.github/workflows/postsubmit-main.yml @@ -16,8 +16,8 @@ jobs: - uses: ./.github/actions/linux-prereq - id: get_commit_msg uses: ./.github/actions/get-commit-msg - - name: Prerequisites - run: pip install tifffile numpy + - name: Build diffimg + run: ./build.sh release diffimg - name: Run update script env: GH_TOKEN: ${{ secrets.FILAMENTBOT_TOKEN }} diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml index 934ab66dd2..98f659469a 100644 --- a/.github/workflows/presubmit.yml +++ b/.github/workflows/presubmit.yml @@ -125,7 +125,6 @@ jobs: uses: ./.github/actions/get-commit-msg - name: Prerequisites run: | - pip install tifffile numpy # Must have at least clang-16 for a webgpu/dawn build. sudo xcode-select -s /Applications/Xcode_16.2.app/Contents/Developer shell: bash @@ -139,6 +138,9 @@ jobs: set -eux GOLDEN_BRANCH=$(echo "${COMMIT_MESSAGE}" | python3 ${TEST_DIR}/src/commit_msg.py) bash ${TEST_DIR}/generate.sh + # Build diffimg tool + ./build.sh release diffimg + python3 ${TEST_DIR}/src/golden_manager.py \ --branch=${GOLDEN_BRANCH} \ --output=${GOLDEN_OUTPUT_DIR} @@ -149,7 +151,9 @@ jobs: python3 ${TEST_DIR}/src/compare.py \ --src=${GOLDEN_OUTPUT_DIR} \ --dest=${RENDER_OUTPUT_DIR} \ - --out=${DIFF_OUTPUT_DIR} 2>&1 | tee compare_output.txt + --out=${DIFF_OUTPUT_DIR} \ + --diffimg="$(pwd)/out/cmake-release/tools/diffimg/diffimg" \ + --test="${TEST_DIR}/tests/presubmit.json" 2>&1 | tee compare_output.txt if grep "Failed" compare_output.txt > /dev/null; then DELIMITER="EOF_FILE_CONTENT_$(date +%s)" # Using timestamp to make it more unique diff --git a/CMakeLists.txt b/CMakeLists.txt index b33bbd73a2..0a7c3cd64e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -955,6 +955,7 @@ if (IS_HOST_PLATFORM) add_subdirectory(${TOOLS}/cmgen) add_subdirectory(${TOOLS}/cso-lut) + add_subdirectory(${TOOLS}/diffimg) add_subdirectory(${TOOLS}/filamesh) add_subdirectory(${TOOLS}/glslminifier) add_subdirectory(${TOOLS}/matc) diff --git a/test/renderdiff/generate.sh b/test/renderdiff/generate.sh index dfe6ce9398..2bfe78169d 100755 --- a/test/renderdiff/generate.sh +++ b/test/renderdiff/generate.sh @@ -31,7 +31,7 @@ function start_render_() { python3 -m venv ${VENV_DIR} source ${VENV_DIR}/bin/activate - NEEDED_PYTHON_DEPS=("numpy" "tifffile") + NEEDED_PYTHON_DEPS=() for cmd in "${NEEDED_PYTHON_DEPS[@]}"; do if ! python3 -m pip show -q "${cmd}"; then python3 -m pip install ${cmd} diff --git a/test/renderdiff/local_test.sh b/test/renderdiff/local_test.sh index 2d6fb6afd6..1f7e5a8d75 100755 --- a/test/renderdiff/local_test.sh +++ b/test/renderdiff/local_test.sh @@ -26,6 +26,7 @@ else fi bash `dirname $0`/generate.sh "$@" && \ + ./build.sh release diffimg && \ python3 ${RENDERDIFF_TEST_DIR}/src/golden_manager.py \ --branch=${GOLDEN_BRANCH} \ --output=${GOLDEN_OUTPUT_DIR} && \ @@ -33,6 +34,7 @@ bash `dirname $0`/generate.sh "$@" && \ --src=${GOLDEN_OUTPUT_DIR} \ --dest=${RENDER_OUTPUT_DIR} \ --out=${DIFF_OUTPUT_DIR} \ + --diffimg="$(pwd)/out/cmake-release/tools/diffimg/diffimg" \ --test="${RENDERDIFF_TEST_DIR}/tests/presubmit.json" "$@" # $@ Pass arguments to generate.sh, e.g. --test_filter diff --git a/test/renderdiff/src/compare.py b/test/renderdiff/src/compare.py index 6b0ca2461c..81fb735661 100644 --- a/test/renderdiff/src/compare.py +++ b/test/renderdiff/src/compare.py @@ -4,9 +4,10 @@ import sys import pprint import json import fnmatch +import subprocess +import tempfile from utils import execute, ArgParseImpl, important_print, mkdir_p -from image_diff import same_image, output_image_diff from results import RESULT_OK, RESULT_FAILED, RESULT_MISSING, GOLDEN_MISSING import test_config @@ -24,65 +25,43 @@ def _get_tolerance_for_test_case(test_case_name, test_config_obj): return None -def _format_tolerance_summary(stats): - """ - Create human-readable summary of tolerance statistics. +def _run_diffimg(diffimg_path, ref_path, cand_path, tolerance=None, diff_out_path=None): + cmd = [diffimg_path, ref_path, cand_path] - Args: - stats: Statistics dictionary from tolerance evaluation + config_file = None + if tolerance: + fd, config_file = tempfile.mkstemp(suffix='.json', text=True) + with os.fdopen(fd, 'w') as f: + json.dump(tolerance, f) + cmd.extend(['--config', config_file]) - Returns: - str: Formatted summary string - """ - if 'error' in stats: - return f"Error: {stats['error']}" + if diff_out_path: + cmd.extend(['--diff', diff_out_path]) - if 'operator' in stats: - # Nested criteria with operator - operator = stats['operator'] - criteria_count = len(stats['criteria_results']) - passed_count = sum(1 for c in stats['criteria_results'] if c.get('passed', False)) - summary = f"{operator} of {criteria_count} criteria: {passed_count} passed, {criteria_count - passed_count} failed" + try: + result_proc = subprocess.run(cmd, capture_output=True, text=True) + # diffimg outputs JSON to stdout even on failure (exit code might be non-zero for mismatch) + # However, if it crashed or failed to run, stdout might be empty or not JSON. - # Add details for each criteria - details = [] - for i, criteria_stats in enumerate(stats['criteria_results']): - details.append(f" Criteria {i+1}: {_format_tolerance_summary(criteria_stats)}") + output = result_proc.stdout.strip() + if not output: + return False, {'error': 'No output from diffimg', 'stderr': result_proc.stderr} - return summary + "\n" + "\n".join(details) - else: - # Single criteria - total_pixels = stats.get('total_pixels', 0) - failing_pixels = stats.get('failing_pixels', 0) - failing_percentage = stats.get('failing_percentage', 0.0) - allowed_percentage = stats.get('allowed_percentage', 0.0) - max_abs_diff = stats.get('max_abs_diff', 0) - mean_abs_diff = stats.get('mean_abs_diff', 0) - max_diff_per_channel = stats.get('max_diff_per_channel', []) + try: + result_json = json.loads(output) + passed = result_json.get('passed', False) + return passed, result_json + except json.JSONDecodeError: + return False, {'error': 'Invalid JSON output from diffimg', 'stdout': output, 'stderr': result_proc.stderr} - criteria = stats.get('criteria', {}) - criteria_desc = [] - if 'max_pixel_diff' in criteria: - criteria_desc.append(f"max_pixel_diff: {criteria['max_pixel_diff']}") - if 'max_pixel_diff_percent' in criteria: - criteria_desc.append(f"max_pixel_diff_percent: {criteria['max_pixel_diff_percent']}%") - if 'allowed_diff_pixels' in criteria: - criteria_desc.append(f"allowed_diff_pixels: {criteria['allowed_diff_pixels']}%") + except Exception as e: + return False, {'error': f'Failed to run diffimg: {e}'} + finally: + if config_file and os.path.exists(config_file): + os.remove(config_file) - summary_lines = [ - f"Tolerance: {', '.join(criteria_desc)}", - f"Pixels: {failing_pixels:,} / {total_pixels:,} ({failing_percentage:.2f}%) exceed tolerance", - f"Allowed: {allowed_percentage:.2f}% - {'PASS' if stats.get('passed', False) else 'FAIL'}", - f"Max difference: {max_abs_diff} (mean: {mean_abs_diff:.1f})" - ] - if len(max_diff_per_channel) > 1: - channel_info = ", ".join(f"Ch{i}: {diff}" for i, diff in enumerate(max_diff_per_channel)) - summary_lines.append(f"Per-channel max: {channel_info}") - - return "\n".join(summary_lines) - -def _compare_goldens(base_dir, comparison_dir, out_dir=None, test_filter=None, test_config_path=None): +def _compare_goldens(base_dir, comparison_dir, diffimg_path, out_dir=None, test_filter=None, test_config_path=None): def test_name(p): return p.replace('.tif', '') @@ -115,16 +94,19 @@ def _compare_goldens(base_dir, comparison_dir, out_dir=None, test_filter=None, t # Get tolerance configuration for this test case tolerance = _get_tolerance_for_test_case(test_case.replace('.tif', ''), test_config_obj) - # Compare images and get detailed statistics - comparison_result, stats = same_image(src_fname, dest_fname, tolerance) + diff_fname = None + if output_test_dir: + diff_fname = os.path.join(output_test_dir, f"{test_case.replace('.tif', '_diff.tif')}") + # Ensure subdirectories exist for diff output + os.makedirs(os.path.dirname(diff_fname), exist_ok=True) + + # Compare images using diffimg + comparison_result, stats = _run_diffimg(diffimg_path, src_fname, dest_fname, tolerance, diff_fname) if not comparison_result: result['result'] = RESULT_FAILED - if output_test_dir: - # just the file name - diff_fname = f"{test_case.replace('.tif', '_diff.tif')}" - output_image_diff(src_fname, dest_fname, os.path.join(output_test_dir, diff_fname)) - result['diff'] = diff_fname + if diff_fname and os.path.exists(diff_fname): + result['diff'] = os.path.basename(diff_fname) else: result['result'] = RESULT_OK @@ -132,16 +114,11 @@ def _compare_goldens(base_dir, comparison_dir, out_dir=None, test_filter=None, t if tolerance: result['tolerance_used'] = True result['tolerance_config'] = tolerance - if stats: - result['tolerance_stats'] = stats - # Add human-readable summary - result['tolerance_summary'] = _format_tolerance_summary(stats) - elif stats is None and comparison_result: - result['comparison_type'] = 'exact_match' - elif stats and 'error' in stats: - result['error'] = stats['error'] - if 'details' in stats: - result['error_details'] = stats['details'] + + if stats: + result['stats'] = stats + if 'error' in stats: + result['error'] = stats['error'] return result @@ -191,6 +168,7 @@ if __name__ == '__main__': parser.add_argument('--src', help='Directory of the base of the diff.', required=True) parser.add_argument('--dest', help='Directory of the comparison of the diff.') parser.add_argument('--out', help='Directory of output for the result of the diff.') + parser.add_argument('--diffimg', help='Path to the diffimg tool.', required=True) parser.add_argument('--test_filter', help='Filter for the tests to run') parser.add_argument('--test', help='Path to test configuration JSON file for tolerance settings.') @@ -202,42 +180,37 @@ if __name__ == '__main__': dest = os.path.join(os.getcwd(), './out/renderdiff') assert os.path.exists(dest), f"Destination folder={dest} does not exist." - results = _compare_goldens(args.src, dest, out_dir=args.out, + if not os.path.exists(args.diffimg): + print(f"Error: diffimg tool not found at {args.diffimg}") + sys.exit(1) + + results = _compare_goldens(args.src, dest, args.diffimg, out_dir=args.out, test_filter=args.test_filter, test_config_path=args.test) # Categorize results failed = [k for k in results if k['result'] != RESULT_OK] passed = [k for k in results if k['result'] == RESULT_OK] - tolerance_used_count = len([k for k in results if k.get('tolerance_used', False)]) # Create detailed failure report failed_details = [] for k in failed: failure_line = f" {k['name']} ({k['result']})" - if 'tolerance_summary' in k: - failure_line += f"\n {k['tolerance_summary'].replace(chr(10), chr(10) + ' ')}" + if 'stats' in k: + stats = k['stats'] + if 'maxDiffFound' in stats: + failure_line += f"\n Max Diff: {stats['maxDiffFound']}" + if 'failingPixelCount' in stats: + failure_line += f"\n Failing Pixels: {stats['failingPixelCount']}" failed_details.append(failure_line) - # Create success report with tolerance details - tolerance_used_details = [] - for k in passed: - if k.get('tolerance_used', False) and 'tolerance_summary' in k: - tolerance_used_details.append(f" {k['name']}: {k['tolerance_summary'].split(chr(10))[0]}") - # Main summary success_count = len(passed) important_print(f'Successfully compared {success_count} / {len(results)} images') - if tolerance_used_details: - pstr = 'Passed:' - for detail in tolerance_used_details: - pstr += '\n' + detail - important_print(pstr) - if failed_details: pstr = 'Failed:' for detail in failed_details: pstr += '\n' + detail important_print(pstr) if len(failed) > 0: - exit(1) + exit(1) \ No newline at end of file diff --git a/test/renderdiff/src/image_diff.py b/test/renderdiff/src/image_diff.py deleted file mode 100644 index 13f21a3502..0000000000 --- a/test/renderdiff/src/image_diff.py +++ /dev/null @@ -1,206 +0,0 @@ -# Copyright (C) 2025 The Android Open Source Project -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import tifffile -import numpy as np - -def evaluate_tolerance_criteria(img1_data, img2_data, tolerance_spec): - """ - Recursively evaluate tolerance criteria with AND/OR logic. - - Args: - img1_data: First image array - img2_data: Second image array - tolerance_spec: Dictionary containing tolerance criteria with structure: - { - "operator": "AND" | "OR", # How to combine criteria results - "criteria": [...] # List of criteria or nested tolerance specs - } - OR for leaf criteria: - { - "max_pixel_diff": int, # Max absolute difference per channel (0-255) - "max_pixel_diff_percent": float, # Max difference as percentage (0-100%) - "allowed_diff_pixels": float # Percentage of pixels allowed to exceed (0-100%) - } - - Returns: - tuple: (bool: pass/fail, dict: detailed statistics) - """ - - if 'criteria' not in tolerance_spec: - # Leaf criteria - evaluate single condition - return evaluate_single_criteria(img1_data, img2_data, tolerance_spec) - - operator = tolerance_spec.get('operator', 'AND').upper() - criteria_list = tolerance_spec['criteria'] - - results_and_stats = [evaluate_tolerance_criteria(img1_data, img2_data, criteria) - for criteria in criteria_list] - - results = [r[0] for r in results_and_stats] # Extract pass/fail results - all_stats = [r[1] for r in results_and_stats] # Extract statistics - - if operator == 'AND': - final_result = all(results) - elif operator == 'OR': - final_result = any(results) - else: - raise ValueError(f"Unknown operator: {operator}") - - # Combine statistics - combined_stats = { - 'operator': operator, - 'criteria_results': all_stats, - 'passed': bool(final_result) - } - - return final_result, combined_stats - -def evaluate_single_criteria(img1_data, img2_data, criteria): - """ - Evaluate a single tolerance criteria. - - Args: - img1_data: First image array - img2_data: Second image array - criteria: Dictionary with tolerance parameters: - - max_pixel_diff: Maximum absolute difference per channel (0-255 range) - - max_pixel_diff_percent: Maximum difference as percentage (0-100%) - - allowed_diff_pixels: Percentage of pixels allowed to exceed tolerance (0-100%) - - Returns: - tuple: (bool: pass/fail, dict: detailed statistics) - """ - diff_abs = np.abs(img1_data.astype(np.int16) - img2_data.astype(np.int16)) - - max_diff = criteria.get('max_pixel_diff', float('inf')) - max_diff_percent = criteria.get('max_pixel_diff_percent', float('inf')) - allowed_diff_pixels = criteria.get('allowed_diff_pixels', 0.0) - - # Calculate which pixels exceed absolute threshold - exceeds_abs = diff_abs > max_diff if max_diff < float('inf') else np.zeros_like(diff_abs, dtype=bool) - - # Calculate which pixels exceed percentage threshold - if max_diff_percent < float('inf'): - max_val = np.maximum(img1_data, img2_data) - # Avoid division by zero - diff_percent = np.divide(diff_abs * 100.0, np.maximum(max_val, 1), - out=np.zeros_like(diff_abs, dtype=np.float32), - where=max_val!=0) - exceeds_percent = diff_percent > max_diff_percent - else: - exceeds_percent = np.zeros_like(diff_abs, dtype=bool) - - # A pixel fails if it exceeds either threshold (OR logic at pixel level) - exceeds_tolerance = exceeds_abs | exceeds_percent - - # Check per-pixel: pixel fails if ANY channel exceeds tolerance - exceeds_per_pixel = np.any(exceeds_tolerance, axis=-1) if len(exceeds_tolerance.shape) > 2 else exceeds_tolerance - - # Calculate detailed statistics - total_pixels = exceeds_per_pixel.size - failing_pixels = np.sum(exceeds_per_pixel) - failing_percentage = (failing_pixels / total_pixels) * 100.0 - - # Distribution analysis - max_abs_diff = np.max(diff_abs) if diff_abs.size > 0 else 0 - mean_abs_diff = np.mean(diff_abs) if diff_abs.size > 0 else 0 - - # Per-channel max differences - if len(diff_abs.shape) > 2: - max_diff_per_channel = [np.max(diff_abs[:, :, c]) for c in range(diff_abs.shape[2])] - else: - max_diff_per_channel = [max_abs_diff] - - stats = { - 'criteria': criteria, - 'total_pixels': int(total_pixels), - 'failing_pixels': int(failing_pixels), - 'failing_percentage': float(failing_percentage), - 'allowed_percentage': float(allowed_diff_pixels), - 'max_abs_diff': int(max_abs_diff), - 'mean_abs_diff': float(mean_abs_diff), - 'max_diff_per_channel': [int(x) for x in max_diff_per_channel], - 'passed': bool(failing_percentage <= allowed_diff_pixels) - } - - return failing_percentage <= allowed_diff_pixels, stats - -def same_image(tiff_file_a, tiff_file_b, tolerance=None): - """ - Compare two TIFF images for equality with optional tolerance. - - Args: - tiff_file_a: Path to first TIFF file - tiff_file_b: Path to second TIFF file - tolerance: Optional tolerance specification dictionary. If None, performs exact comparison. - Can be either: - 1. Single criteria: {"max_pixel_diff": 5, "allowed_diff_pixels": 1.0} - 2. Nested criteria: {"operator": "OR", "criteria": [...]} - - Returns: - tuple: (bool: pass/fail, dict: detailed statistics or None) - For exact comparison, returns (bool, None) - """ - try: - img1_data = tifffile.imread(tiff_file_a) - img2_data = tifffile.imread(tiff_file_b) - - # If the dimensions (height, width, number of channels, number of pages/frames) - # are different, the images are not the same. - if img1_data.shape != img2_data.shape: - print(f"Images have different shapes: {img1_data.shape} vs {img2_data.shape}") - return False, {'error': 'Shape mismatch', 'shape1': img1_data.shape, 'shape2': img2_data.shape} - - # If no tolerance specified, use exact comparison - if tolerance is None: - exact_match = np.array_equal(img1_data, img2_data) - return exact_match, None - - # Use tolerance-based comparison - return evaluate_tolerance_criteria(img1_data, img2_data, tolerance) - - except FileNotFoundError: - print(f"Error: One or both files not found ('{tiff_file_a}', '{tiff_file_b}').") - return False, {'error': 'File not found'} - except tifffile.TiffFileError as e: - print(f"Error: One or both files are not valid TIFF files or could not be read. Details: {e}") - return False, {'error': 'Invalid TIFF file', 'details': str(e)} - except Exception as e: - print(f"An unexpected error occurred: {e}") - return False, {'error': 'Unexpected error', 'details': str(e)} - -def output_image_diff(tiff_file_a, tiff_file_b, output_path): - try: - img1_data = tifffile.imread(tiff_file_a) - img2_data = tifffile.imread(tiff_file_b) - - # If the dimensions (height, width, number of channels, number of pages/frames) - # are different, the images are not the same. - if img1_data.shape != img2_data.shape: - raise RuntimeError(f"Images {tiff_file_a} and {tiff_file_b} have different shapes: {img1_data.shape} vs {img2_data.shape}") - - diff_img = np.abs(img1_data.astype(np.int16) - img2_data.astype(np.int16)) - diff_img = diff_img.astype(np.uint8) - tifffile.imwrite(output_path, diff_img) - - except FileNotFoundError: - print(f"Error: One or both files not found ('{file_path1}', '{file_path2}').") - return False - except tifffile.TiffFileError as e: - print(f"Error: One or both files are not valid TIFF files or could not be read. Details: {e}") - return False - except Exception as e: - print(f"An unexpected error occurred: {e}") - return False diff --git a/test/renderdiff/src/render.py b/test/renderdiff/src/render.py index 935eea8286..8eb5166f43 100644 --- a/test/renderdiff/src/render.py +++ b/test/renderdiff/src/render.py @@ -24,7 +24,6 @@ import fnmatch from utils import execute, ArgParseImpl, mkdir_p, mv_f, important_print import test_config -from image_diff import same_image from results import RESULT_OK, RESULT_FAILED def _render_single_model(gltf_viewer, test_json_path, named_output_dir, diff --git a/test/renderdiff/src/rendering_requirements.txt b/test/renderdiff/src/rendering_requirements.txt index cde05dbf7a..42880268dc 100644 --- a/test/renderdiff/src/rendering_requirements.txt +++ b/test/renderdiff/src/rendering_requirements.txt @@ -1,6 +1,4 @@ Mako==1.3.10 MarkupSafe==3.0.2 -numpy==2.3.3 PyYAML==6.0.2 setuptools==80.9.0 -tifffile==2025.9.9 diff --git a/test/renderdiff/src/test_config.py b/test/renderdiff/src/test_config.py index 42458c2535..cb7f34bbd9 100644 --- a/test/renderdiff/src/test_config.py +++ b/test/renderdiff/src/test_config.py @@ -23,13 +23,13 @@ from os import path def _is_list_of_strings(field): return isinstance(field, list) and\ - all(isinstance(item, str) for item in field) + all(isinstance(item, str) for item in field) def _is_string(s): - return isinstance(s, str) + return isinstance(s, str) def _is_dict(s): - return isinstance(s, dict) + return isinstance(s, dict) class RenderingConfig(): def __init__(self, data): @@ -74,40 +74,40 @@ class PresetConfig(RenderingConfig): """ Validate tolerance configuration structure. - Tolerance can be: - 1. Single criteria: {"max_pixel_diff": 5, "allowed_diff_pixels": 1.0} - 2. Nested criteria: {"operator": "OR", "criteria": [...]} + Tolerance follows the imagediff/diffimg schema: + { + "mode": "LEAF" | "AND" | "OR", + "maxAbsDiff": float, // For LEAF mode + "maxFailingPixelsFraction": float, // Global check + "children": [...] // For AND/OR mode + } """ - if 'criteria' in tolerance: - # Nested structure with operator - operator = tolerance.get('operator', 'AND') - assert operator.upper() in ['AND', 'OR'], f"Invalid operator: {operator}" + valid_keys = {'mode', 'maxAbsDiff', 'maxFailingPixelsFraction', 'children', 'swizzle', 'channelMask'} + tolerance_keys = set(tolerance.keys()) + invalid_keys = tolerance_keys - valid_keys + assert len(invalid_keys) == 0, f"Invalid tolerance keys: {invalid_keys}" - criteria_list = tolerance['criteria'] - assert isinstance(criteria_list, list), "criteria must be a list" - assert len(criteria_list) > 0, "criteria list cannot be empty" + if 'children' in tolerance: + # Nested structure + mode = tolerance.get('mode', 'AND') + assert mode in ['AND', 'OR'], f"Invalid mode for node with children: {mode}" - # Recursively validate each criteria - for criteria in criteria_list: - self._validate_tolerance(criteria) + children = tolerance['children'] + assert isinstance(children, list), "children must be a list" + assert len(children) > 0, "children list cannot be empty" + + # Recursively validate each child + for child in children: + self._validate_tolerance(child) else: - # Leaf criteria - validate individual parameters - valid_keys = {'max_pixel_diff', 'max_pixel_diff_percent', 'allowed_diff_pixels'} - tolerance_keys = set(tolerance.keys()) - invalid_keys = tolerance_keys - valid_keys - assert len(invalid_keys) == 0, f"Invalid tolerance keys: {invalid_keys}" + # Leaf criteria + if 'maxAbsDiff' in tolerance: + assert isinstance(tolerance['maxAbsDiff'], (int, float)), "maxAbsDiff must be numeric" + assert tolerance['maxAbsDiff'] >= 0, "maxAbsDiff must be non-negative" - if 'max_pixel_diff' in tolerance: - assert isinstance(tolerance['max_pixel_diff'], (int, float)), "max_pixel_diff must be numeric" - assert 0 <= tolerance['max_pixel_diff'] <= 255, "max_pixel_diff must be 0-255" - - if 'max_pixel_diff_percent' in tolerance: - assert isinstance(tolerance['max_pixel_diff_percent'], (int, float)), "max_pixel_diff_percent must be numeric" - assert 0 <= tolerance['max_pixel_diff_percent'] <= 100, "max_pixel_diff_percent must be 0-100%" - - if 'allowed_diff_pixels' in tolerance: - assert isinstance(tolerance['allowed_diff_pixels'], (int, float)), "allowed_diff_pixels must be numeric" - assert 0 <= tolerance['allowed_diff_pixels'] <= 100, "allowed_diff_pixels must be 0-100%" + if 'maxFailingPixelsFraction' in tolerance: + assert isinstance(tolerance['maxFailingPixelsFraction'], (int, float)), "maxFailingPixelsFraction must be numeric" + assert 0 <= tolerance['maxFailingPixelsFraction'] <= 1.0, "maxFailingPixelsFraction must be 0.0-1.0" class TestConfig(RenderingConfig): def __init__(self, data, existing_models, presets, backends): @@ -146,7 +146,7 @@ class TestConfig(RenderingConfig): if models: assert _is_list_of_strings(models) assert all(m in existing_models for m in models) - self.models = set(models + self.models) + self.models = set(list(models) + list(self.models)) # Parse tolerance configuration - test-level tolerance overrides preset tolerance tolerance = data.get('tolerance') @@ -166,8 +166,8 @@ class TestConfig(RenderingConfig): def to_filament_format(self): json_out = { - 'name': self.name, - 'base': self.rendering + 'name': self.name, + 'base': self.rendering } return json.dumps(json_out) @@ -188,9 +188,9 @@ class RenderTestConfig(): assert all(path.isdir(p) for p in model_search_paths) model_paths = list( - chain(*(glob.glob(f'{d}/**/*.glb', recursive=True) for d in model_search_paths))) + \ - list( - chain(*(glob.glob(f'{d}/**/*.gltf', recursive=True) for d in model_search_paths))) + chain(*(glob.glob(f'{d}/**/*.glb', recursive=True) for d in model_search_paths))) + \ + list( + chain(*(glob.glob(f'{d}/**/*.gltf', recursive=True) for d in model_search_paths))) # This flatten the output for glob.glob self.models = {path.splitext(path.basename(model))[0]: model for model in model_paths} diff --git a/test/renderdiff/src/update_golden.py b/test/renderdiff/src/update_golden.py index 8ca35b109b..3ea600930e 100644 --- a/test/renderdiff/src/update_golden.py +++ b/test/renderdiff/src/update_golden.py @@ -16,9 +16,10 @@ import sys import os import glob import time +import subprocess +import json from golden_manager import GoldenManager, ACCESS_TYPE_SSH, ACCESS_TYPE_TOKEN -from image_diff import same_image from utils import execute, ArgParseImpl from utils import prompt_helper, PROMPT_YES, PROMPT_NO @@ -61,21 +62,54 @@ def _do_update(golden_manager, config): deletes=deletes, push_to_remote=push_to_remote) -def _get_deletes_updates(update_dir, golden_dir): +def _same_image_diffimg(diffimg_path, img1, img2): + cmd = [diffimg_path, img1, img2] + try: + result = subprocess.run(cmd, capture_output=True, text=True) + output = result.stdout.strip() + if not output: + return False + + try: + res_json = json.loads(output) + return res_json.get('passed', False) + except json.JSONDecodeError: + return False + except Exception: + return False + +def _get_deletes_updates(update_dir, golden_dir, diffimg_path): ret_delete = [] ret_update = [] - for ext in ['tif', 'json']: - base = set(glob.glob(f'./**/*.{ext}', root_dir=golden_dir, recursive=True)) - new = set(glob.glob(f'./**/*.{ext}', root_dir=update_dir, recursive=True)) + # Scan update_dir for files to potentially update or add + for ext in ['tif', 'json']: + # Get relative paths from golden_dir (base) and update_dir (new) + # Note: glob with root_dir returns relative paths + base_files = set(glob.glob(f'./**/*.{ext}', root_dir=golden_dir, recursive=True)) + new_files = set(glob.glob(f'./**/*.{ext}', root_dir=update_dir, recursive=True)) + + # Files in base but not in new are candidates for deletion (if we decide to prune) + # However, update_golden typically only updates/adds based on the new render set. + # But strict sync might imply deleting missing ones. + # The original logic was: delete = list(base - new). delete = list(base - new) + + # Files in new but not in base are definitely updates (additions) update = list(new - base) - for fpath in base.intersection(new): + # Files in both need comparison + for fpath in base.intersection(new_files): base_fpath = os.path.join(golden_dir, fpath) new_fpath = os.path.join(update_dir, fpath) - if (ext == 'tif' and not same_image(new_fpath, base_fpath)[0]) or \ - (ext == 'json' and _file_as_str(new_fpath) != _file_as_str(base_fpath)): + + is_different = False + if ext == 'tif': + is_different = not _same_image_diffimg(diffimg_path, new_fpath, base_fpath) + elif ext == 'json': + is_different = _file_as_str(new_fpath) != _file_as_str(base_fpath) + + if is_different: update.append(fpath) ret_update += update @@ -84,7 +118,7 @@ def _get_deletes_updates(update_dir, golden_dir): return ret_delete, ret_update # Ask a bunch of questions to gather the configuration for the update -def _interactive_mode(base_golden_dir): +def _interactive_mode(base_golden_dir, diffimg_path): config = {} cur_branch = _get_current_branch() if prompt_helper( @@ -121,7 +155,7 @@ def _interactive_mode(base_golden_dir): config[CONFIG_COMMIT_MSG] = f'Update {time.time()} from filament ({cur_branch})' new_golden_dir = config[CONFIG_NEW_SRC_DIR] - deletes, updates = _get_deletes_updates(new_golden_dir, base_golden_dir) + deletes, updates = _get_deletes_updates(new_golden_dir, base_golden_dir, diffimg_path) if len(deletes) + len(updates) != 0: prompt = 'The following files will be changed:\n' + \ '\n'.join([f' {fname} [delete]' for fname in deletes]) + \ @@ -146,6 +180,8 @@ if __name__ == "__main__": parser.add_argument('--branch', help='Branch of the golden repo to write to') parser.add_argument('--golden-repo-token', help='Access token for the golden repo') parser.add_argument('--push-to-remote', action="store_true", help='Access token for the golden repo') + parser.add_argument('--diffimg', help='Path to the diffimg tool', + default='./out/cmake-release/tools/diffimg/diffimg') # write-to-branch mode parser.add_argument('--source', help='Directory containing the new goldens') @@ -164,10 +200,21 @@ if __name__ == "__main__": ) base_golden_dir = golden_manager.directory() + diffimg_path = args.diffimg + # Try to find diffimg if default path doesn't exist, mainly for local interactive use convenience + if not os.path.exists(diffimg_path): + # fallback check for debug build + debug_path = './out/cmake-debug/tools/diffimg/diffimg' + if os.path.exists(debug_path): + diffimg_path = debug_path + # This is the write-to-branch mode if args.branch and args.source and args.commit_msg: + if not os.path.exists(diffimg_path): + print(f"Error: diffimg tool not found at {diffimg_path}. Please build it first (e.g., ./build.sh release diffimg)") + sys.exit(1) assert os.path.exists(args.source), f'{args.source} (--source) directory not found' - deletes, updates = _get_deletes_updates(args.source, base_golden_dir) + deletes, updates = _get_deletes_updates(args.source, base_golden_dir, diffimg_path) config = { CONFIG_PUSH_TO_REMOTE: args.push_to_remote, CONFIG_GOLDENS_BRANCH: args.branch, @@ -182,5 +229,8 @@ if __name__ == "__main__": golden_manager.merge_to_main(branch=args.branch, tag=args.filament_tag, push_to_remote=True) # Else, we're in interactive mode of write-to-branch (for local execution). else: - config = _interactive_mode(base_golden_dir) + if not os.path.exists(diffimg_path): + print(f"Error: diffimg tool not found at {diffimg_path}. Please build it first (e.g., ./build.sh release diffimg)") + sys.exit(1) + config = _interactive_mode(base_golden_dir, diffimg_path) _do_update(golden_manager, config) diff --git a/test/renderdiff/tests/presubmit.json b/test/renderdiff/tests/presubmit.json index 74dd9716ed..40911b9eed 100644 --- a/test/renderdiff/tests/presubmit.json +++ b/test/renderdiff/tests/presubmit.json @@ -12,8 +12,8 @@ "view.dithering": "NONE" }, "tolerance": { // [optional] Simple tolerance - single criteria - "max_pixel_diff": 5, // Max absolute difference per channel (0-255) - "allowed_diff_pixels": 0.1 // Percentage of pixels allowed to exceed (0-100%) + "maxAbsDiff": 0.0196, // Max absolute difference per channel (5/255) + "maxFailingPixelsFraction": 0.001 // Percentage of pixels allowed to exceed (0.1%) } }, { diff --git a/tools/diffimg/CMakeLists.txt b/tools/diffimg/CMakeLists.txt new file mode 100644 index 0000000000..f5748510e3 --- /dev/null +++ b/tools/diffimg/CMakeLists.txt @@ -0,0 +1,30 @@ +cmake_minimum_required(VERSION 3.19) +project(diffimg) + +set(TARGET diffimg) + +add_executable(${TARGET} main.cpp) + +target_link_libraries(${TARGET} PRIVATE + getopt + imagediff + imageio + imageio-lite + image + utils +) + +set_target_properties(${TARGET} PROPERTIES FOLDER Tools) + +# ================================================================================================= +# Licenses +# ================================================================================================== +set(MODULE_LICENSES getopt) +set(GENERATION_ROOT ${CMAKE_CURRENT_BINARY_DIR}/generated) +list_licenses(${GENERATION_ROOT}/licenses/licenses.inc ${MODULE_LICENSES}) +target_include_directories(${TARGET} PRIVATE ${GENERATION_ROOT}) + +# ================================================================================================== +# Installation +# ================================================================================================== +install(TARGETS ${TARGET} RUNTIME DESTINATION bin) diff --git a/tools/diffimg/README.md b/tools/diffimg/README.md new file mode 100644 index 0000000000..660452ae5d --- /dev/null +++ b/tools/diffimg/README.md @@ -0,0 +1,54 @@ +# diffimg + +`diffimg` is a command-line tool for comparing two images using the `imagediff` library's tolerance logic. It supports various image formats and allows for fine-grained control over comparison thresholds via JSON configuration and optional masking. + +## Usage + +```bash +diffimg [options] +``` + +### Arguments +- ``: Path to the golden/expected image. +- ``: Path to the test/actual image. + +### Options +- `--config `: Path to a JSON configuration file defining comparison thresholds. +- `--mask `: Path to a grayscale mask image (0 = ignore, >0 = compare). +- `--diff `: Path to output a visual difference image (unmasked absolute difference). +- `--help, -h`: Print help message. + +## Output + +The tool outputs a JSON object to `stdout` containing the comparison results: + +```json +{ + "status": 0, + "passed": true, + "failingPixelCount": 0, + "maxDiffFound": [0.0, 0.0, 0.0, 0.0] +} +``` + +### Status Codes +- `0`: PASSED (Images are considered the same within tolerance) +- `1`: SIZE_MISMATCH (Images have different dimensions) +- `2`: PIXEL_DIFFERENCE (Images differ beyond the allowed tolerance) + +## Configuration Format + +The optional JSON configuration follows the `imagediff` schema: + +```json +{ + "mode": "LEAF", + "maxAbsDiff": 0.01, + "maxFailingPixelsFraction": 0.05 +} +``` + +## Dependencies +- `libs/imagediff`: Comparison logic. +- `libs/imageio`: Primary image decoding (PNG, HDR, EXR). +- `libs/imageio-lite`: Fallback decoding (TIFF). diff --git a/tools/diffimg/main.cpp b/tools/diffimg/main.cpp new file mode 100644 index 0000000000..1f8b13e8db --- /dev/null +++ b/tools/diffimg/main.cpp @@ -0,0 +1,212 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +using namespace utils; +using namespace image; + +static void printUsage(const char* name) { + std::cerr << "Usage: " << name << " [options] \n" + << "Options:\n" + << " --config Path to JSON configuration file.\n" + << " --mask Path to mask image.\n" + << " --diff Path to output difference image.\n" + << " --help, -h Print this help message.\n"; +} + +static LinearImage loadImage(const Path& path) { + if (!path.exists()) { + std::cerr << "Error: File not found: " << path << std::endl; + return LinearImage(); + } + + auto decodeWithImageIO = [](std::istream& stream, const std::string& name) { + return image::ImageDecoder::decode(stream, name, image::ImageDecoder::ColorSpace::LINEAR); + }; + + auto decodeWithImageIOLite = [](std::istream& stream, const std::string& name) { + return imageio_lite::ImageDecoder::decode(stream, CString(name.c_str()), + imageio_lite::ImageDecoder::ColorSpace::LINEAR); + }; + + std::string ext = path.getExtension(); + std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower); + + bool preferLite = (ext == "tif" || ext == "tiff"); + + std::ifstream stream(path, std::ios::binary); + if (!stream) { + std::cerr << "Error: Could not open file: " << path << std::endl; + return LinearImage(); + } + + LinearImage img; + if (preferLite) { + img = decodeWithImageIOLite(stream, path.getName()); + if (!img.isValid()) { + stream.clear(); + stream.seekg(0, std::ios::beg); + img = decodeWithImageIO(stream, path.getName()); + } + } else { + img = decodeWithImageIO(stream, path.getName()); + if (!img.isValid()) { + stream.clear(); + stream.seekg(0, std::ios::beg); + img = decodeWithImageIOLite(stream, path.getName()); + } + } + + if (!img.isValid()) { + std::cerr << "Error: Could not decode image: " << path << std::endl; + } + return img; +} + +static std::string readFile(const Path& path) { + std::ifstream t(path); + if (!t) return ""; + t.seekg(0, std::ios::end); + size_t size = t.tellg(); + std::string buffer(size, ' '); + t.seekg(0); + t.read(&buffer[0], size); + return buffer; +} + +static void saveDiffImage(const Path& path, const LinearImage& image) { + std::ofstream stream(path, std::ios::binary); + if (!stream) { + std::cerr << "Error: Could not open output file: " << path << std::endl; + return; + } + + std::string ext = path.getExtension(); + std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower); + + bool success = false; + if (ext == "tif" || ext == "tiff") { + success = imageio_lite::ImageEncoder::encode(stream, + imageio_lite::ImageEncoder::Format::TIFF, + image, "", CString(path.getName().c_str())); + } else { + image::ImageEncoder::Format format = image::ImageEncoder::chooseFormat(path.getName()); + success = image::ImageEncoder::encode(stream, format, image, "", path.getName()); + } + + if (!success) { + std::cerr << "Error: Failed to write difference image to " << path << std::endl; + } +} + +int main(int argc, char** argv) { + static struct option longOptions[] = { + {"config", required_argument, 0, 'c'}, + {"mask", required_argument, 0, 'm'}, + {"diff", required_argument, 0, 'd'}, + {"help", no_argument, 0, 'h'}, + {0, 0, 0, 0} + }; + + Path configPath; + Path maskPath; + Path diffPath; + + int opt; + int optionIndex = 0; + while ((opt = getopt_long(argc, argv, "c:m:d:h", longOptions, &optionIndex)) != -1) { + switch (opt) { + case 'c': + configPath = Path(optarg); + break; + case 'm': + maskPath = Path(optarg); + break; + case 'd': + diffPath = Path(optarg); + break; + case 'h': + printUsage(argv[0]); + return 0; + default: + printUsage(argv[0]); + return 1; + } + } + + if (optind + 2 > argc) { + std::cerr << "Error: Missing arguments." << std::endl; + printUsage(argv[0]); + return 1; + } + + Path refPath(argv[optind]); + Path candPath(argv[optind + 1]); + + LinearImage refImg = loadImage(refPath); + if (!refImg.isValid()) return 1; + + LinearImage candImg = loadImage(candPath); + if (!candImg.isValid()) return 1; + + LinearImage maskImg; + if (!maskPath.isEmpty()) { + maskImg = loadImage(maskPath); + if (!maskImg.isValid()) return 1; + } + + imagediff::ImageDiffConfig config; + if (!configPath.isEmpty()) { + std::string jsonContent = readFile(configPath); + if (jsonContent.empty()) { + std::cerr << "Error: Could not read config file: " << configPath << std::endl; + return 1; + } + if (!imagediff::parseConfig(jsonContent.c_str(), jsonContent.size(), &config)) { + std::cerr << "Error: Failed to parse config file." << std::endl; + return 1; + } + } else { + config.mode = imagediff::ImageDiffConfig::Mode::LEAF; + config.maxAbsDiff = 0.0f; + } + + bool generateDiff = !diffPath.isEmpty(); + imagediff::ImageDiffResult result = imagediff::compare(refImg, candImg, config, + maskImg.isValid() ? &maskImg : nullptr, generateDiff); + + if (generateDiff && result.diffImage.isValid()) { + saveDiffImage(diffPath, result.diffImage); + } + + utils::CString jsonOutput = imagediff::serializeResult(result); + std::cout << jsonOutput.c_str() << std::endl; + + return 0; +}