Repository navigation
Fix filter_rgb32_range() with several filters, and make the ImageFilter tests real tests - #1682
Merged
Merged
Conversation
…mages The old ImageFilter tests were debug code: they were not registered, ran 3-second timing loops, and only ran the implementation the CPU dispatch picks. Now each range filter and filter_by_mask() implementation that the build has and the CPU supports gets a test on 4 CommandLineTests images, checked against a scalar reference. The two Euclidean tests are removed. Kernels::RGB32Euclidean covers them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The overload that runs several range filters in one call passed the filter's pixels_in_range, which is not set yet, as replace_color_within_range. It should pass the filter's own flag, invert. This broke in 5bb2f45. The PokemonLA MountDetector uses this overload. Also set pixels_in_range to 0 in both multi-filter structs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add a test that runs filter_rgb32_range() and to_blackwhite_rgb32_range() with 8 filters in one call, and checks each output against the scalar reference. It fails without the fix in the previous commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix a bug in
filter_rgb32_range()with several filters, and make the ImageFilter kernel tests real tests.Split from #1669, as Mysticial asked. The other part is #1681 (
PackedBinaryMatrix::submatrix()).The bug: the overload that runs several range filters in one call passed a field that is not set yet as the replace flag, so the result was undefined. It now passes each filter's own flag. The PokemonLA
MountDetectoruses this overload. This broke in 5bb2f45.The tests now check each range filter and
filter_by_mask()implementation that the CPU supports against a scalar reference, on real images. A new test checksfilter_rgb32_range()andto_blackwhite_rgb32_range()with several filters in one call. Before, the tests were debug code. They were not registered, so CI did not run them. They tested only the implementation that the dispatch picks, and had 3-second timing loops.Other changes:
Kernels::RGB32Euclidean(RGB32 Euclidean filters: count the max distance as in range on x86 SIMD #1637) already tests them.The images are from the CommandLineTests repo. No width or height is a multiple of 8, so the tests also check the leftover pixels after the SIMD loops and the edge tiles of the mask. #1554 also changes this test file, so it will need a small merge fix.
0 test files added, 1 test file changed:
Test results: added when the CI of this PR finishes.
🤖 Generated with Claude Code