Overhaul GD test helpers and affected tests #17309
Open
Conversation
This file contains 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
libgd stores the pixel as an array of rows, so we should use row-major- order traversal to improve caching.
The assertion regarding the image dimensions won't break any tests, and we had it already as a comment. However, asserting that the images are truecolor images is important for `calc_image_dissimilarity()` which otherwise would calculate nonsense, and not unreasonable for `test_image_equals_image()` which otherwise is overspecified (for our purposes, it doesn't matter which palette entry a pixel refers to, but rather whether the actual colors referred by a palette color match). Since the truecolor assertions break two tests, we fix these by converting to truecolor. That should likely be backported to lower branches.
Conversion to truecolor is a relatively expensive operation, and as such should not be implicit; instead test authors are encouraged to use truecolor images in the first place where possible, or to even find better ways to verify expectations than doing a full image comparison.
There is no particular reason to have a separate file for similarity comparisons.
`calc_image_dissimilarity()` calculates the sum of the euclidean distance of the RGB channels of all pixels. The euclidean distance is either zero or greater than or equal to one (but never in ]0, 1[). The sum of these values also has this property, so it doesn't make sense to check for less than 1e-5. Thus we just call `test_image_equals_file()` instead.
`calc_image_dissimilarity()` has the drawback that it did sum up the pixel differences, so for large images the result could be way larger than for small images. It also has the drawback that it likely is not as well understood as the mean squared error. Thus we replace it with the latter, and calculate the mean squared error of the individual RGB channels (to be precise). The result is always in range 0..255**2 what makes reasoning about thresholds easier.
Girgias
approved these changes
Jan 2, 2025
cmb69
commented
Jan 2, 2025
cmb69
commented
Jan 2, 2025
Comment on lines
+170
to
+172
| $e += ((($c1 >> 16) & 255) - (($c2 >> 16) & 255)) ** 2 | ||
| + ((($c1 >> 8) & 255) - (($c2 >> 8) & 255)) ** 2 | ||
| + ((($c1 >> 0) & 255) - (($c2 >> 0) & 255)) ** 2; |
There was a problem hiding this comment.
Just stumbled upon libgd/libgd#751, where the differences are divided by the maximum value, thus yielding values in range [0, 1] (instead of [0, 255]), so the squares would stay in range [0, 1] (instead of [0, 65025]), as would the final result of mse(). That might be even better (at least wrt possibly adding alpha support later).
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.
See individual commit messages for details. TL;DR: this cleans up/modernizes the test helpers a bit (including some minor speed-ups), fixes/improves/simplifies a couple of related test cases, and maybe most importantly replaces the somewhat obsure
calc_image_dissimilarity()with the well-know mean squared error.