Skip to content

Commit aac14b8

Browse files
committed
Introduce bounds check pixel get/get_mut
The intention here is to help the optimizer. For methods where access out-of-bounds is not fatal the bounds checking code appears to be repeated unnecessarily in many cases—once for a manual check to avoid the panic and once in the impl in the path that would lead to such panic. This is backwards compatible in that the new methods are default implemented on the traits. In the long term it seems nevertheless advisable to flip this around and default implement the panicking method instead. This would also improve possibilities for indexing and adherence to the API guidelines.
1 parent 98a20f4 commit aac14b8

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

src/buffer.rs

+16
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,14 @@ where
775775
*self.get_pixel(x, y)
776776
}
777777

778+
fn opt_pixel(&self, x: u32, y: u32) -> Option<P> {
779+
if let Some(idx) = self.pixel_indices(x, y) {
780+
Some(*P::from_slice(&self.data[idx]))
781+
} else {
782+
None
783+
}
784+
}
785+
778786
/// Returns the pixel located at (x, y), ignoring bounds checking.
779787
#[inline(always)]
780788
unsafe fn unsafe_get_pixel(&self, x: u32, y: u32) -> P {
@@ -799,6 +807,14 @@ where
799807
self.get_pixel_mut(x, y)
800808
}
801809

810+
fn opt_pixel_mut(&mut self, x: u32, y: u32) -> Option<&mut P> {
811+
if let Some(idx) = self.pixel_indices(x, y) {
812+
Some(P::from_slice_mut(&mut self.data[idx]))
813+
} else {
814+
None
815+
}
816+
}
817+
802818
fn put_pixel(&mut self, x: u32, y: u32, pixel: P) {
803819
*self.get_pixel_mut(x, y) = pixel
804820
}

src/image.rs

+37
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,25 @@ pub trait GenericImageView {
583583
/// TODO: change this signature to &P
584584
fn get_pixel(&self, x: u32, y: u32) -> Self::Pixel;
585585

586+
/// Get the pixel value at (x, y) when in bounds.
587+
///
588+
/// Return `Some(_)` with the pixel value if the coordinates are [`in_bounds`] and otherwise
589+
/// returns `None`.
590+
///
591+
/// This method can be overridden by image implementations to be more efficient than the manual
592+
/// bounds check. In particular, `get_pixel` will often do its own bounds check and would
593+
/// duplicate this prior step. The optimizer may not detect all of them.
594+
///
595+
/// [`in_bounds`]: #method.in_bounds
596+
// TODO: swap the default implementation to `get_pixel`.
597+
fn opt_pixel(&self, x: u32, y: u32) -> Option<Self::Pixel> {
598+
if self.in_bounds(x, y) {
599+
Some(self.get_pixel(x, y))
600+
} else {
601+
None
602+
}
603+
}
604+
586605
/// Returns the pixel located at (x, y)
587606
///
588607
/// This function can be implemented in a way that ignores bounds checking.
@@ -629,6 +648,24 @@ pub trait GenericImage: GenericImageView {
629648
/// Panics if `(x, y)` is out of bounds.
630649
fn get_pixel_mut(&mut self, x: u32, y: u32) -> &mut Self::Pixel;
631650

651+
/// Gets a reference to the mutable pixel when location `(x, y)` is in bounds.
652+
///
653+
/// Return `Some(_)` with the pixel value if the coordinates are [`in_bounds`] and otherwise
654+
/// returns `None`.
655+
///
656+
/// This method can be overridden by image implementations to be more efficient than the manual
657+
/// bounds check. In particular, `get_pixel` will often do its own bounds check and would
658+
/// duplicate this prior step. The optimizer may not detect all of them.
659+
///
660+
/// [`in_bounds`]: trait.GenericImageView.html#method.in_bounds
661+
fn opt_pixel_mut(&mut self, x: u32, y: u32) -> Option<&mut Self::Pixel> {
662+
if self.in_bounds(x, y) {
663+
Some(self.get_pixel_mut(x, y))
664+
} else {
665+
None
666+
}
667+
}
668+
632669
/// Put a pixel at location (x, y)
633670
///
634671
/// # Panics

0 commit comments

Comments
 (0)