Send more events from Scene3D #209
Comments
chapulina
added
enhancement
|
Hi! Sorry for the intrusion. 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 If you consider it properly and worth it I would be happy to open a PR with these changes. Thanks in advance. |
Not at all, this issue is up for grabs!
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 #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. |
|
Thanks for the quick response!
Yes, sure. I tried to connect to that signal from another plugin indeed. 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
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
That's great! This would work for us and moving forward on this front would be great. :D |
|
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.
Cool, I'll see if I can unblock that PR in the coming days then 👍 Let me know if you see any issues using it. |
|
Sorry, I think I misunderstood you.
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 👍 |
|
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 I don't have much experience with ign-gui though, so please set me straight if that doesn't make sense! |
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 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. |
|
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:
There was one thing that I could have done to hook to the mouse event. Translating this to code: ...
// 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 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. |
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 |
|
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! 👍 |
|
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 For instance we are loading the configuration of the layout by using a .config file. <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. |
|
#213 has now been merged, but after looking at @francocipollone's implementation, I realized that the I would propose adding an additional |
we can store a |
we could also return the intersected |
|
I just started testing ignition-gui3 3.6.0 with the maliput visualizer, and I'm getting a lot of seg-faults from |
|
@scpeters I've tried to create a unit tests that replicates the error without luck. I've used the same 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 |
|
|
Desired behavior
Since #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
GzScene3Dplugin already broadcasts right/left clicks and hover events using Ignition GUI events. We just need to add that functionality toign-gui'sScene3D.Alternatives considered
One alternative that users have now is forking
Scene3Dand 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 gazebosim/gz-rviz#70.
The text was updated successfully, but these errors were encountered: