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

Migrate to newer version of ignition-gui #332

Closed
scpeters opened this issue Dec 3, 2020 · 41 comments
Closed

Migrate to newer version of ignition-gui #332

scpeters opened this issue Dec 3, 2020 · 41 comments

Comments

@scpeters
Copy link
Contributor

scpeters commented Dec 3, 2020

We are currently using ignition-gui0, but this is an old package that is not publicly supported. Open Robotics is supporting it for TRI's purposes, but it would be good to migrate to a newer version. Since we have already upgraded to ignition-rendering2, it would be a good first to step to migrate to supporting ignition-gui2 since it also uses ignition-rendering2. Upgrading to ignition-msgs4 and ignition-transport7 as well would consolidate to using only packages from the ignition-blueprint collection.

It is straightforward to upgrade to ignition-msgs4 and ignition-transport7 (see the scpeters/ignition-blueprint branches in this repo and in delphyne, which require small changes to the protobuf API in order to compile in c747432 and maliput/delphyne@5181f0a). The most significant challenge is upgrading to ignition-gui2 since it has undergone major architectural changes, using QtQuick instead of QWidgets and since display plugins haven't yet been migrated.

It would be ideal to be able to use both ignition-gui0 and ignition-gui2 in side-by-side visualizer executables to allow for a gradual transition. This would require updating ignition-gui0 to use ignition-msgs4 and ignition-transport7, which I think is plausible.

In terms of the effort to migrate each of our widgets, a co-worker (chapulina) has provided the following estimate of the complexity of porting each widget:

"Ok, let's see each plugin
S for days, M for weeks, L for months

  • 3D scene: S: just use the new Scene3D
  • Topic viewer: S: just use the new TopicViewer
  • Scene tree: M: I'm not sure what data they're using to populate that. The widget generation code needs to be ported to QML.
  • Time: S: just use the new WorldControl and WorldStats plugins
  • Topic Stats: M: we need to port it to QML
  • RTF display: S: comes in the new WorldStats plugin, unless they really want a floating 3D text again. With QML we can easily overlay widgets, so I think they may be happy with that.
  • Grid display: S if they accept to use the Grid3D plugin that we already have
  • Teleop widget: S for someone with QML experience, M for someone new
  • Control panel: it looks like only static content, and there's maybe some 3D interfacing? I'd say M
  • Playback widget: S: if they're just publishing ign-msgs, we could port our playback widget from ign-gazebo to ign-gui where it can be more generally used"
@scpeters
Copy link
Contributor Author

scpeters commented Dec 10, 2020

I have been working on enabling multiple versions of the visualizer in side-by-side executables that use different versions of ign-gui to simplify the migration process.

Adding the side-by-side visualizer executables with ign-gui0 / ign-gui2 is in another branch: delphyne-gui branch scpeters/visualizers_side_by_side

I'll work on making pull requests for this and then make a first step of the migration process in order to estimate the scope of the whole task.

@scpeters
Copy link
Contributor Author

I've started experimenting with configuring the Scene3D plugin from ign-gui2, which looks for a service name in its xml configuration, and will call that service to get scene information. I've added some configuration in 089c4ea, using the service name /get_scene used by delphyne, but that service uses different datatypes, so it won't be completely plug-and-play, but should still be doable.

@scpeters
Copy link
Contributor Author

scpeters commented Feb 4, 2021

I've updated the datatypes used by /get_scene in maliput/delphyne#746 and #349 and have managed to use the Scene3d plugin to display an untextured Prius model in 67f66e0

visualizer2_with_car

Next steps:

  • fix the texturing of the Prius model
  • fix rendering of the road mesh
  • connect pose updates to the interface expected by the Scene3d plugin

@scpeters
Copy link
Contributor Author

scpeters commented Feb 4, 2021

  • fix rendering of the road mesh

I see the following console messages related to the road mesh:

