Expose YUV Comparison and Utilities#13
Merged
Merged
Conversation
The internal_yuv_hybrid_compare function was doing the image size check which doubles the checks over internal_yuv_hybrid_compare's. move this to yuv_hybrid_compare and check all 3 channels to avoid double checking when using rgb images and check all channels with yuv_hybrid_compare
ChrisRega
requested changes
Aug 12, 2025
Owner
ChrisRega
left a comment
ChrisRega
left a comment
There was a problem hiding this comment.
Thank you for the suggestion, it looks really good and helpful to me, especially since it was spawned by a user request. Please add feature-toggle guarded unit tests to pin the API behavior as we dont ever want to break public API without knowing.
added 2 commits
August 12, 2025 20:21
avoids using mixed syntax for lifetimes, per clippy's #[warn(mismatched_lifetime_syntaxes)]
ChrisRega
reviewed
Aug 16, 2025
| Scenario Outline: Compare newly split yuv channels to expected values | ||
| Given the image '<full_image>' is loaded | ||
| When comparing yuv channels | ||
| Then check split success |
Owner
There was a problem hiding this comment.
Suggested change
| Then check split success | |
| Then the split channels match the references |
then steps should be formulated in passive, but this is just a hint - I can fix this later :)
Author
There was a problem hiding this comment.
i will keep that in mind for future use- was the first time i'd worked with that test runner
ChrisRega
approved these changes
Aug 16, 2025
Owner
|
Will merge tomorrow and draft release, thank you for the contribution :) |
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
2 participants
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.
To solve #12 in a fairly user friendly way, exposes the
Decomposetrait, rgb/yuv conversion, and ayuv_hybrid_comparemethod. As well as fixing some visibility issues that arose from exposing theutilsmodule.These changes are hidden behind the "yuv_compare" feature flag since it will, in general, probably not be needed by the majority of users. But in more performance critical areas it can yield a good speed boost such as in my more recent project which benefited greatly from swapping to yuv to avoid a significant amount of data copying and format conversion. In other cases where one can use yuv rather than rgb, a similar potential exists. The speed boost is not as great as one may see with gpu-based comparison however it is enough to justify the possibility, saving 5-10% of runtime for me in some cases.