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

New feature: Flood-fill #684

Merged
merged 5 commits into from
Oct 19, 2024
Merged

Conversation

tkallady
Copy link
Contributor

@tkallady tkallady commented Sep 5, 2024

Similar to bucket tool in MS-PAINT.
I've just taken the algorithm from here:
https://en.wikipedia.org/wiki/Flood_fill#Span_filling

I'm fairly new to rust and didn't know how to do generic Pixel equality without first converting to rgba pixel, perhaps someone knows how to do this better?:

x < width && y < height && image.get_pixel(x, y).to_rgba() == target_color.to_rgba()

src/drawing/fill.rs Outdated Show resolved Hide resolved
Comment on lines 752 to 770
#[test]
fn test_draw_flood_filled_shape() {
use imageproc::drawing::{draw_hollow_ellipse_mut, flood_fill};

let red = Rgb([255, 0, 0]);
let green = Rgb([0, 255, 0]);
let blue = Rgb([0, 0, 255]);
let mut image = RgbImage::from_pixel(200, 200, Rgb([255, 255, 255]));

draw_hollow_ellipse_mut(&mut image, (100, 100), 50, 50, red);
draw_hollow_ellipse_mut(&mut image, (50, 100), 40, 90, blue);
draw_hollow_ellipse_mut(&mut image, (100, 150), 80, 30, green);
draw_hollow_ellipse_mut(&mut image, (150, 150), 100, 60, blue);

flood_fill(&mut image, 120, 120, red);

compare_to_truth_image(&image, "flood_filled_shape.png");
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome test!

src/drawing/fill.rs Show resolved Hide resolved
src/drawing/fill.rs Outdated Show resolved Hide resolved
filled_image
}

#[doc=generate_mut_doc_comment!("draw_line_segment")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to update this to the same name as the non-mut method

Comment on lines 766 to 768
flood_fill(&mut image, 120, 120, red);
let filled_image = flood_fill(&image, 120, 120, red);
compare_to_truth_image(&filled_image, "flood_filled_shape.png");

flood_fill_mut(&mut image, 120, 120, red);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really any need to test both variants since they are the same method underneath.

@ripytide
Copy link
Member

Apart from the two new nits, I'm happy for this PR to be merged.

@tkallady
Copy link
Contributor Author

All done. Thanks.

@cospectrum
Copy link
Contributor

cospectrum commented Sep 19, 2024

for clippy lint you can merge master, for doc lint just fix url

@tkallady
Copy link
Contributor Author

tkallady commented Oct 9, 2024

CI should pass now.

@cospectrum
Copy link
Contributor

cospectrum commented Oct 19, 2024

Actually I think clippy will break with each new lint until they run out of ideas. Almost every month

@theotherphil
Copy link
Contributor

Thanks!

@theotherphil theotherphil merged commit a6fe217 into image-rs:master Oct 19, 2024
15 of 16 checks passed
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

Successfully merging this pull request may close these issues.

4 participants