gazebosim / gz-gui

Builds on top of Qt to provide widgets which are useful when developing robotics applications, such as a 3D view, plots, dashboard, etc, and can be used together in a convenient unified interface.
https://gazebosim.org
Apache License 2.0
72 stars 41 forks source link

Support using ign-gui QML components in fully custom downstream application #320

Open zflat opened 2 years ago

zflat commented 2 years ago

Desired behavior

I would like to incorporate ign-gui and also ign-rviz QML components in an existing Qt Quick application. The existing application sets up a QGuiApplication already and needs to do some different things than what ignition::gui::Aplication does for the following:

Implementation suggestion

In order to use ign-gui QML components in a downstream application, I would like to suggest a couple of changes.

  1. The method ignition::gui::App() returns a pointer of type ignition::gui::Application. Can an abstract base class be defined and then ignition::gui::App() returns the base class? Then Application would be changed to inherit from the base class.
  2. The Application class performs a lot of plugin loading logic. Could the majority of this logic be extracted to a helper class that could also be used downstream? Ideally, the Application class would then only be specific to setting up and interacting with ignition::gui::MainWindow and other "top level" things like signal handlers and logging.
    • Note that the method ignition::gui::Application::AddPluginsToWindow is specific to proably too specific to igntion::gui::MainWindow to be extracted, but maybe it could be broken down some with common functionality extracted.

Alternatives considered

I could subclass the ignition::gui::Application class as-is and override the constructor and other methods. I would have to work around the issues described here and I am not sure how feasible this solution is without testing it.

Additional context

N/A

chapulina commented 2 years ago

The method ignition::gui::App() returns a pointer of type ignition::gui::Application.

That's just a wrapper around qGuiApp:

https://github.com/ignitionrobotics/ign-gui/blob/f8af480c564cda1ace207378b7c526870dce468d/src/Application.cc#L173-L177

Could you use qGuiApp directly for your use case?

Can an abstract base class be defined and then ignition::gui::App() returns the base class?

Could you elaborate on what functionality would be on the new abstract class, and what would remain in gui::Application?

Could the majority of this logic be extracted to a helper class that could also be used downstream?

Yes, I think that's a good idea. While implementing it, we should try to be backwards-compatible as much as possible, so we don't break existing users. For example, Application::LoadPlugin could call the new PluginManager::LoadPlugin (or any other name it has).

zflat commented 2 years ago

Could you use qGuiApp directly for your use case?

The issue is with ignition::gui code that uses the App() method. For example, see ign-gui/src/Plugin.cc in the method PluginLoad

this->dataPtr->context = new QQmlContext(App()->Engine()->rootContext());

In order for plugin loading to work downstream, the qGuiApp pointer must be to an object that can be cast to an Application type. So it is not downstream code, but ignition::gui code that is using App().


Can an abstract base class be defined and then ignition::gui::App() returns the base class?

Could you elaborate on what functionality would be on the new abstract class, and what would remain in gui::Application?

I think the abstract class would have public functions for the public functions currently implemented in Application


Could the majority of this logic be extracted to a helper class that could also be used downstream?

Yes, I think that's a good idea. While implementing it, we should try to be backwards-compatible as much as possible, so we don't break existing users. For example, Application::LoadPlugin could call the new PluginManager::LoadPlugin (or any other name it has).