Skip to content

Conversation

@sergiomb2
Copy link
Contributor

@sergiomb2 sergiomb2 commented Oct 14, 2021

The cvv module is compiled conditionally on HAVE_QT5. But in opencv commit 87d4970 this variable was renamed to HAVE_QT, so the module is never compiled.

fixes #3063

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to 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
buildworker:Custom=linux-1,linux-4,linux-6
build_image:Custom=qt:16.04

@antonio-rojas
Copy link

Note that HAVE_QT can mean Qt 4, 5 or 6. So you need to check that the found Qt version is actually compatible with cvv code.

@mshabunin
Copy link
Contributor

I agree, QT_VERSION_MAJOR EQUAL 5 check should be added.

@alalek
Copy link
Member

alalek commented Oct 15, 2021

Thank you for contribution!

This patch should go into 3.4 branch first.
We will merge changes from 3.4 into master regularly (weekly/bi-weekly).

Please:

  • change "base" branch of this PR: master => 3.4 (use "Edit" button near PR title)
  • rebase your commits from master onto 3.4 branch. For example:
    git rebase -i --onto upstream/3.4 upstream/master
    (check list of your commits, save and quit (Esc + "wq" + Enter)
    where upstream is configured by following this GitHub guide and fetched (git fetch upstream).
  • push rebased commits into source branch of your fork (with --force option)

Note: no needs to re-open PR, apply changes "inplace".

@sergiomb2
Copy link
Contributor Author

I agree, QT_VERSION_MAJOR EQUAL 5 check should be added.

and for QT6, can we enable it ? or is known that is not working for QT6 and we also should disable it ?

@sergiomb2 sergiomb2 changed the base branch from master to 3.4 October 18, 2021 00:38
The cvv module is compiled conditionally on HAVE_QT5. But in opencv commit 87d4970 this variable was renamed to HAVE_QT, so the module is never compiled.

Also check if QT_VERSION_MAJOR >= 5
@sergiomb2
Copy link
Contributor Author

Thank you for contribution!

This patch should go into 3.4 branch first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly).

Please:

* change "base" branch of this PR: master => 3.4 (use "Edit" button near PR title)

* rebase your commits from master onto 3.4 branch. For example:
  `git rebase -i --onto upstream/3.4 upstream/master`
  (check list of your commits, save and quit (Esc + "wq" + Enter)
  where `upstream` is configured by following [this GitHub guide](https://help.github.com/articles/configuring-a-remote-for-a-fork/) and fetched (`git fetch upstream`).

* push rebased commits into source branch of your fork (with `--force` option)

Note: no needs to re-open PR, apply changes "inplace".

done, I propose add if not HAVE_QT OR QT_VERSION_MAJOR LESS 5 , disable it, allowing build on QT6 !

Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Well done! Thank you 👍

@opencv-pushbot opencv-pushbot merged commit a1eec28 into opencv:3.4 Oct 18, 2021
@alalek alalek mentioned this pull request Oct 23, 2021
@alalek alalek mentioned this pull request Dec 30, 2021
@alalek alalek mentioned this pull request Feb 22, 2022
@bebuch bebuch mentioned this pull request Jun 16, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cvv module doesn't build in 4.5.4

5 participants