[visualizer] [GUI] [Err] [MeshManager.cc:119] Invalid mesh filename extension[/tmp/SShapeRoad.obj?culling=off]
[visualizer] [GUI] [Err] [MeshDescriptor.cc:56] Mesh manager can't find mesh named [/tmp/SShapeRoad.obj?culling=off]
[visualizer] [GUI] [Err] [OgreMeshFactory.cc:379] Cannot load null mesh
[visualizer] [GUI] [Err] [Scene3D.cc:638] Failed to load geometry for visual: 

I think the ?culling=off suffix may be the culprit. delphyne_gui's renderin_widget is using delphyne::utility::PackageManager to resolve the filename and there is separate logic for interpreting the ?culling=off suffix.

  • connect pose updates to the interface expected by the Scene3d plugin
[visualizer] [GUI] [Wrn] [Scene3D.cc:439] The pose topic, set via <pose_topic>, for the Scene3D plugin is missing or empty. Please set this topic so that the Scene3D can receive and process pose information.

delphyne publishes ignition::msgs::Model_V on the /visualizer/scene_update topic with integer id's for models and link names. Scene3d expects a topic publishing ignition::msgs::Pose_V with integer id's for all entities.

@scpeters
Copy link
Contributor Author

scpeters commented Feb 4, 2021

delphyne publishes ignition::msgs::Model_V on the /visualizer/scene_update topic with integer id's for models and link names. Scene3d expects a topic publishing ignition::msgs::Pose_V with integer id's for all entities.

The Model_V messages are populated by LcmViewerDrawToIgnModelV by translating from lcmt_viewer_draw.lcm messages, which have integer model id's and string name types. In order to supply unique integers for each entity to Scene3D, I think we could make an intermediate system that connects to the output of LcmViewerDrawToIgnModelV and inserts unique id's to the links (and visuals?).

@agalbachicar
Copy link
Collaborator

I think the ?culling=off suffix may be the culprit. delphyne_gui's renderin_widget is using delphyne::utility::PackageManager to resolve the filename and there is separate logic for interpreting the ?culling=off suffix.

Indeed. Removing that from the mesh URI in delphyne makes it to load with the same blue background as the prius.

image

@scpeters
Copy link
Contributor Author

by looking in ~/.ignition/rendering/ogre.log, I can confirm that the new visualizer is correctly finding car_texture.jpg. I wonder if we need to add some light to the scene?

@agalbachicar
Copy link
Collaborator

by looking in ~/.ignition/rendering/ogre.log, I can confirm that the new visualizer is correctly finding car_texture.jpg. I wonder if we need to add some light to the scene?

#359 improves the visualization pairing the ambient light, camera pose and other scene attributes but does not solve the material of the meshes.

@agalbachicar
Copy link
Collaborator

by looking in ~/.ignition/rendering/ogre.log, I can confirm that the new visualizer is correctly finding car_texture.jpg. I wonder if we need to add some light to the scene?

The old visualizer tells the same about the COLOR attribute so I guess the ogre log belongs to the old visualizer instead to the new one. I think we are missing something yet about the material.

@agalbachicar
Copy link
Collaborator

About simulation control buttons (play/pause buttons): I could not notice any topic/service being sent from the buttons. However, I noticed that in our install space inside the workspace there are no plugin libraries (inside ignition-gui0 folder). Instead, the new visualizer is taking the system installed ones in /usr/lib/x86_64-linux-gnu/ign-gui-2/plugins/.

@scpeters
Copy link
Contributor Author

scpeters commented Mar 2, 2021

About simulation control buttons (play/pause buttons): I could not notice any topic/service being sent from the buttons. However, I noticed that in our install space inside the workspace there are no plugin libraries (inside ignition-gui0 folder). Instead, the new visualizer is taking the system installed ones in /usr/lib/x86_64-linux-gnu/ign-gui-2/plugins/.

I just noticed that the space bar works to play and pause, but I'm not able to click on the buttons with the mouse

@scpeters
Copy link
Contributor Author

there are 3 improvements that make the rendering look much better:

to use the fix for road textures in ign-gui2, we will need to build ign-gui2 from source, since that version is EOL and I don't think we can get another release made. I would propose adding ign-gui2 to the workspace until we are ready to migrate forward to newer versions of these packages

@scpeters
Copy link
Contributor Author

this is what it looks like with those 3 fixes:

Screenshot from 2021-03-11 16-56-40

@scpeters
Copy link
Contributor Author

this has been merged to ign-gui3 and is being merged forward to ign-gui4 in gazebosim/gz-gui#192

@scpeters
Copy link
Contributor Author

this has been merged to ign-gui3 and is being merged forward to ign-gui4 in ignitionrobotics/ign-gui#192

the fix has just been released in ignition-gui3 3.5.1; debs are building now

@scpeters
Copy link
Contributor Author

I just opened maliput/delphyne#768, #360, and maliput/maliput_infrastructure#178 to update the workspace to use dependencies from the ignition-citadel collection, since the ignition-blueprint versions we are using are already EOL

@agalbachicar
Copy link
Collaborator

agalbachicar commented Mar 23, 2021

New work allocation:

@francocipollone
Copy link
Collaborator

francocipollone commented Mar 25, 2021

[TopicViewer plugin]

TopicViewer plugin isn't working because

file::/TopicViewer/TopicViewer.qml:17:1: module "QtQml.Models" is not installed

Link to file

So I proceeded to install qml-module-qtqml-models2 package via ubuntu package manager and it worked.

image

I've verified it and sadly it isn't available via rosdep.

However it is odd because this dependency is part of the dependencies of ign-gui3: Check packages.apt

@agalbachicar
Copy link
Collaborator

Update on Scene Tree plugin

  • It was formerly named TopicInspector in ign-gui0 but in delphyne-gui we renamed it to Scene tree.
  • A considerable amount of code in ign-gui0 was dedicated to that plugin in particular because it allows you to dynamically visualize almost any type of message. Specific widgets are required for that.
  • New versions of ign-gui deprecated that code. @scpeters found component_inspector in ign-gazebo which does more or less the same.
  • I'm currently in the process of migrating that code and evaluating what can be removed from there. It has a considerable amount of cross-references to ign-gazebo internals for the specific types you can visualize. Code in igntion libraries is licensed under Apache 2, we should be fine with copying and modifying while stating changes.

@francocipollone
Copy link
Collaborator

francocipollone commented May 12, 2021

Once all the plugins are migrated (#332 (comment)):

Next steps:

Removal of the ign-gui0 package from workspace should be tackled after fixing the issues related to its behaviour(font size, crash, etc)

@francocipollone
Copy link
Collaborator

francocipollone commented May 28, 2021

  1. Verify the arguments parsing in the new visualizer with respect the old one.

There is only one argument that is missing with respect to the old visualizer. It is related to plugin-injection and it made sense for ign-gui0 but the API has changed so there is no direct migration to that, and kind of it doesn't make real sense for our applications at least for the moment.

https://github.com/ToyotaResearchInstitute/delphyne-gui/blob/f0aac49b1cd69c0f65f931bef2e5afc65e030dfc/delphyne_gui/visualizer/visualizer0.cc#L98-L100

I will continue with

  1. Point all demos in delphyne_demos to the new visualizer.

@stonier
Copy link
Collaborator

stonier commented Jun 3, 2021

@francocipollone - wondering when is a good time to close the loop on minor issues. Took it for a road test today and noticed quite a few things awry here and there and was going to spin up issues, but I think it's probably better to wait until you've got 4-6 above done and dusted.

@francocipollone
Copy link
Collaborator

wondering when is a good time to close the loop on minor issues.

delphyne_gui/ignition-gui related? I think it is a good time to do so.

Steps 4 to 6 are on hold until this issue is solved, so as not to remove the old visualizer before we have the new one fully functional.
However those left steps are quite well defined: In essence removing old gui plugins (based on ign-gui0) files (done in #423 ) and removing the ign-gui0 from the workspace.

@agalbachicar
Copy link
Collaborator

@francocipollone - wondering when is a good time to close the loop on minor issues. Took it for a road test today and noticed quite a few things awry here and there and was going to spin up issues, but I think it's probably better to wait until you've got 4-6 above done and dusted.

@stonier please go ahead and add them!

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

No branches or pull requests

4 participants