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

Missing HomographyMethodRHO HomographyMethod added #1191

Merged

Conversation

Krzysztofz01
Copy link
Contributor

Hello!
This is my first pull-request here. There is a link to the official OpenCV documentation attached to the FindHomography function. It mentions four supported homogpraphy methods, but in gocv only three of them have been implemented. I ran local tests on whether the library can work with the RHO method, and all the tests were successful. In accordance with the documentation, I propose to add the missing method.

I also noticed an inconsistency in the names of values and the type (HomographyMethod/HomograpyMethod), but I assume that due to compatibility with previous versions of the library, it will be hard to correct this (marking old values as deprecated and adding new ones without a typo?).

Greetings

@deadprogram
Copy link
Member

deadprogram commented Aug 14, 2024

Thanks for the addition @Krzysztofz01

As far as the misspelling, I think it should be fixed. Once GoCV hits a 1.0 release then we will respect the usual Go version guarantees.

If you want to make that change in this same PR, that would be fine with me! :)

@Krzysztofz01
Copy link
Contributor Author

I have corrected the typos mentioned earlier. I think the changes are ready to be merged :)

@deadprogram
Copy link
Member

Thanks for adding this @Krzysztofz01 now merging.

@deadprogram deadprogram merged commit b881703 into hybridgroup:dev Aug 15, 2024
2 checks passed
@Krzysztofz01 Krzysztofz01 deleted the feature-find-homography-rho-method branch August 15, 2024 11:28
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.

2 participants