search

emoon / rust_minifb

Cross platfrom window and framebuffer crate for Rust
MIT License
1.01k stars 97 forks source link

fix image example buffer size calculation #359

Closed JanNeuenfeld closed 2 weeks ago

JanNeuenfeld commented 2 weeks ago

The calculation of the buffer size for a loaded image in the example code currently allocates a Vec<u32> that is exactly 4 times longer than needed. This is due to png::decoder::Reader::output_buffer_size() returning the size of the buffer in bytes. These bytes are later packed to u32s, which reduces the needed length of the u32 buffer to png::decoder::Reader::output_buffer_size() / 4

JanNeuenfeld commented 2 weeks ago

Also i went through the clippy suggestions. Please pay extra attention to the changed slice size calculations as I'm extremely tired at the moment and not sure if i overlooked reasons to not use std::mem::size_of_val in certain places.

emoon commented 2 weeks ago

Thanks for the PR! LGTM 👍

emoon commented 2 weeks ago

I will do some testing before merging this

emoon commented 2 weeks ago

This change breakes the image test

cargo run --example image

Gives

thread 'main' panicked at examples/image.rs:23:39:
called `Result::unwrap()` on an `Err` value: Parameter(ParameterError { inner: ImageBufferSize { expected: 24, actual: 409600 } })
stack backtrace:
   0: rust_begin_unwind
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_fmt
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/result.rs:1679:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/result.rs:1102:23
   4: image::main
             at ./examples/image.rs:23:5
   5: core::ops::function::FnOnce::call_once
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
JanNeuenfeld commented 2 weeks ago

Okay that's embarrassing, sorry about that. As far as I can tell everything still works on commit 3a4b729, so I'm pretty sure I messed up on dbe8eb1 when replacing manual slice size calculations.

emoon commented 2 weeks ago

Sorry, I kinda screwed up when merging this and then reverting it. I wonder if you can open up a new PR with the correct fixes?