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

Better support for QT5 / VTK 6.x #1217

Merged
merged 2 commits into from
May 11, 2015
Merged

Better support for QT5 / VTK 6.x #1217

merged 2 commits into from
May 11, 2015

Conversation

Tobias-Fischer
Copy link
Contributor

Hi all,

This PR adds some features related to QT5.

Firstly, by default QT5 is used. Previously, find_package(qt4) ahead of finding QT5 gave linking issues as the wrong paths to qmake etc. were found. Please note that it would be nicer if one could choose the version of QT one wants to use. This could look like this:

SET(PCL_QT_VERSION 4 CACHE STRING "Which QT version to use")
IF("${PCL_QT_VERSION}" STREQUAL "4")
  find_package(Qt4)
ELSE()
  include(cmake/pcl_find_qt5.cmake)
ENDIF()

Let me know if you prefer that alternative, and I'll update the PR.

Secondly, some apps relied on QVTK in QT4. As this is now (somehow) integrated in QT5, those apps were not compiled. I changed this so these apps are properly compiled.
As soon as you are happy and merged this PR, I will have a look that also apps/cloud_composer, apps/modeler and apps/optronic_viewer are compiled with QT5 (same changes needed as in apps/CMakeLists.txt).

Best,
Tobias

@taketwo
Copy link
Member

taketwo commented Apr 28, 2015

Hi Tobias, great contribution! I'd vote for giving the user control over the version and set the default version to 5.

@Tobias-Fischer
Copy link
Contributor Author

Hi,
I actually just found that the changes regarding QVTK are not correct. As far as I understand, this is not an issue concerning QT4 vs QT5, but an issue regarding VTK 5.x vs VTK 6.x!
In VTK 6.x, there is no such thing as QVTK anymore. Instead, ${VTK_LIBRARIES} should be used instead (from http://www.vtk.org/Wiki/VTK/Tutorials/CMakeListsFileForQt4).
As I am using VTK 6.x, this would perfectly make sense.

I'll update the pull request tomorrow accordingly, and take your advice and provide the user with the ability to choose the QT version.

Best, Tobias

@Tobias-Fischer Tobias-Fischer changed the title Better support for QT5 Better support for QT5 / VTK 6.x Apr 28, 2015
@Tobias-Fischer
Copy link
Contributor Author

Hi,
short update on this:
The evil commit which broke functionality was this one: 2618b52

I reverted it here, as QVTK_LIBRARY_DEBUG_NAME and QVTK_LIBRARY_RELEASE_NAME is nowhere set. @Tonsty How was/is this suppose to work?

I still added the option to switch between QT4 and QT5, with QT5 being the default option.

Furthermore, I fixed some issues in apps/cloud_composer regarding QT5. However, the application does not work properly (although compiling and linking fine). This is because I had to comment the deprecated macro Q_EXPORT_PLUGIN2(). Instead, one should use Q_PLUGIN_METADATA, but I am not a QT expert so I leave it to someone who better knows what's going on there (I added TODO's in the code).

Best,
Tobias

@VictorLamoine
Copy link
Contributor

Just in case you missed this discussion.
Also see #1196

@Tobias-Fischer
Copy link
Contributor Author

Indeed I missed this discussion. Thanks for pointing that out!

@Tobias-Fischer
Copy link
Contributor Author

Hi,
just a quick check how you want me to proceed with this pull request. Shall I take out all the stuff related to VTK? Although it is seriously broken the way it is now ;).
Also, is there someone around who could know about the (correct usage of the) Q_PLUGIN_METADATA macro?

Thanks,
Tobias

@jspricke
Copy link
Member

jspricke commented May 5, 2015

Hi,

VTK6 and QT5 are the future, so I'm really looking forward to integrate your pull request. But as you write, I think we need someone to fix the TODOs in there first ;).

@Tobias-Fischer
Copy link
Contributor Author

Hi,
an hour of work and I got the plugin system working with QT5. Please let me know what other issues need to be resolved so that pull request can be merged.
I did a brief test by opening the cloud composer, loading a cloud from the test folder, applying the sanitizer filter, and then Euclidian clustering.

Best, Tobias

@jspricke jspricke mentioned this pull request May 6, 2015
@Tobias-Fischer
Copy link
Contributor Author

