Skip to content

Update OpenCVDetectVTK.cmake#20764

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
sturkmen72:Update_OpenCVDetectVTK_cmake
Sep 30, 2021
Merged

Update OpenCVDetectVTK.cmake#20764
opencv-pushbot merged 1 commit intoopencv:masterfrom
sturkmen72:Update_OpenCVDetectVTK_cmake

Conversation

@sturkmen72
Copy link
Copy Markdown
Contributor

solves #20717

@asmorkalov asmorkalov requested a review from rogday September 29, 2021 05:59
@asmorkalov asmorkalov added category: viz OpenCV 4.0+: moved to opencv_contrib category: build/install and removed category: viz OpenCV 4.0+: moved to opencv_contrib labels Sep 29, 2021
find_package(VTK QUIET NAMES vtk VTK)
if(VTK_FOUND)
if(VTK_VERSION VERSION_EQUAL "9") # VTK 9.0
if(VTK_VERSION VERSION_GREATER "8.2.0") # VTK 9.0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is about 8.2.1 patch fix? Why is it should be handled as 9.0?

I expecting this condition here:

if(NOT (VTK_VERSION VERSION_LESS "9.0.0"))  # VTK 9.x+

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

see https://github.com/Kitware/VTK/releases

8.2.0 seems the last ver of 8.x series

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

see https://github.com/Kitware/VTK/releases

8.2.0 seems the last ver of 8.x series

I think, @alalek means that there can be bugfix backported to 8.2, which will break VTK_VERSION VERSION_GREATER "8.2.0" condition. It can be fixed like:
if(NOT (VTK_VERSION VERSION_LESS "9.0.0") AND (VTK_VERSION VERSION_LESS "10.0.0")) # VTK 9.x

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i updated if(NOT (VTK_VERSION VERSION_LESS "9.0.0") AND (VTK_VERSION VERSION_LESS "10.0.0")) # VTK 9.x
but maybe if(NOT (VTK_VERSION VERSION_LESS "9.0.0")) # VTK 9.x+ better because maybe version 10.x will not need any additional changes here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@sturkmen72, it will, since the line below reads find_package(VTK 9 QUIET NAMES vtk COMPONENTS ....

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh! OK.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@alalek let me rebase later if needed.

@sturkmen72 sturkmen72 force-pushed the Update_OpenCVDetectVTK_cmake branch from ff0656c to f8f6cd6 Compare September 30, 2021 09:03
Copy link
Copy Markdown
Member

@rogday rogday left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@opencv-pushbot opencv-pushbot merged commit 9b093c9 into opencv:master Sep 30, 2021
@sturkmen72 sturkmen72 deleted the Update_OpenCVDetectVTK_cmake branch September 30, 2021 19:56
@alalek alalek mentioned this pull request Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: build/install category: viz OpenCV 4.0+: moved to opencv_contrib

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants