Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-88252

QEventPoint::velocity must be accurate and in logical pixels

    XMLWordPrintable

Details

    Description

      Velocity is useful so that Flickable can avoid calculating velocity for itself, among other reasons. In https://codereview.qt-project.org/c/qt/qtdeclarative/+/315938 Flickable begins to use it. And then I discovered an issue here in QGuiApplicationPrivate::processMouseEvent():

      #ifndef QT_NO_CURSOR
          if (!e->synthetic()) {
              if (const QScreen *screen = window->screen())
                  if (QPlatformCursor *cursor = screen->handle()->cursor()) {
                      const QPointF nativeLocalPoint = QHighDpi::toNativePixels(localPoint, screen);
                      const QPointF nativeGlobalPoint = QHighDpi::toNativePixels(globalPoint, screen);
                      QMouseEvent ev(type, nativeLocalPoint, nativeLocalPoint, nativeGlobalPoint,
                                     button, e->buttons, e->modifiers, e->source, device);
                      ev.setTimestamp(e->timestamp); // triggers velocity calculation
                      cursor->pointerEvent(ev);
                  }
          }
      #endif
      

      Because velocity calculation is a side-effect of QEventPoint::setTimestamp(), and constructing any QMouseEvent automatically updates QEventPoint::globalLastPosition() in the persistent QEventPoint instance belonging to that mouse, it's a problem to construct a mouse event in another coordinate system like that. globalPosition (and globalLastPosition) end up jumping back and forth between logical and native coordinates, which makes the velocity wrong.

      As it happens, that code is only useful on embedded platforms: apparently it makes the mouse cursor follow the mouse position that the application sees. (It started with https://codereview.qt-project.org/c/qt/qtbase/+/2083 and https://codereview.qt-project.org/c/qt/qtbase/+/140034 updated it to use native coordinates.) So we could skip it on most platforms somehow (by checking a flag provided by the plugin, or something)... but that would also make this bug more obscure.

      So I'm fixing it like this for now: https://codereview.qt-project.org/c/qt/qtbase/+/320669 : restore the globalLastPosition after that "special" event is constructed, and make sure the native-coordinate event does not contribute to the Kalman filter either.

      But maybe this idea is a bit fragile; maybe QGuiApplication should drive the Kalman filter more directly, only with actual incoming events and only when it's sure the coordinates are logical coordinates. Or it could be somewhere else.

      It seems to need an autotest if we can write a reliable one, in any case.

      Attachments

        Issue Links

          For Gerrit Dashboard: QTBUG-88252
          # Subject Branch Project Status CR V

          Activity

            People

              srutledg Shawn Rutledge
              srutledg Shawn Rutledge
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:

                Gerrit Reviews

                  There are no open Gerrit changes