kornelski / imgref

A trivial Rust struct for interchange of pixel buffers with width, height & stride
https://lib.rs/crates/imgref
Apache License 2.0
59 stars 6 forks source link

`Img::map_buf_unchecked` #17

Closed LoganDark closed 2 years ago

LoganDark commented 2 years ago

I'm doing unsafe shenanigans:

    let buf_ptr = Img::new_stride(*buffer.buf() as *const [T], buffer.width(), buffer.height(), buffer.stride());

I promise this is for a good reason (efficiently iterating over the columns of a buffer). I see Img::map_buf exists, but it requires the new buffer to be AsRef, which pointers are not.

I'd appreciate an unsafe function map_buf_unchecked that doesn't need to do any validation checks.

kornelski commented 2 years ago

If you don't need it to be mutable, you can make a slice from the raw pointer. &[T] and *const [T] have identical representation in memory.

LoganDark commented 2 years ago

If you don't need it to be mutable, you can make a slice from the raw pointer. &[T] and *const [T] have identical representation in memory.

I'm not sure what you mean by this. I'm trying to convert an Img<&[T]> to an Img<*const [T]>. map_buf doesn't work for this because it requires that *const [T] be AsRef<[T]> for validation purposes.

kornelski commented 2 years ago

I'm saying don't convert, don't use *const [T].

LoganDark commented 2 years ago

I'm saying don't convert, don't use *const [T].

I need Img<*const [T]> and Img<*mut [T]> at the same time. I can't use normal slices without violating Rust's aliasing rules.

My current implementation (the new_stride line above) passes miri, using references would not.

kornelski commented 2 years ago

Unfortunately, this crate won't work for this. The AsRef and &[T] bounds are on every useful method.

Additionally, I've made a mistake of exposing "padding" (meant for working with block-based video formats), which makes it unsound for column-only slices.

LoganDark commented 2 years ago

Unfortunately, this crate won't work for this. The AsRef and &[T] bounds are on every useful method.

Actually it does, my current solution works quite well. I don't use the imgref iterators, but the Img abstraction is extremely useful for my API surface, especially allowing downstream crates to specify subregions and etc.

In other words, I don't ingest Img<*mut [T]>, I only use it internally so I can use optimized internal iterators. And the type only exists because I choose to use it instead of putting width, height, stride in separate fields. As a type that bundles those 3 extra ints, Img works just fine.

So I really do only need map_buf_unchecked. This is legitimately the only gripe I have with this library right now. :)

kornelski commented 2 years ago

Oh, so I may have misunderstood. You can use it, via new_stride, you just don't like that it's verbose this way, and want a shorter method?

LoganDark commented 2 years ago

Oh, so I may have misunderstood. You can use it, via new_stride, you just don't like that it's verbose this way, and want a shorter method?

Exactly :)

kornelski commented 2 years ago

I'll pass on this. It's only a syntax sugar, and for type that doesn't work well anyway.

LoganDark commented 2 years ago

Then close it as not planned. :p

(the dropdown next to the close/reopen button)