Send more events from Scene3D

[chapulina]

Desired behavior

Since https://github.com/ignitionrobotics/ign-gui/issues/68, any plugin can safely interact with the 3D scene during the render event. We should expose more events from the rendering scene, so plugins can react to clicks, hover, etc.

Ignition Gazebo's GzScene3D plugin already broadcasts right/left clicks and hover events using Ignition GUI events. We just need to add that functionality to ign-gui's Scene3D.

Alternatives considered

One alternative that users have now is forking Scene3D and embedding functionality into it, but that doesn't scale well and doesn't allow multiple plugins from various developers to play well together.

Implementation suggestion

We can do exactly the same as the ign-gazebo implementation.

Additional context

This should help with https://github.com/ignitionrobotics/ign-rviz/issues/70.

[francocipollone]

Hi! Sorry for the intrusion. We are using ign-gui3 in our project and we faced the issue of not being able to hook to a Scene3D mouse event from other plugins, which is partially related to the complete scope of this ticket.

I came here to fill a ticket to suggest and provide a way to hook to a mouse press event but given this issue is already created I found it redundant to create one on my own, although this ticket is way more general and covers several events.

Basically, I've forked from the last commit and modified Scene3D plugin to emit a signal when a mouse press event takes place. Only 10 lines are modified. --> commit: Emit signal when mouse press event occurs

If you consider it properly and worth it I would be happy to open a PR with these changes.

Thanks in advance.

[chapulina]

Hi! Sorry for the intrusion.

Not at all, this issue is up for grabs!

If you consider it properly and worth it I would be happy to open a PR with these changes.

Cool, that's a clean implementation. Are you able to listen to that signal from other plugins though?

The pattern we've been using is to emit events to the main window, and other plugins can filter events coming from the main window. I started porting events from Ignition Gazebo on https://github.com/ignitionrobotics/ign-gui/pull/213, but I'm currently stuck trying to write tests.

@francocipollone , does that pattern work for you? I think your implementation is cleaner, and if there's an easy way for plugins that don't know about each other to listen to each other's events using that pattern I'm all ears.

If that addresses your needs, I can try to put more time into writing tests, or we can merge it without tests for now.

[francocipollone]

Thanks for the quick response!

Cool, that's a clean implementation. Are you able to listen to that signal from other plugins though?

Yes, sure. I tried to connect to that signal from another plugin indeed. I am connecting to signal doing something like this:

  Plugin* scene3D{nullptr};
  QObject* parent = this->parent();
  scene3D = parent->findChild<Plugin *> ("Scene3D");

  auto renderWindowItem = scene3D->PluginItem()->findChild<QQuickItem *>();
  QObject::connect(renderWindowItem, SIGNAL(MousePressEventSignal(ignition::common::MouseEvent)), this, SLOT(MousePressEventHandler(ignition::common::MouseEvent)));

Being MousePressEventHandler the function SLOT being hook to the signal. I did that this slot function prints the 2D coordinates of the click that holds the ignition::common::MouseEvent and it is working correctly.

The pattern we've been using is to emit events to the main window, and other plugins can filter events coming from the main window. I started porting events from Ignition Gazebo on #213, but I'm currently stuck trying to write tests.

Oh, I see that probably then my proposal doesn't follow the approach you've already started, but it is great that it is being addressed already :D

@francocipollone , does that pattern work for you? I think your implementation is cleaner, and if there's an easy way for plugins that don't know about each other to listen to each other's events using that pattern I'm all ears. If that addresses your needs, I can try to put more time into writing tests, or we can merge it without tests for now.

That's great! This would work for us and moving forward on this front would be great. :D

[chapulina]

Thank you for the code snippet! That's a nice pattern, it just requires that the other plugin hardcodes the name of the scene object, which may be fine in most use cases.

This would work for us

Cool, I'll see if I can unblock that PR in the coming days then :+1: Let me know if you see any issues using it.

[francocipollone]

Sorry, I think I misunderstood you.

does that pattern work for you? I think your implementation is cleaner, and if there's an easy way for plugins that don't know about each other to listen to each other's events using that pattern I'm all ears. If that addresses your needs, I can try to put more time into writing tests, or we can merge it without tests for now.

You were asking about the pattern implemented on #213, right? I was answering to my pattern, not yours. Sorry for the confusion.

I will do the next thing. I will try the changes on #213 on my code and see if that patterns effectively work for us. However, it shouldn't be a problem. I'll let you know :+1:

[scpeters]

I am working with @francocipollone on this plugin and have just one question: if a plugin is only interested in MouseEvents from the Scene3D plugin, wouldn't it be cleaner to get them directly from that plugin than from the main window? If there are multiple plugins publishing mouse events it could be tricky to distinguish between them. Seems like an advantage of using a signal from Scene3D if that is precisely what you're interested in, instead of filtering the stream of main window events.

I don't have much experience with ign-gui though, so please set me straight if that doesn't make sense!

[chapulina]

