search

AvtechScientific / ASL

Advanced Simulation Library - hardware accelerated multiphysics simulation platform.
http://asl.org.il
GNU Affero General Public License v3.0
217 stars 55 forks source link

Explicitly depend on vtkIOGeometry for vtkSTLReader #26

Closed opoplawski closed 7 years ago

opoplawski commented 7 years ago

Fixes issue #25

AvtechScientific commented 7 years ago

Dear @opoplawski,

thank you for the Pull Request!

Will this not break the compatibility with the version 7.0 of VTK? Maybe it should be added as an extra "if" after the existing one? Like this:

if(NOT VTK_FOUND)
    find_package(VTK 7.0 QUIET COMPONENTS vtkRenderingCore vtkImagingCore vtkFiltersCore vtkIOCore vtkIOLegacy vtkIOXML vtkIOMINC vtkCommonCore vtkViewsCore vtksys vtkDICOMParser vtkexpat vtkzlib NO_MODULE)
endif()

if(NOT VTK_FOUND)
    find_package(VTK 7.1 REQUIRED COMPONENTS vtkRenderingCore vtkImagingCore vtkFiltersCore vtkIOCore vtkIOGeometry vtkIOLegacy vtkIOXML vtkIOMINC vtkCommonCore vtkViewsCore vtksys vtkDICOMParser vtkexpat vtkzlib NO_MODULE)
endif()

or even better:

find_package(VTK 6.1 QUIET COMPONENTS vtkRenderingCore vtkImagingCore vtkFiltersCore vtkIOCore vtkIOLegacy vtkIOXML vtkIOMINC vtkCommonCore vtkViewsCore vtkftgl vtksys vtkDICOMParser vtkexpat vtkzlib NO_MODULE)

find_package(VTK 7.0 QUIET COMPONENTS vtkRenderingCore vtkImagingCore vtkFiltersCore vtkIOCore vtkIOLegacy vtkIOXML vtkIOMINC vtkCommonCore vtkViewsCore vtksys vtkDICOMParser vtkexpat vtkzlib NO_MODULE)

if(NOT VTK_FOUND)
    find_package(VTK 7.1 REQUIRED COMPONENTS vtkRenderingCore vtkImagingCore vtkFiltersCore vtkIOCore vtkIOGeometry vtkIOLegacy vtkIOXML vtkIOMINC vtkCommonCore vtkViewsCore vtksys vtkDICOMParser vtkexpat vtkzlib NO_MODULE)
endif()
AvtechScientific commented 7 years ago

@opoplawski ,

can you confirm that following piece of code works with different versions of VTK (6.0, 7.0, 7.1)? If yes, please, update your PR - we would like to credit your contribution.

find_package(VTK 6.1 QUIET COMPONENTS vtkRenderingCore vtkImagingCore vtkFiltersCore vtkIOCore vtkIOLegacy vtkIOXML vtkIOMINC vtkCommonCore vtkViewsCore vtkftgl vtksys vtkDICOMParser vtkexpat vtkzlib NO_MODULE)

find_package(VTK 7.0 QUIET COMPONENTS vtkRenderingCore vtkImagingCore vtkFiltersCore vtkIOCore vtkIOLegacy vtkIOXML vtkIOMINC vtkCommonCore vtkViewsCore vtksys vtkDICOMParser vtkexpat vtkzlib NO_MODULE)

if(NOT VTK_FOUND)
    find_package(VTK 7.1 REQUIRED COMPONENTS vtkRenderingCore vtkImagingCore vtkFiltersCore vtkIOCore vtkIOGeometry vtkIOLegacy vtkIOXML vtkIOMINC vtkCommonCore vtkViewsCore vtksys vtkDICOMParser vtkexpat vtkzlib NO_MODULE)
endif()

Thank you.

opoplawski commented 7 years ago

It should not make a difference to 6.1 as I didn't change that line. I don't have 7.0 so can't really test with that - but all we are doing is explicitly linking to another library which we do depend on. It probably should be added to the 6.1 line just for correctness.

AvtechScientific commented 7 years ago

We have added 'vtkIOGeometry' to the 6.1 line and it seems to work. Please, update your PR. Does 'find_package(VTK 7.0' works for you? Don't you need 'find_package(VTK 7.1'

AvtechScientific commented 7 years ago

@opoplawski,

probably 7.1 is compatible with 7.0, so CMake makes no troubles. It looks like your PR can be merged. Thank you very much for the patch! Could you (like all our contributors), please, follow the steps outlined bellow so we can accept your PR and credit your contribution:

http://asl.org.il/copyright_assignment/

While it looks like a bureaucratic formality - it is the only mechanism we have to enforce AGPL and thus protect ASL (including your contributions) from being exploited. This needs to be done only once for the first pull request.

Thank you again and welcome to the ASL community!