Skip to content

Commit

Permalink
Return Option from GenericImageView::get_pixel and GenericImage::get_…
Browse files Browse the repository at this point in the history
…pixel_mut

Return Option<P> from GenericImageView::get_pixel
Return Option<&mut P> from GenericImage::get_pixel_mut
Move the old `get_pixel[_mut]` behavior to Index/IndexMut for ImageBuffer
  • Loading branch information
okaneco committed Mar 16, 2021
1 parent d2f9fe8 commit 710d044
Show file tree
Hide file tree
Showing 14 changed files with 230 additions and 212 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ let (width, height) = img.dimensions();
let pixel = img[(100, 100)];

// Or use the `get_pixel` method from the `GenericImage` trait.
let pixel = *img.get_pixel(100, 100);
let pixel = img.get_pixel(100, 100).unwrap();

// Put a pixel at coordinate (100, 100).
img.put_pixel(100, 100, pixel);
Expand Down Expand Up @@ -223,7 +223,7 @@ fn main() {
i += 1;
}
let pixel = imgbuf.get_pixel_mut(x, y);
let pixel = imgbuf.get_pixel_mut(x, y).unwrap();
let image::Rgb(data) = *pixel;
*pixel = image::Rgb([data[0], i as u8, data[2]]);
}
Expand Down
4 changes: 2 additions & 2 deletions examples/fractal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ fn main() {
i += 1;
}

let pixel = imgbuf.get_pixel_mut(x, y);
let data = (*pixel as image::Rgb<u8>).0;
let pixel = imgbuf.get_pixel_mut(x, y).unwrap();
let data = pixel.0;
*pixel = image::Rgb([data[0], i as u8, data[2]]);
}
}
Expand Down
80 changes: 42 additions & 38 deletions src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,15 +715,13 @@ where
}

/// Gets a reference to the pixel at location `(x, y)`
///
/// # Panics
///
/// Panics if `(x, y)` is out of the bounds `(width, height)`.
pub fn get_pixel(&self, x: u32, y: u32) -> &P {
match self.pixel_indices(x, y) {
None => panic!("Image index {:?} out of bounds {:?}", (x, y), (self.width, self.height)),
Some(pixel_indices) => <P as Pixel>::from_slice(&self.data[pixel_indices]),
pub fn get_pixel(&self, x: u32, y: u32) -> Option<P> {
if let Some(pixel_indices) = self.pixel_indices(x, y) {
if let Some(p) = self.data.get(pixel_indices) {
return Some(*<P as Pixel>::from_slice(p));
}
}
None
}

/// Test that the image fits inside the buffer.
Expand Down Expand Up @@ -866,24 +864,20 @@ where
}

/// Gets a reference to the mutable pixel at location `(x, y)`
///
/// # Panics
///
/// Panics if `(x, y)` is out of the bounds `(width, height)`.
pub fn get_pixel_mut(&mut self, x: u32, y: u32) -> &mut P {
match self.pixel_indices(x, y) {
None => panic!("Image index {:?} out of bounds {:?}", (x, y), (self.width, self.height)),
Some(pixel_indices) => <P as Pixel>::from_slice_mut(&mut self.data[pixel_indices]),
pub fn get_pixel_mut(&mut self, x: u32, y: u32) -> Option<&mut P> {
if let Some(pixel_indices) = self.pixel_indices(x, y) {
if let Some(p) = self.data.get_mut(pixel_indices) {
return Some(<P as Pixel>::from_slice_mut(p));
}
}
None
}

/// Puts a pixel at location `(x, y)`
///
/// # Panics
///
/// Panics if `(x, y)` is out of the bounds `(width, height)`.
pub fn put_pixel(&mut self, x: u32, y: u32, pixel: P) {
*self.get_pixel_mut(x, y) = pixel
if let Some(p) = self.get_pixel_mut(x, y) {
*p = pixel;
}
}
}

Expand All @@ -896,7 +890,7 @@ where
/// Saves the buffer to a file at the path specified.
///
/// The image format is derived from the file extension.
/// Currently only jpeg, png, ico, pnm, bmp and
/// Currently only jpeg, png, ico, pnm, bmp and
/// tiff files are supported.
pub fn save<Q>(&self, path: Q) -> ImageResult<()>
where
Expand Down Expand Up @@ -1018,7 +1012,10 @@ where
type Output = P;

fn index(&self, (x, y): (u32, u32)) -> &P {
self.get_pixel(x, y)
match self.pixel_indices(x, y) {
None => panic!("Image index {:?} out of bounds {:?}", (x, y), (self.width, self.height)),
Some(pixel_indices) => <P as Pixel>::from_slice(&self.data[pixel_indices]),
}
}
}

Expand All @@ -1029,7 +1026,10 @@ where
Container: Deref<Target = [P::Subpixel]> + DerefMut,
{
fn index_mut(&mut self, (x, y): (u32, u32)) -> &mut P {
self.get_pixel_mut(x, y)
match self.pixel_indices(x, y) {
None => panic!("Image index {:?} out of bounds {:?}", (x, y), (self.width, self.height)),
Some(pixel_indices) => <P as Pixel>::from_slice_mut(&mut self.data[pixel_indices]),
}
}
}

Expand Down Expand Up @@ -1065,8 +1065,8 @@ where
(0, 0, self.width, self.height)
}

