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

added getColorCharts() #3645

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Conversation

AleksandrPanov
Copy link
Contributor

@AleksandrPanov AleksandrPanov commented Feb 29, 2024

merge this PR after #3647
Computes and returns the coordinates of the central parts of the charts modules.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@AleksandrPanov AleksandrPanov added feature category: documentation Documentation fix or update, does not affect code category: mcc color calibration module labels Feb 29, 2024
* and find by this the coordinates of the central parts of the charts modules.
* It is used in `cv::mcc::CCheckerDraw::draw()` and in `ChartsRGB` calculation.
*/
CV_WRAP virtual std::vector<Point2f> getComputedCharts() = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

M.b. getColorCharts() is more relevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mb getComputedColorCharts()?

Copy link
Contributor

Choose a reason for hiding this comment

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

I propose to name it getColorCharts without computed. The function behaviour may change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@asmorkalov
Copy link
Contributor

Could you add the new method to test(s) and at least one of the samples.

@AleksandrPanov
Copy link
Contributor Author

AleksandrPanov commented Mar 3, 2024

Could you add the new method to test(s) and at least one of the samples.

Method CCheckerDraw::draw() call method getColorCharts()and CCheckerDraw::draw() tested in CV_mccRunCCheckerDrawTest tests.

But now I'm adding a stronger test.

@AleksandrPanov AleksandrPanov changed the title added getComputedCharts() added getComputedColorCharts() Mar 3, 2024
@AleksandrPanov AleksandrPanov marked this pull request as ready for review March 7, 2024 07:20
@AleksandrPanov AleksandrPanov changed the title added getComputedColorCharts() added getColorCharts() Mar 22, 2024
@AleksandrPanov
Copy link
Contributor Author

getColorCharts() tested in CV_mccRunCCheckerDrawTest tests, I don't think a special test is needed.

Copy link
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

👍

@asmorkalov asmorkalov merged commit c8c750a into opencv:4.x Mar 26, 2024
10 checks passed
@asmorkalov asmorkalov self-assigned this Mar 26, 2024
@asmorkalov asmorkalov mentioned this pull request Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: documentation Documentation fix or update, does not affect code category: mcc color calibration module feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants