Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derive AnyBitPattern for ColorU8 #120

Open
Pr0methean opened this issue May 5, 2024 · 5 comments
Open

Derive AnyBitPattern for ColorU8 #120

Pr0methean opened this issue May 5, 2024 · 5 comments

Comments

@Pr0methean
Copy link

If ColorU8 derived AnyBitPattern, I'd be able to replace the transmute calls in this method with bytemuck's cast and remove the unsafe code. This will be possible even if bytemuck becomes an optional dependency; it'll just have to be guarded by cfg_attr.

https://github.com/Pr0methean/OcHd-RustBuild/blob/main/main/src/image_tasks/png_output.rs#L312-L324

@RazrFalcon
Copy link
Collaborator

Why do you even need to cast ColorU8? Pixmaps store PremultipliedColorU8 and this struct already implements bytemuck::Pod.

@Pr0methean
Copy link
Author

Pr0methean commented May 6, 2024

Because I need to demultiply the Pixmap and still have it in Vec form to pass it to oxipng's raw mode, and I'm trying to minimize allocations in a way I'm pretty sure pixels.into_iter().map(PremultipliedColorU8:: demultiply).collect() wouldn't accomplish. The reason I cast the slice before the conversion and not after is that each PremultipliedColorU8 is also valid as a ColorU8 but not vice-versa.

@RazrFalcon
Copy link
Collaborator

Pixmap cannot be demultiplied. You should call Pixmap::take() and then demultiply bytes manually. No unsafe needed.

@Pr0methean
Copy link
Author

Pr0methean commented May 6, 2024

Right, but I'm trying to do it in place, which means that during the loop the slice will actually contain a mix of multiplied and demultiplied colors that AFAIK can't be represented in safe Rust. I guess I could represent that with a union to avoid the cast/transmute in the demultiply() loop, but unions are unsafe too.

Or are you suggesting I reinvent the demultiply() wheel just to change its argument type to &mut [u8;4], which is exactly the burden I was hoping to avoid by using bytemuck or mem::transmute?

@RazrFalcon
Copy link
Collaborator

RazrFalcon commented May 6, 2024

I still don't understand what you're trying to do. If you're trying to demultiply a Pixmap then it's not possible by design. If you're trying do demultiply just a Vec<u8> then you can easily do it on your own. No need to cast anything.

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

No branches or pull requests

2 participants