We would like to merge this PR in vtk upstream since @jcfr is updating the Slicer VTK branch.
We tested the PR around one year ago and at the time we didn’t see any issue with VTK widgets or views. However, we are concerned that the PR can break some applications. Please let us know if there would be any issue or if you would like to keep the previous mapping (single clicks and double clicks having the same event).
No objections, but this seems to have broken some other things in VTK and ParaView.
Specifically, vtkControlPointsItem::MouseDoubleClickEvent is no longer invoked because vtkContextScene::DoubleClickEvent is not invoked because vtkContextInteractorStyle::ProcessMousePress is asking vtkRenderWindowInteractor for its RepeatCount and never getting anything > 1.
@Davide_Punzo@jcfr , do you know what about the MR could have changed processing in VTK’s Rendering/Context2D ?
Hi @dcthomp, I could reproduce as well, thanks for reporting!
I have to say that I am not sure where the RepeatCount is increased to 1. It seems that it is required the invoke of single button press also in the double click ones, something like this:
else if (t == QEvent::MouseButtonDblClick)
{
switch (e2->button())
{
case Qt::LeftButton:
iren->InvokeEvent(vtkCommand::LeftButtonPressEvent, e2);
iren->InvokeEvent(vtkCommand::LeftButtonDoubleClickEvent, e2);
break;
case Qt::MidButton:
iren->InvokeEvent(vtkCommand::MiddleButtonPressEvent, e2);
iren->InvokeEvent(vtkCommand::MiddleButtonDoubleClickEvent, e2);
break;
case Qt::RightButton:
iren->InvokeEvent(vtkCommand::RightButtonPressEvent, e2);
iren->InvokeEvent(vtkCommand::RightButtonDoubleClickEvent, e2);
break;
default:
break;
}
}
of course, having two signals for the same event will not be optimal.
I’ll try to investigate asap.
@Davide_Punzo The change you suggest might be better than nothing, but it feels like it could also cause problems should an application be listening for single-click events. Without at least updating the RepeatCount to 1 before invoking vtkCommand::xxxButtonPressEvent, this seems problematic.
I’ve merged my change, which simply ensures that double-click events are passed to interactor styles and updates one subclass (vtkContextInteractorStyle) to pay attention to them.
@simonesneault I looked at vtkImagePlaneWidget but didn’t see RepeatCount in use anywhere in the file or even in Interaction/Widgets. What does double-click do for the image-plane widget?
The repeatCount is equal to 1 when the vtkCommand::xxxButtonPressEvent is fired in the if condition QEvent::MouseButtonDblClick in the temp solution (I really didn’t undestrand why and where it gets update to 1), so one can distinguish vtkCommand::xxxButtonPressEvent from single and double clicks.
But again, I agree with you and I don’t like the temp solution: it’s dirty and would confuse developers to have two double clicks events “systems”. I would prefer to have just vtkCommand::xxxButtonDoubleClickEvent at QEvent::MouseButtonDblClick events (current status) and remove the repeatCount ivar along VTK (by updating it where necessary). On the other hand, I don’t have time to address this properly now, and the temp solution may help with the transition.