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

Implementing Laplacian and LaplacianEdgeDetector Builders for Laplacian Edge Detection #516

Merged
merged 7 commits into from
Apr 29, 2024

Conversation

thecodergus
Copy link
Contributor

This pull request adds two new builder structures, Laplacian and LaplacianEdgeDetector, to address the request in Issue #505 titled "Laplacian edge detection". These builders are designed to make it easy and flexible for users to perform Laplacian edge detection on grayscale images.

The Laplacian builder allows users to apply a Laplacian filter to a grayscale image. It provides the option to use the default Laplacian kernel or to apply a diagonal adjustment to the kernel. This diagonal adjustment changes the kernel to emphasize diagonal edges in the image. To maximize compatibility and maintainability, the Laplacian builder utilizes existing project functions, such as the filter3x3 function, for filtering the image.

The LaplacianEdgeDetector builder builds on the Laplacian builder and provides a complete edge detection solution. Users can customize the edge detection process by setting parameters such as the Gaussian blur sigma value and the edge threshold value. If these parameters are not provided, the builder will use default values (sigma = 1.4 for Gaussian blur and Otsu's method for edge threshold). The LaplacianEdgeDetector builder also offers an option for diagonal adjustment, similar to the Laplacian builder.

These new builders make it easy for users to perform Laplacian edge detection on grayscale images with just a few lines of code. The flexibility of the builders allows for customization of the edge detection process according to the user's needs. By leveraging existing functions within the project, these additions ensure seamless integration and improved maintainability. I believe these enhancements will be a valuable addition to the project and help users achieve better edge detection results.

@cospectrum
Copy link
Contributor

An entire struct for a Laplacian filter seems redundant. Maybe just 2 functions.

@thecodergus
Copy link
Contributor Author

An entire struct for a Laplacian filter seems redundant. Maybe just 2 functions.

The idea is to give freedom of customization within a certain scope, it is not as if the algorithm does not have variations.

@theotherphil
Copy link
Contributor

This is useful functionality to have, but I agree with @cospectrum that the builder types are unnecessary - a single function for filtering using the Laplacian kernel and a single function for computing edges would be better.

@thecodergus
Copy link
Contributor Author

Following guidance from @cospectrum and @theotherphil, changes have been made

@theotherphil theotherphil merged commit feb8fe7 into image-rs:master Apr 29, 2024
4 of 14 checks passed
@theotherphil
Copy link
Contributor

Thanks, @thecodergus !

@thecodergus
Copy link
Contributor Author

thecodergus commented Apr 29, 2024

Unexpected errors occurred in the merge testing phase @theotherphil, in the version of the library that I forked and edited in the branch, the threshold function had the structure "threshold(image: &GrayImage, thresh: u8) -> GrayImage" but the current one has the structure "threshold(image: &GrayImage, threshold :u8, threshold_type: ThresholdType) -> GrayImage". In this case, I would use the ThresholdType::Binary enumerator in contrast::threshold(&laplacian_img, otsu_threshold, /* contrast::ThresholdType */).

@cospectrum
Copy link
Contributor

Unexpected errors occurred in the merge testing phase @theotherphil, in the version of the library that I forked and edited in the branch, the threshold function had the structure "threshold(image: &GrayImage, thresh: u8) -> GrayImage" but the current one has the structure "threshold(image: &GrayImage, threshold :u8, threshold_type: ThresholdType) -> GrayImage". In this case, I would use the ThresholdType::Binary enumerator in contrast::threshold(&laplacian_img, otsu_threshold, /* contrast::ThresholdType */).

He fixed it #582

@thecodergus thecodergus deleted the feature/add_laplacian_edge branch April 29, 2024 13:49
@cospectrum
Copy link
Contributor

cospectrum commented Apr 29, 2024

It's borken I think #585

@thecodergus
Copy link
Contributor Author

thecodergus commented Apr 29, 2024

It's borken I think #585

I wanted to make sure, so I tested it again, and fortunately, it still seem to be working correctly. Could you please help me take another look at it?
ovo1
ovo1_laplace

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.

3 participants