graspit-simulator / graspit

The GraspIt! simulator
http://graspit-simulator.github.io/
Other
166 stars 80 forks source link

migrating GraspIt! from qt4 to qt5 #178

Closed iretiayo closed 3 years ago

iretiayo commented 3 years ago

qt5 is the minimum qt requirement for Ubuntu 20.04.

This migration borrows some ideas from a previous effort here

This PR has been tested on Ubuntu 20.04 to confirm that the behavior matched the working Qt4 version on Ubuntu 18.04 and below.

The only known issue is with the "Save Image" functionality which breaks on this line with the same error message described here.

Other functionalities of GraspIt! backend and UI seem to be working fine.

mateiciocarlie commented 3 years ago

This looks terrific! Thank you again for this effort.

I have gone through the changes in reasonable details, but it is of course a massive undertaking, so did not look through in complete detail. Unfortunately, we never got proper automated "regression tests"... I know @iretiayo engaged in extensive testing, but there might still be obscure functionality that has regressed. We'll address those cases as people run into them in the future.

From my perspective, this can be merged. @jvarley : anything you want to look at before we go ahead?

jvarley commented 3 years ago

You can enable some tests via: https://github.com/graspit-simulator/graspit/blob/master/CMakeLists.txt#L32

I haven't run the tests in a long time, so it is unclear to me if they would even be passing prior to this PR.

Overall I think this PR is a huge positive, I would say go for it. IMO.

iretiayo commented 3 years ago

Good point. I updated the tests to work with the current version of GraspIt! and successfully ran the tests by flipping the flag as @jvarley pointed out.

All four tests finished and PASSED.