imgref icon indicating copy to clipboard operation
imgref copied to clipboard

Add accessor for continuous buffer

Open kornelski opened this issue 7 years ago • 4 comments

Interoperability with other libraries may require getting the underlying buffer as a slice or vec, but entirely continuous (where width == stride).

kornelski avatar Jul 19 '18 15:07 kornelski

I've needed something similar in the current project I'm working on; Basically I wrote a trait which provides as_slice() but asserts that width == stride. This is super useful for image transfer to graphics APIs such as OpenGL, which always assume that width == stride.

Of course users could do this manually with #8, but it's a common enough use case to warrant additions to the API (IMO).

I'd like to implement this in a PR, but the hardest part is deciding what naming to use in the API.

I thought about as_contiguous_pixels_slice, as_mut_contiguous_pixels_slice, and their friends as_contiguous_pixels_ptr/as_mut_contiguous_pixels_ptr.
These would be implemented for Img<Container> where Container: AsRef<Pixel>/AsMut<Pixel>.
Would that be OK?

yoanlcq avatar Jul 28 '18 09:07 yoanlcq

Yes, naming is hard.

  • Copying method should be named to_*.
  • A method that asserts (and panics) would be useful, but I'm not sure about the name. I can't think of any in std that's similar. Maybe it could be called expect_*?

No idea about the * part though. continuous and contiguous are long and confusingly similar.

There's no need to have *_ptr, since .as_ptr() on the result will work.

kornelski avatar Jul 29 '18 02:07 kornelski

Maybe rename buf to something else, and use buffer or buf to mean contiguous data?

kornelski avatar Jul 29 '18 02:07 kornelski

There's indeed no need for _ptr, but since it's trivial we might as well do it; this would imitate the "look'n feel" of e.g Vec and friends.
I'm also not a big fan of repurposing "buf" because to me, a "buffer" is just a piece raw memory, which only we give semantics to. To me, there's no telling if a buffer is even a contiguous sequence of "elements".

"expect" looks good, but then what would the function names look like ? I think it's OK to provide a panicking API which doesn't explicitly states it in its function names, as long as it's documented.

I think that either way the names will necessarily look a bit funky. I personnally don't mind too much about details as long as it follows the Rust naming conventions (for as_, to_, etc) and names are reasonably self-explanatory :)

yoanlcq avatar Jul 29 '18 05:07 yoanlcq