Proposal to change double click event vtk mapping

Hi all,

for 3DSlicer we use different mapping for double clicks in the QVTKInteractor: we differentiate single and double click events, i.e

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).

Thanks,

Davide.

MR: https://gitlab.kitware.com/vtk/vtk/-/merge_requests/7193
for reference: https://github.com/Slicer/Slicer/pull/5141#discussion_r479690583

3 Likes

To follow up this, unless there are objections we will shortly move forward with the integration of the corresponding merge request.

2 Likes

Corresponding topic has just been merged.

1 Like

Thanks!

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.

I have a merge request up now:

https://gitlab.kitware.com/vtk/vtk/-/merge_requests/7333

How does that compare to this?

@Davide_Punzo I tried to add you as a reviewer to the MR but couldn’t find your gitlab account (assuming you have one, which you may not).

not using the gitlab account tto much indeed :slightly_smiling_face: . It is this one: https://gitlab.kitware.com/Punzo

@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.

great, thanks!