if a plugin is only interested in MouseEvents from the Scene3D plugin, wouldn't it be cleaner to get them directly from that plugin than from the main window?

If you know ahead of time that this is the only plugin you may be interacting with, this is probably the most straightforward approach.

The approach we've been taking assumes there's no authority on who's sending an event, so we can easily swap plugins under the hood and maintain the behaviour. It's much like how you subscribe to a topic regardless of which node is publishing it.

If your plugin is listening to any mouse event, you can use it with ignition::gazebo::Scene3D or ignition::gui::Scene3D interchangeably. We could get a similar effect by giving those 2 plugins the same object name, but object names can easily collide, so we need to be careful.

The distributed approach is more important for events like selecting an entity, because that really has no central authority and could come from the 3D scene, the entity tree, etc. We've been using the same approach for all use cases for consistency, but we could consider a more direct approach for cases where there's a clear authority. One concern would be making users confused and unsure of what approach to take.

[scpeters]

ok, now that I look at it more closely, I think the easiest thing in our case would be to subscribe to the LeftClickToScene events added by #213, which are more specialized than generic mouse events. Let me know if there's anything I can do to aid with #213

[chapulina]

Let me know if there's anything I can do to aid with #213

I'm honestly stuck writing the tests. The Ogre plugin refuses to be loaded. If you have any cycles to look into that I'd appreciate it. If not, I'm also ok merging that without tests for now to unblock you (that's what we did on #170 too).

[francocipollone]

Some updates

1 - I've taken a look at the code and it would definitely work for us.

2 - After checking your code I realized that the first comment I made about this issue wasn't completely right:

... we faced the issue of not being able to hook to a Scene3D mouse event from other plugins...

There was one thing that I could have done to hook to the mouse event. I could have overriden the eventFilter method and then look for the correspondent event I need.

Translating this to code: In my custom plugin's LoadConfig method I could do:

 ...
  // Install event filter to get mouse event on the scene.
  QList<Plugin*> plugins = parent()->findChildren<Plugin*>();
  Plugin* scene3D{nullptr};
  for (auto& plugin : plugins) {
    if (std::string(plugin->metaObject()->className()).find("Scene3D") != std::string::npos) {
      scene3D = plugin;
      break;
    }
  }
  if (!scene3D) {
    ignerr << "Scene3D plugin wasn't found." << std::endl;
  }
  auto renderWindowItem = scene3D->PluginItem()->findChild<QQuickItem*>();
  renderWindowItem->installEventFilter(this);
 ....

And override the eventFilter method:

bool eventFilter(QObject* _obj, QEvent* _event) override {
  if (_event->type() == QEvent::Type::MouseButtonPress) {
    QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(_event);
    if (mouseEvent) {
      std::cout << "\t(x , y) : ( " << mouseEvent->x() << " , " << mouseEvent->y() << " )" << std::endl;
    }
  }
  // Standard event processing
  return QObject::eventFilter(_obj, _event);
}

I tried it locally and I was able to get the events and print the coordinates.

Note: Sorry I'd not noticed this before, I don't have much experience using qt.

[chapulina]

I could have overriden the eventFilter method and then look for the correspondent event I need.

Yeah Qt is pretty flexible in that there's always a way for any widget to access pretty much anything on the entire application. A bit dangerous sometimes, but very powerful.

That approach also uses the object name to find it. If we're very diligent about how we name objects, it can be ok. A more robust strategy would use findChild<ignition::gui::plugins::Scene3D>(), but that forces us to link against another plugin :confused:

[francocipollone]

Given that we are now able to hook to the MouseEvents that take place in the scene, this isn't blocking us anymore.

So if #213 isn't merged right away is ok for us. Later on, when the PR is merged probably we will adapt our code to the new features but at the moment it isn't a priority for us.

Having said that, thank you @chapulina a lot for your help! :+1:

[chapulina]

Cool, glad to hear you're not blocked. How did you manage to get the scene's events without setting its object name?

[francocipollone]

Cool, glad to hear you're not blocked. How did you manage to get the scene's events without setting its object name?

I filter the available Scene3D plugins (if there were more than one) in the application by checking the title of the plugin

For instance we are loading the configuration of the layout by using a .config file. So when adding the plugin to the .config I set a title:

<plugin filename="Scene3D">
  <ignition-gui>
    <title>Main3DScene</title>
    ...
  </ignition-gui>
 ...
</plugin>

And then I could look for that Scene3D plugin with that title in particular.

Something like:

  QList<Plugin*> plugins = parent()->findChildren<Plugin*>();
  Plugin* scene3D{nullptr};
  for (auto& plugin : plugins) {
    if (plugin->Title() == "Main3DScene") {
      scene3D = plugin;
      break;
    }
  }
  auto renderWindowItem = scene3D->PluginItem()->findChild<QQuickItem*>();
  renderWindowItem->installEventFilter(this);

Feel free to see this PR (and comment if you want) I think you have access.

