Feature: methods for getting a pixel using signed coordinates #1851
Conversation
- add `GenericImageView::checked_get_pixel` method with `i64` arguments returning `Option` - add `GenericImageView::saturating_get_pixel` method with `i64` arguments returning nearest pixel in case location is not in bounds
|
This could be simplified using the max() and min() methods. For instance, Also worth double checking that zero-sized images are either prohibited by GenericImage or properly handled by this patch |
src/image.rs
Outdated
| fn saturating_get_pixel(&self, x: i64, y: i64) -> Self::Pixel { | ||
| #[inline(always)] | ||
| fn convert(value: i64, bound: u32) -> u32 { | ||
| value.is_positive() | ||
| .then(|| { | ||
| match u32::try_from(value) { | ||
| Ok(value) => { | ||
| if value < bound { | ||
| value | ||
| } else { | ||
| bound - 1 | ||
| } | ||
| }, | ||
| Err(_) => bound, | ||
| } | ||
| }).unwrap_or(0u32) | ||
| } | ||
|
|
||
| unsafe { | ||
| self.unsafe_get_pixel( | ||
| convert(x, self.width()), | ||
| convert(y, self.height()) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Also worth double checking that zero-sized images are either prohibited by GenericImage or properly handled by this patch
They are not. Panic in those cases would be permissible behavior but maybe it's not desirable. The methodical evaluation of how we wrote the trait is, however:
- The coordinates must be in_bounds of the image.
Note that in_bounds is both safe, and the trait itself is safe. Thus, we can't make any assumption about its implementation other than the method signature. Since we haven't called the method here either, we can not argue to have fulfilled the precondition properly. I don't think this method possible in a generic, sound manner either way.
However, we can add specialized implementations of these two new methods for ImageBuffer. Since the in_bounds method is local, we can use its implementation details and rely on them being correct.
There was a problem hiding this comment.
This is similar to get_pixel_unchecked. Note that the generic implementation provided by the trait does not in fact call any unsafe method, it defaults to a purely safe fallback. This is correct and intended; since there's nothing unsafe about the trait, the implementor has no obligations to 'do the right thing'. There's no inherent property of in_bounds that the generic implementation could rely on for any effect; so calling the safe fallback is indeed the best useful implementation we can provide.
Note to self: ImageBuffer is still dubious if the user chooses a non-default container. Damn.
|
When designing image types (e.g. https://github.com/strasdat/Sophus/blob/23.04-beta/cpp/sophus/image/image_view.h#L125), I came to the same conclusion that signed integer pixel coordinates are beneficial. For instance, it is quite useful/common to ask whether a 3d point almost projects into a camera's field of view - hence talking about a pixel coordinate of e.g. (-5, -10) can indeed by meaningful. |
Make it consistent with `ImageBuffer` and `SubImageInner`
This PR adds two methods, each with default implementation to
GenericImageViewtrait:checked_get_pixelandsaturating_get_pixel. Both allow to get a pixel from an image view using signed coordinates. That can be convenient for different computer vision algorithms like object detection or tracking.I license past and future contributions under the dual MIT/Apache-2.0 license, allowing licensees to choose either at their option.