Skip to content

Replace impl of Rows with ChunksExact#2942

Open
RunDevelopment wants to merge 1 commit into
image-rs:mainfrom
RunDevelopment:image-buffer-rows-rm
Open

Replace impl of Rows with ChunksExact#2942
RunDevelopment wants to merge 1 commit into
image-rs:mainfrom
RunDevelopment:image-buffer-rows-rm

Conversation

@RunDevelopment
Copy link
Copy Markdown
Member

@RunDevelopment RunDevelopment commented May 7, 2026

Changes:

  • Make Rows a type alias for ChunksExact. Same for mut.

After #2938, Rows was only a wrapper around ChunksExact. In this PR I removed the wrapper and made it a type alias. At the cost of removing an abstraction (and having less control), I removed 150 LOC. This should also improve performance in some cases, since ChunksExact implements certain unstable traits and relies less on defaults. E.g. image.rows().nth(100) is now an efficient way to get the 100th row. Same for mut.


I also contemplated removing the type aliases, since they are only used once.

@RunDevelopment
Copy link
Copy Markdown
Member Author

Another idea: Make Rows a view of a slice.

Basically like this:

#[derive(Copy, Clone, Debug, ...)]
pub struct Rows<'a, P> {
  pixels: &'a [P],
  width: u32,
}
impl Rows<'_, _> {
  pub fn iter(&self) -> ChunksExact;
  pub fn par_iter(&self) -> ParChunksExact;
  pub fn get(&self, y: u32) -> Option<&[P]>;
}
impl IntoIterator for Rows;
impl Index<u32> for Rows;
// similar for RowsMut

This would grow the API surface, but enable us to (1) put all row-related functionality in one place and (2) add some functionality that would be harder to justify without. E.g. a user (or we) may need mutable access to 2 different rows. With RowsMut like that, there'd be a natural way to put a get_disjoint_mut à la Vec. (This can be done by slicing pixels but is tricky.)


Honestly, I'm not sure what the best API for image.rows() should be. I just feel like an iterator is the wrong choice.

@197g
Copy link
Copy Markdown
Member

197g commented May 8, 2026

Hm, providing IntoIterator without implementing Iterator itself is certainly possible? It would also justify having the struct be Copy and the indexing idea / future direction in particular for mutable rows is cute. Hm. I don't necessarily want to add breaking changes for the sake of cute, churn is a real downside. In particular here where a slice to the whole image is readily available (now) and there are considerable alternatives for representing the channel matrix such that you can slice them in both directions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants