emoon / rust_minifb

Cross platfrom window and framebuffer crate for Rust
MIT License
980 stars 92 forks source link

Make scalar code plain C #350

Closed Speykious closed 3 months ago

Speykious commented 3 months ago

I discovered that minifb uses and compiles non-Rust code for a few use cases, one of which is to make the image scaling functions optimized on Linux in all profiles. I'm not keen on this approach, as it means that minifb is reliant on a separate compiler to build the project. As an example, I am using minifb for one of my projects, and someone who wanted to run it got a compiler error when compiling it on Linux. They didn't have a C++ compiler installed: image

Ideally, I'd like to not depend on a C/C++ compiler on Linux.

One option is writing the scalar functions in Rust into their own crate, make minifb depend on that crate and ensure the profile for that dependency is always optimized:

[profile.dev.package.minifb-scalar]
opt-level = 3

although that means there is a second crate to be published on crates.io.

Another option is to just not optimize it, and instruct users of minifb to compile it with optimizations on all profiles:

[profile.dev.package.minifb]
opt-level = 3

although that means slower minifb if we want its debug info in our program. (There's also debug = true I guess?)

What are your thoughts on this matter?

This discussion aside, I made this PR to remove the need for a C++ compiler to be installed. In my experience, it is much more likely for a C compiler to be installed on someone's Linux machine. I hope it's a bit useful :)

Speykious commented 3 months ago

Shoot, I didn't realize I was out of date... Edit: updated!

emoon commented 3 months ago

Thanks for the PR!

The reason why it's implement the this way is because when the code was added there was no cargo support for having different opt-levels for other crates which has since been added. I think the change to making the code use C only (as it really is) is fine, but I'm not sure if it's worth splitting the code into a separate crate.

Speykious commented 3 months ago

I see. Yeah that makes sense Thanks for merging!