Feature/qt4 qt5

[pierrewillenbrockdfki]

This makes the vizkit3d-qt5 library available on systems having the required qt5 prerequisites. Qt4 using packages should (and have been tested to) be unaffected.

[planthaber]

Looks good to me, bu i didn't test the qt4 version

[doudou]

Right.

So Qt4 plugins can't be built on Qt5 and Qt5 on Qt4.

My question for you guys is: wouldn't it make more sense to create a separate package for qt5 support instead of this ? It's a lot of if (qt4/qt5) and a lot of optional dependencies.

[planthaber]

No, this would tear the development apart, new features will probably not be merged and only available for qt4 or qt5, not both. This way, those are instantly available in all rock flavors, no difference if qt4/5 or soon 6 is used.

The other packages have a lot less changes, as most differences are handled in the base classes here (Plugins, Widgets)

The majority of code is still shared between qt4 and 5, changes are mostly only needed on plugins (e.g. QT designer interface)

#if QT_VERSION >= 0x050000
#include <QtUiPlugin/QDesignerCustomWidgetInterface>
#else
#include <QtDesigner/QDesignerCustomWidgetInterface>
#endif

Ans those parts are also a nice documantation on which parts probably have to be changed when porting to another future version.

Even the rock_widget_collection has fixes that are not in master yet: rock-core/gui-rock_widget_collection@master...feature/qt4-qt5

[chhtz]

I would actually appreciate if we could improve the cmake-macros such that instead of writing (e.g. for the base-viz plugin)

if (ROCK_QT_VERSION_4)
rock_vizkit_plugin(base-viz
        ${BASE_VIZ_SOURCES}
    HEADERS
        ${BASE_VIZ_HEADERS}
    MOC
        ${BASE_VIZ_MOC}
    DEPS base-types
    LIBS ${Boost_SYSTEM_LIBRARY}
    DEPS_PKGCONFIG base-logging PrimitivesFactory
)
endif()

if (ROCK_QT_VERSION_5)
rock_vizkit_plugin_qt5(base-viz-qt5
        ${BASE_VIZ_SOURCES}
    HEADERS
        ${BASE_VIZ_HEADERS}
    MOC5
        ${BASE_VIZ_MOC}
    DEPS base-types
    LIBS ${Boost_SYSTEM_LIBRARY}
    DEPS_PKGCONFIG base-logging PrimitivesFactory
)
endif()

One could simply write

rock_vizkit_plugin_qt(base-viz   # _qt suffix adds -qt5 suffix to plugin if needed
        ${BASE_VIZ_SOURCES}
    HEADERS
        ${BASE_VIZ_HEADERS}
    MOC                          # for qt5 this will do the same as MOC5
        ${BASE_VIZ_MOC}
    DEPS base-types
    LIBS ${Boost_SYSTEM_LIBRARY}
    DEPS_PKGCONFIG base-logging PrimitivesFactory
)

Of course something equivalent like rock_library_qt, rock_executable_qt etc.
And some mechanism which automatically qt5 suffixes to dependencies which may have them.

On platforms without qt4-support the -qt5 suffix could be dropped (or maybe this can be made configurable) -- also this will ease transitioning to qt6 (and later) once that is necessary.

With a global configuration option one could even easily deactivate everything qt-related (e.g., when building on robots with limited HD space, or non x86-architectures without qt implementations -- or just to save some build time).

[pierrewillenbrockdfki]

My question for you guys is: wouldn't it make more sense to create a separate package for qt5 support instead of this? It's a lot of if (qt4/qt5) and a lot of optional dependencies.

I do admit that the CMakeLists.txt do not look pretty. I have people asking about making macros to unify the qt library creation, but i don't see how to do that while retaining the users ability to see what library targets exist. The best i could come up with is setting variables with the file lists and then have the if(ROCK_QT_4)/if(ROCK_QT_5) sections with their respective differences.

I am open to suggestions on how to improve the cmake integration. Maybe documenting that a new macro produces more than one target, and under what circumstances it does so, is enough? Users can also check for the targets existence, if they need to.

On the cpp/hpp side, the Qts do not differ much. vizkit3d is especially hit because the Qts plugin system changed. Simple plugins generally don't need care at all, and plugins with multiple visualizations look like this: base types PluginLoader.hpp base types PluginLoader.cpp. Visualizations can even depend on the Qts pulled by vizkit3d in their manifest.xml.

(The other changes that i am hitting a lot is around the toAscii(removed)/toUtf8/toLatin1/toLocal8Bit/toStdString area, and include renaming around uitools, widgets, gui.)

If we split vizkit3d and visualizaions, we'd have the visualizations each in their own package(base/types-viz-qt4 and base/types-viz-qt5 separate from base/types), the former two with extremely similar code(if not exactly the same code) that needs to be maintained with exactly the same patches, when changing cpp/hpp files.

If we only split vizkit3d, we'd have each visualization with a pair of optional vizkit3d dependencies, while still duplicating most of vizkit3ds code, and the visualizations still need to figure out what Qt is available.

There is another option to create two vizkit3d packages from the same source(using a cmake parameter to differentiate the qt version), which has the same effect for visualizations as above, but does not duplicate actual code.

[pierrewillenbrockdfki]

I'd rather see

rock_vizkit_plugin_qt(base   # _qt suffix adds -viz or -viz-qt5 suffix to plugin as needed

so there is no confusion that this sometimes creates a base target. Also, this will need to have LIBSQT5, LIBSQT4, DEPSQT5, DEPSQT4 etc to capture the differences in requirements(if there are any). But maybe it still is usable for simple plugins as is.

[chhtz]

I'd rather see

rock_vizkit_plugin_qt(base   # _qt suffix adds -viz or -viz-qt5 suffix to plugin as needed

Fine for me as well.

[...] Also, this will need to have LIBSQT5, LIBSQT4, DEPSQT5, DEPSQT4 etc to capture the differences in requirements(if there are any). But maybe it still is usable for simple plugins as is.

Yes, but also LIBSQT, DEPSQT, etc which will add the -qt5 suffix when it is required (i.e., if that is the only difference between the qt4 and qt5 libraries). This may need a global configuration option.

[doudou]

@pierrewillenbrockdfki thanks for all that work. Feel free to merge as you see fit ;-)