fn get_pixel(&self, x: u32, y: u32) -> P {
*self.get_pixel(x, y)
fn get_pixel(&self, x: u32, y: u32) -> Option<P> {
self.get_pixel(x, y)
}

/// Returns the pixel located at (x, y), ignoring bounds checking.
Expand All @@ -1089,12 +1089,14 @@ where
{
type InnerImage = Self;

fn get_pixel_mut(&mut self, x: u32, y: u32) -> &mut P {
fn get_pixel_mut(&mut self, x: u32, y: u32) -> Option<&mut P> {
self.get_pixel_mut(x, y)
}

fn put_pixel(&mut self, x: u32, y: u32, pixel: P) {
*self.get_pixel_mut(x, y) = pixel
if let Some(p) = self.get_pixel_mut(x, y) {
*p = pixel;
}
}

/// Puts a pixel at location (x, y), ignoring bounds checking.
Expand All @@ -1109,14 +1111,16 @@ where
///
/// DEPRECATED: This method will be removed. Blend the pixel directly instead.
fn blend_pixel(&mut self, x: u32, y: u32, p: P) {
self.get_pixel_mut(x, y).blend(&p)
if let Some(pixel) = self.get_pixel_mut(x, y) {
pixel.blend(&p);
}
}

fn copy_within(&mut self, source: Rect, x: u32, y: u32) -> bool {
let Rect { x: sx, y: sy, width, height } = source;
let dx = x;
let dy = y;
assert!(sx < self.width() && dx < self.width());
assert!(sx < self.width() && dx < self.width());
assert!(sy < self.height() && dy < self.height());
if self.width() - dx.max(sx) < width || self.height() - dy.max(sy) < height {
return false;
Expand Down Expand Up @@ -1149,8 +1153,8 @@ where
}
}

// FIXME non-generic `core::slice::copy_within` implementation used by `ImageBuffer::copy_within`. The implementation is rewritten
// here due to minimum rust version support(MSRV). Image has a MSRV of 1.34 as of writing this while `core::slice::copy_within`
// FIXME non-generic `core::slice::copy_within` implementation used by `ImageBuffer::copy_within`. The implementation is rewritten
// here due to minimum rust version support(MSRV). Image has a MSRV of 1.34 as of writing this while `core::slice::copy_within`
// has been stabilized in 1.37.
#[inline(always)]
fn slice_copy_within<T: Copy>(slice: &mut [T], Range { start: src_start, end: src_end }: Range<usize>, dest: usize) {
Expand Down Expand Up @@ -1301,12 +1305,12 @@ where
/// ```no_run
/// use image::buffer::ConvertBuffer;
/// use image::GrayImage;
///
///
/// let image_path = "examples/fractal.png";
/// let image = image::open(&image_path)
/// .expect("Open file failed")
/// .to_rgba();
///
///
/// let gray_image: GrayImage = image.convert();
/// ```
fn convert(&self) -> ImageBuffer<ToType, Vec<ToType::Subpixel>> {
Expand Down Expand Up @@ -1360,15 +1364,15 @@ mod test {
let b = a.get_mut(3 * 10).unwrap();
*b = 255;
}
assert_eq!(a.get_pixel(0, 1)[0], 255)
assert_eq!(a.get_pixel(0, 1).unwrap()[0], 255)
}

#[test]
fn mut_iter() {
let mut a: RgbImage = ImageBuffer::new(10, 10);
{
let val = a.pixels_mut().next().unwrap();
*val = color::Rgb([42, 0, 0]);
*val = Rgb([42, 0, 0]);
}
assert_eq!(a.data[0], 42)
}
Expand Down Expand Up @@ -1466,7 +1470,7 @@ mod benchmarks {
let mut sum: usize = 0;
for y in 0..1000 {
for x in 0..1000 {
let pixel = image.get_pixel(x, y);
let pixel = image[(x, y)];
sum = sum.wrapping_add(pixel[0] as usize);
sum = sum.wrapping_add(pixel[1] as usize);
sum = sum.wrapping_add(pixel[2] as usize);
Expand All @@ -1493,7 +1497,7 @@ mod benchmarks {
let mut sum: usize = 0;
for x in 0..1000 {
for y in 0..1000 {
let pixel = image.get_pixel(x, y);
let pixel = image.get_pixel(x, y).unwrap();
sum = sum.wrapping_add(pixel[0] as usize);
sum = sum.wrapping_add(pixel[1] as usize);
sum = sum.wrapping_add(pixel[2] as usize);
Expand Down
29 changes: 16 additions & 13 deletions src/codecs/gif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl<'a, R: 'a + Read> ImageDecoder<'a> for GifDecoder<R> {
assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes()));

let frame = match self.reader.next_frame_info()
.map_err(ImageError::from_decoding)? {
.map_err(ImageError::from_decoding)? {
Some(frame) => FrameInfo::new_from_frame(frame),
None => {
return Err(ImageError::Parameter(ParameterError::from_kind(
Expand Down Expand Up @@ -140,8 +140,8 @@ impl<'a, R: 'a + Read> ImageDecoder<'a> for GifDecoder<R> {
let frame_x = x.wrapping_sub(frame.left);
let frame_y = y.wrapping_sub(frame.top);

if frame_x < frame.width && frame_y < frame.height {
*pixel = *frame_buffer.get_pixel(frame_x, frame_y);
if let Some(frame_buffer_pixel) = frame_buffer.get_pixel(frame_x, frame_y) {
*pixel = frame_buffer_pixel;
} else {
// this is only necessary in case the buffer is not zeroed
*pixel = Rgba([0, 0, 0, 0]);
Expand Down Expand Up @@ -261,23 +261,26 @@ impl<R: Read> Iterator for GifFrameIterator<R> {
let image_buffer = if (frame.left, frame.top) == (0, 0)
&& (self.width, self.height) == frame_buffer.dimensions() {
for (x, y, pixel) in frame_buffer.enumerate_pixels_mut() {
let previous_pixel = self.non_disposed_frame.get_pixel_mut(x, y);
blend_and_dispose_pixel(frame.disposal_method, previous_pixel, pixel);
if let Some(previous_pixel) = self.non_disposed_frame.get_pixel_mut(x, y) {
blend_and_dispose_pixel(frame.disposal_method, previous_pixel, pixel);
}
}
frame_buffer
} else {
ImageBuffer::from_fn(self.width, self.height, |x, y| {
let frame_x = x.wrapping_sub(frame.left);
let frame_y = y.wrapping_sub(frame.top);
let previous_pixel = self.non_disposed_frame.get_pixel_mut(x, y);

if frame_x < frame_buffer.width() && frame_y < frame_buffer.height() {
let mut pixel = *frame_buffer.get_pixel(frame_x, frame_y);
blend_and_dispose_pixel(frame.disposal_method, previous_pixel, &mut pixel);
pixel
} else {
// out of bounds, return pixel from previous frame
*previous_pixel
let frame_buffer_pixel = frame_buffer.get_pixel(frame_x, frame_y);

match (previous_pixel, frame_buffer_pixel) {
(Some(previous_pixel), Some(frame_buffer_pixel)) => {
let mut pixel = frame_buffer_pixel;
blend_and_dispose_pixel(frame.disposal_method, previous_pixel, &mut pixel);
pixel
}
(Some(previous_pixel), None) => *previous_pixel,
_ => Rgba([0, 0, 0, 0])
}
})
};
Expand Down
6 changes: 3 additions & 3 deletions src/codecs/jpeg/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -813,13 +813,13 @@ fn rgb_to_ycbcr<P: Pixel>(pixel: P) -> (u8, u8, u8) {
/// otherwise the closest pixel in the image
#[inline]
fn pixel_at_or_near<I: GenericImageView>(source: &I, x: u32, y: u32) -> I::Pixel {
if source.in_bounds(x, y) {
source.get_pixel(x, y)
if let Some(pixel) = source.get_pixel(x, y) {
pixel
} else {
source.get_pixel(
x.min(source.width() - 1),
y.min(source.height() - 1),
)
).unwrap()
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/codecs/png.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,9 @@ impl<R: Read> ApngDecoder<R> {
BlendOp::Over => {
// TODO: investigate speed, speed-ups, and bounds-checks.
for (x, y, p) in source.enumerate_pixels() {
self.current.get_pixel_mut(x + px, y + py).blend(p);
if let Some(pixel) = self.current.get_pixel_mut(x + px, y + py) {
pixel.blend(p);
}
}
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/dynimage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,8 +996,12 @@ impl GenericImageView for DynamicImage {
dynamic_map!(*self, ref p -> p.bounds())
}

fn get_pixel(&self, x: u32, y: u32) -> color::Rgba<u8> {
dynamic_map!(*self, ref p -> p.get_pixel(x, y).to_rgba().into_color())
fn get_pixel(&self, x: u32, y: u32) -> Option<Self::Pixel> {
dynamic_map!(*self, ref p -> if let Some(pix) = p.get_pixel(x, y) {
Some(pix.to_rgba().into_color())
} else {
None
})
}

fn inner(&self) -> &Self::InnerImageView {
Expand Down Expand Up @@ -1040,7 +1044,7 @@ impl GenericImage for DynamicImage {
}

/// DEPRECATED: Do not use is function: It is unimplemented!
fn get_pixel_mut(&mut self, _: u32, _: u32) -> &mut color::Rgba<u8> {
fn get_pixel_mut(&mut self, _: u32, _: u32) -> Option<&mut color::Rgba<u8>> {
unimplemented!()
}

Expand Down Expand Up @@ -1260,7 +1264,7 @@ pub fn write_buffer_with_format<W, F>(
format: F,
) -> ImageResult<()>
where
W: std::io::Write,
W: Write,
F: Into<ImageOutputFormat>,
{
// thin wrapper function to strip generics
Expand Down
Loading

0 comments on commit 710d044

Please sign in to comment.