@jspricke Regarding your comment in #1196: if you want, I can just remove all QVTK related stuff in my pull request. Then, only the QT5 things are left, and everything related to QVTK can be handled in #1196?

Best, Tobias

@jspricke
Copy link
Member

jspricke commented May 7, 2015

@Tobias-Fischer I was waiting for the others (@VictorLamoine) to comment. Otherwise feel free to include the revert commit in here as well, as I would say it's related.

@VictorLamoine
Copy link
Contributor

I think it's more logical to do everything here!
If you could take care of reverting #1053 that would be nice.

- provide option which QT version to use
- compile more apps which were not available using VTK 6.x
This reverts commit afab49f, reversing
changes made to 637ebb2.

Conflicts:
	CMakeLists.txt
@Tobias-Fischer
Copy link
Contributor Author

Okay cool, all done 👍. Looking forward to see this merged :).

Best, Tobias

@VictorLamoine
Copy link
Contributor

From what I understand what was QVTK in VTK 5.x is now vtkGUISupportQt in VTK 6.x.

I would like to make sure that we don't break VTK 5.x support as Ubuntu 14.04 still ships with VTK 5.8 by default! Ubuntu 14.04 ships with Qt 5.2.1 but it would be more than nice to keep Qt 4.x compatibility.

Without BUILD_apps = OFF (as Tobias said apps needs further fixing), PCL builds fine on my side, so I think it's ok to merge this.

jspricke added a commit that referenced this pull request May 11, 2015
Better support for QT5 / VTK 6.x
@jspricke jspricke merged commit 451f9ff into PointCloudLibrary:master May 11, 2015
@Tobias-Fischer
Copy link
Contributor Author

Hi,
Thanks for merging :).
VTK 5.x definitely should not be broken, in fact this merge did not change anything related to VTK except of going back to the old, working way of finding the package.
All the changes were made related to QT5. @VictorLamoine: you can set BUILD_apps=on, as everything is fixed by now!

Best, Tobias

@VictorLamoine
Copy link
Contributor

No, everything is not fixed:

 47%] Building CXX object apps/CMakeFiles/pcl_openni_passthrough.dir/include/pcl/apps/moc_openni_passthrough.cpp.o
In file included from /home/dell/libraries/pcl/build_VTK_5.8/apps/ui_openni_passthrough.h:21:0,
                 from /home/dell/libraries/pcl/src/apps/include/pcl/apps/openni_passthrough_qt.h:45,
                 from /home/dell/libraries/pcl/build_VTK_5.8/apps/include/pcl/apps/../../../../../src/apps/include/pcl/apps/openni_passthrough.h:42,
                 from /home/dell/libraries/pcl/build_VTK_5.8/apps/include/pcl/apps/moc_openni_passthrough.cpp:9:
/usr/include/vtk-5.8/QVTKWidget.h:40:25: fatal error: QtGui/QWidget: No such file or directory
 #include <QtGui/QWidget>
                         ^
compilation terminated.
make[2]: *** [apps/CMakeFiles/pcl_openni_passthrough.dir/include/pcl/apps/moc_openni_passthrough.cpp.o] Error 1
make[1]: *** [apps/CMakeFiles/pcl_openni_passthrough.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

They were already broken before... in another way 😄

@Tobias-Fischer
Copy link
Contributor Author

What versions of QT/VTK are you using? Are you sure you compiled VTK with the same version of QT you are trying to compile pcl?

@VictorLamoine
Copy link
Contributor

Sorry for the delay Tobias, I tested again and everything works seamlessly. I was probably mixing versions.

@Tobias-Fischer
Copy link
Contributor Author

Hi Victor, I'm glad to hear all works nicely for you as well.
Indeed strange things happen if you mix the QT versions of VTK and pcl, but I don't know how one would be able to detect that someone is trying to do that. Maybe a warning could be added somewhere?

@42loop
Copy link

42loop commented Feb 9, 2016

hi,
i still feel this is not fixed. yesterday in desperation i dumped the vtk5.8 on ubuntu 14.04.3 for vtk6.0 and got a little further. still if i check 'build apps' i get

/usr/include/vtk-6.0/QVTKWidget.h:41:25: fatal error: QtGui/QWidget: No such file or directory
#include <QtGui/QWidget>
^
compilation terminated.

this has been bugging me for months now - out of ideas what to modify and where

any help appreciated

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.

5 participants