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

Ability to use a image::ImageBuffer directly? #1

Closed
bramp opened this issue Dec 29, 2021 · 6 comments
Closed

Ability to use a image::ImageBuffer directly? #1

bramp opened this issue Dec 29, 2021 · 6 comments

Comments

@bramp
Copy link

bramp commented Dec 29, 2021

Firstly thanks for this library. It worked great for a project I'm working on.

One feature request. I have a image::ImageBuffer and I'd like to pass it directly to bits_to_paths, instead I have to copy it into a Vec::<Vec<i8>>. For example:

    let mut img = GrayImage::new(width, height); // the image::ImageBuffer
    let background_color = Luma([0]); // Black
    let foreground_color = Luma([1]); // White

    contour_tracing::image_to_paths(img, background_color, true);

Thanks

@STPR
Copy link
Owner

STPR commented Dec 30, 2021

Thanks for your comment and I'm glad it worked for your project 😀.

It's an interesting idea but I think it would add some problems, complexity and you will not gain a lot.
Actually, it's just 4 lines of code to add on your side, something like (not tested):

let mut bits = vec![vec![0i8; width]; height];
for p in img.pixels() {
    if p.2 == image::Luma([0]) {
        bits[p.1 as usize][p.0 as usize] = 1;
    }
}

The way it works and to directly use the buffer, it would need to be expanded to add a border and eventually converted to 8 bits.
Probably not impossible but I need to do some research...

@bramp
Copy link
Author

bramp commented Dec 30, 2021

Thanks. Yes my code looks almost like that :-) I wanted to avoid the double memory usage, and needless copies.

Ah looking at your code now, I see you also do a full copy, to add the border around it. So I guess, accepting a ImageBuffer would allow me to avoid one full copy, but you could easily still do that internally.

Either way, no worries if you don't think this is worth adding. I just figured must folks would be doing the processing on images.

@STPR
Copy link
Owner

STPR commented Dec 30, 2021

The library do a little bit more than a simple copy, it adds a border and changes the 0 to -1 to make the rest easier to process.
It would surely be better without adding a border but rust is very strict and you can't read outside of an array or buffer without making your code unsafe...
If I find a better way to deal with the "bound checking" of rust, I will add your idea 😀.

How would you process a 1bit image knowing that you need values other than 0 or 1 ?
At one point, you necessarily need more memory.

@bramp
Copy link
Author

bramp commented Dec 30, 2021

Well rust vectors do have .get() which allow out of bounds and return Optional.

So something like v.get(-1).unwrap_or(0) will return the actual value or zero if out of bounds.

@STPR
Copy link
Owner

STPR commented Dec 30, 2021

Thanks, I'll have a look and try that.

@STPR
Copy link
Owner

STPR commented Jan 10, 2022

get_pixel() can't return Optional and just panic.
We can expect this in the 0.24 version of the image crate:
image-rs/image#1433

- optional get_pixel methods (and more clarity on assert vs. Result)

So, I've added a function single_l8_to_paths() and avoided the panics differently but it's still not optimized:
https://docs.rs/contour_tracing/latest/contour_tracing/image/fn.single_l8_to_paths.html

It needs the feature "image" so don't forget to change your Cargo.toml:

contour_tracing = { version = "*", features = ["image"] }

Please try this and don't hesitate to report some issues.

@STPR STPR closed this as completed Jan 10, 2022
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