[scpeters]

213 has now been merged, but after looking at @francocipollone's implementation, I realized that the *ClickToScene event structs provide a Vector3d, but don't communicate whether the RayQuery hit an object and that point is on the object or if the RayQuery missed and the point is some distance along the ray. @francocipollone's implementation makes the RayQuery directly and makes use of this information.

I would propose adding an additional double distance or bool objectClicked field to the *ClickToScene and HoverToScene events. Adding these fields will break ABI, so this would be targeted to Fortress

[scpeters]

I would propose adding an additional double distance or bool objectClicked field

we can store a double distance but provide double Distance() and operator bool() accessors

[scpeters]

I would propose adding an additional double distance or bool objectClicked field

we can store a double distance but provide double Distance() and operator bool() accessors

we could also return the intersected objectId...

[scpeters]

I just started testing ignition-gui3 3.6.0 with the maliput visualizer, and I'm getting a lot of seg-faults from BroadcastHoverPos just by moving the mouse over the screen. It happens quite frequently. Here's an example backtrace:

Thread 21 "ignition::gui::" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff7fbff700 (LWP 19909)]
0x00007fffcc4717f0 in Ogre::Math::calculateBasicFaceNormalWithoutNormalize(Ogre::Vector3 const&, Ogre::Vector3 const&, Ogre::Vector3 const&) () from /usr/lib/x86_64-linux-gnu/libOgreMain.so.1.9.0
(gdb) bt
#0  0x00007fffcc4717f0 in Ogre::Math::calculateBasicFaceNormalWithoutNormalize(Ogre::Vector3 const&, Ogre::Vector3 const&, Ogre::Vector3 const&) () from /usr/lib/x86_64-linux-gnu/libOgreMain.so.1.9.0
#1  0x00007fffcc4718c1 in Ogre::Math::intersects(Ogre::Ray const&, Ogre::Vector3 const&, Ogre::Vector3 const&, Ogre::Vector3 const&, bool, bool) () from /usr/lib/x86_64-linux-gnu/libOgreMain.so.1.9.0
#2  0x00007fffccb09a18 in ignition::rendering::v3::OgreRayQuery::ClosestPoint() ()
   from /usr/lib/x86_64-linux-gnu/ign-rendering-3/engine-plugins/libignition-rendering-ogre.so
#3  0x00007fffd3dcba54 in ignition::gui::plugins::IgnRenderer::ScreenToScene(ignition::math::v6::Vector2<int> const&) const () from /usr/lib/x86_64-linux-gnu/ign-gui-3/plugins/libScene3D.so
#4  0x00007fffd3dcbbe9 in ignition::gui::plugins::IgnRenderer::BroadcastHoverPos() ()
   from /usr/lib/x86_64-linux-gnu/ign-gui-3/plugins/libScene3D.so
#5  0x00007fffd3dd4d12 in ignition::gui::plugins::IgnRenderer::HandleMouseEvent() ()
   from /usr/lib/x86_64-linux-gnu/ign-gui-3/plugins/libScene3D.so
#6  0x00007fffd3dd4e43 in ignition::gui::plugins::IgnRenderer::Render() ()
   from /usr/lib/x86_64-linux-gnu/ign-gui-3/plugins/libScene3D.so
#7  0x00007fffd3dd581b in ignition::gui::plugins::RenderThread::RenderNext() ()
   from /usr/lib/x86_64-linux-gnu/ign-gui-3/plugins/libScene3D.so
#8  0x00007ffff6b110c2 in QObject::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#9  0x00007ffff6ae176a in QCoreApplication::notify(QObject*, QEvent*) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#10 0x00007ffff6ae18d8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#11 0x00007ffff6ae404d in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#12 0x00007ffff6b3b263 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#13 0x00007ffff1f4b537 in g_main_context_dispatch () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#14 0x00007ffff1f4b770 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x00007ffff1f4b7fc in g_main_context_iteration () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#16 0x00007ffff6b3a88f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#17 0x00007ffff6adf90a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#18 0x00007ffff68fe23a in QThread::exec() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#19 0x00007ffff690317d in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#20 0x00007ffff3ba26db in start_thread (arg=0x7fff7fbff700) at pthread_create.c:463
#21 0x00007ffff5d7e71f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb)

[agalbachicar]

@scpeters I've tried to create a unit tests that replicates the error without luck. I've used the same QTest API that is already implemented in the tests of Scene3d to check the events but no segfault happened.

Also, and most interestingly, this is error is nor reproducible in focal.

For the record, I've used ignition-gui3 from an docker container with nvidia-docker2 and started from nvidia/opengl:1.0-glvnd-runtime-ubuntu20.04 and nvidia/opengl:1.0-glvnd-runtime-ubuntu18.04. This error was not reproducible with an Intel graphics card in bionic.

[chapulina]

MinimalScene was released into Fortress emitting lots of events. I'll close this issue; please ticket new issues if new specific events are needed.