image-rs / imageproc

Image processing operations
MIT License
758 stars 149 forks source link

`Rect` refactors #638

Closed ripytide closed 5 months ago

ripytide commented 6 months ago

I came across the Rect type when starting on some functions which take rectangular regions such as crop(). I would like to use this type to take arguments to the crop() function. However, there are several issues I have with it at the moment that make me reluctant to go near it.

Currently defined as:

pub struct Rect {
    left: i32,
    top: i32,
    width: u32,
    height: u32,
}

Proposal: May I suggest we change Rect to be defined as:

pub struct Rect {
    pub x: u32,
    pub y: u32,
    pub width: u32,
    pub height: u32,
}

The docs can then state that the xy define the top-left point of the rectangle.

This also matches other Rect types in the rust ecosystem such as the Rectangle from iced.

ripytide commented 6 months ago

The motivation for this change was me wanting to implement crop() which would take a Rect but the current Rect is not conducive to the crop() functions requirements as described above.

crop() and similar functions are what I'm implementing as a part of https://github.com/image-rs/image/issues/2238

ripytide commented 6 months ago

Actually, I've just found the Rect from image which is already fits the exact pattern I'd proposed. I'm going to close this and open a different issue that changes the suggestion to migrating to image::Rect.