Fix zoom centers and coordinate conversions

src/core/control/zoom/ZoomControl.cpp

@@ -32,9 +32,8 @@ auto onScrolledwindowMainScrollEvent(GtkWidget* widget, GdkEventScroll* event, Z
                 (event->direction == GDK_SCROLL_UP || (event->direction == GDK_SCROLL_SMOOTH && event->delta_y < 0)) ?
                         ZOOM_IN :
                         ZOOM_OUT;
-        // use screen pixel coordinates for the zoom center
-        // as relative coordinates depend on the changing zoom level
-        zoom->zoomScroll(direction, Util::toScreenCoords(widget, utl::Point{event->x_root, event->y_root}));
+        // translate absolute window coordinates to the widget-local coordinates and start zooming
+        zoom->zoomScroll(direction, Util::toWidgetCoords(widget, utl::Point{event->x_root, event->y_root}));
         return true;
     }
 
@@ -44,15 +43,13 @@ auto onScrolledwindowMainScrollEvent(GtkWidget* widget, GdkEventScroll* event, Z
 
 auto onTouchpadPinchEvent(GtkWidget* widget, GdkEventTouchpadPinch* event, ZoomControl* zoom) -> bool {
     if (event->type == GDK_TOUCHPAD_PINCH && event->n_fingers == 2) {
-        utl::Point<double> center;
         switch (event->phase) {
             case GDK_TOUCHPAD_GESTURE_PHASE_BEGIN:
                 if (zoom->isZoomFitMode()) {
                     zoom->setZoomFitMode(false);
                 }
-                // use screen pixel coordinates for the zoom center
-                // as relative coordinates depend on the changing zoom level
-                zoom->startZoomSequence(Util::toScreenCoords(widget, utl::Point{event->x_root, event->y_root}));
+                // translate absolute window coordinates to the widget-local coordinates and start zooming
+                zoom->startZoomSequence(Util::toWidgetCoords(widget, utl::Point{event->x_root, event->y_root}));
                 break;
             case GDK_TOUCHPAD_GESTURE_PHASE_UPDATE:
                 zoom->zoomSequenceChange(event->scale, true);
@@ -146,7 +143,7 @@ void ZoomControl::startZoomSequence() {
 
 void ZoomControl::startZoomSequence(utl::Point<double> zoomCenter) {
     // * set zoom center and zoom startlevel
-    this->zoomWidgetPos = zoomCenter;  // window space coordinates of the zoomCenter!
+    this->zoomWidgetPos = zoomCenter;  // widget space coordinates of the zoomCenter!
     this->zoomSequenceStart = this->zoom;
 
     // * set unscaledPixels padding value
@@ -174,7 +171,7 @@ void ZoomControl::startZoomSequence(utl::Point<double> zoomCenter) {
 
 void ZoomControl::zoomSequenceChange(double zoom, bool relative) {
     if (relative) {
-        if (this->zoomSequenceStart != -1) {
+        if (isZoomSequenceActive()) {
             zoom *= zoomSequenceStart;
         } else {
             zoom *= this->zoom;
@@ -186,7 +183,7 @@ void ZoomControl::zoomSequenceChange(double zoom, bool relative) {
 
 void ZoomControl::zoomSequenceChange(double zoom, bool relative, utl::Point<double> scrollVector) {
     if (relative) {
-        if (this->zoomSequenceStart != -1) {
+        if (isZoomSequenceActive()) {
             zoom *= zoomSequenceStart;
         } else {
             zoom *= this->zoom;
@@ -205,12 +202,14 @@ void ZoomControl::endZoomSequence() {
 }
 
 void ZoomControl::cancelZoomSequence() {
-    if (zoomSequenceStart != -1) {
+    if (isZoomSequenceActive()) {
         setZoom(zoomSequenceStart);
         endZoomSequence();
     }
 }
 
+auto ZoomControl::isZoomSequenceActive() const -> bool { return zoomSequenceStart != -1; }
+
 auto ZoomControl::getVisibleRect() -> Rectangle<double> {
     GtkWidget* widget = view->getWidget();
     Layout* layout = gtk_xournal_get_layout(widget);
@@ -222,8 +221,9 @@ auto ZoomControl::getScrollPositionAfterZoom() const -> utl::Point<double> {
     // can't be used to determine the scroll position! Return now.
     // NOTE: this case should never happen currently.
     //       getScrollPositionAfterZoom is called from XournalView after setZoom() fired the ZoomListeners
-    if (this->zoomSequenceStart == -1) {
-        return {-1, -1};
+    if (!this->isZoomSequenceActive()) {
+        assert(false && "ZoomControl::getScrollPositionAfterZoom() was called outside of a zoom sequence. ");
+        return {0, 0};
     }
 
     return this->scrollPosition * this->zoom - this->zoomWidgetPos + this->unscaledPixels;

src/core/control/zoom/ZoomControl.h

@@ -125,9 +125,10 @@ class ZoomControl: public DocumentListener {
     /**
      * Call this before any zoom is done, it saves the current page and position
      *
-     * @param zoomCenter position of zoom focus in window coordinate space
+     * @param zoomCenter position of zoom focus in widget pixel coordinates. That
+     * is, absolute coordinates (not scaling with the document) translated to the
+     * top left corner of the drawing area.
      */
-
     void startZoomSequence(utl::Point<double> zoomCenter);
 
     /**
@@ -160,10 +161,13 @@ class ZoomControl: public DocumentListener {
     /// Revert and end the current zoom sequence
     void cancelZoomSequence();
 
-    /// Update the scroll position manually
-    void setScrollPositionAfterZoom(utl::Point<double> scrollPos);
+    /// Check if we are between calls to `startZoomSequence()` and `endZoomSequence()`
+    bool isZoomSequenceActive() const;
 
-    /// Zoom to correct position on zooming
+    /**
+     * Zoom to correct position on zooming.
+     * This function should only be called during a zoom sequence.
+     */
     utl::Point<double> getScrollPositionAfterZoom() const;
 
     /// Get visible rect on xournal view, for Zoom Gesture
@@ -218,7 +222,7 @@ class ZoomControl: public DocumentListener {
     /// Base zoom on start, for relative zoom (Gesture)
     double zoomSequenceStart = -1;
 
-    /// Zoom center position in window coordinate space, will not be zoomed!
+    /// Zoom center position in widget coordinate space, will not be zoomed!
     utl::Point<double> zoomWidgetPos;
 
     /// Scroll position (top left corner of view) to scale

src/core/gui/MainWindow.cpp

@@ -292,8 +292,8 @@ void MainWindow::initHideMenu() {
 
 auto MainWindow::getLayout() const -> Layout* { return gtk_xournal_get_layout(this->xournal->getWidget()); }
 
-auto MainWindow::getScreenPos() -> utl::Point<double> {
-    return Util::toScreenCoords(this->winXournal, utl::Point{0.0, 0.0});
+auto MainWindow::getNegativeXournalWidgetPos() const -> utl::Point<double> {
+    return Util::toWidgetCoords(this->winXournal, utl::Point{0.0, 0.0});
 }
 
 auto cancellable_cancel(GCancellable* cancel) -> bool {

src/core/gui/MainWindow.h

@@ -116,10 +116,12 @@ class MainWindow: public GladeGui, public LayerCtrlListener {
     [[maybe_unused]] Menubar* getMenubar() const;
 
     /**
-     * Get the position of the top left corner of this window on the screen.
-     * @see gdk_window_get_root_origin
+     * Get the position of the top left corner of screen (X11) or the window (Wayland)
+     * relative to the Xournal Widget top left corner
+     *
+     * @see Util::toWidgetCoords()
      */
-    utl::Point<double> getScreenPos();
+    utl::Point<double> getNegativeXournalWidgetPos() const;
 
     /**
      * Disable kinetic scrolling if there is a touchscreen device that was manually mapped to another enabled input

src/core/gui/XournalView.cpp

@@ -541,12 +541,10 @@ void XournalView::zoomChanged() {
 
     if (zoom->isZoomPresentationMode() || zoom->isZoomFitMode()) {
         scrollTo(currentPage);
-    } else {
+    } else if (zoom->isZoomSequenceActive()) {
         auto pos = zoom->getScrollPositionAfterZoom();
-        if (pos.x != -1 && pos.y != -1) {
-            Layout* layout = gtk_xournal_get_layout(this->widget);
-            layout->scrollAbs(pos.x, pos.y);
-        }
+        Layout* layout = gtk_xournal_get_layout(this->widget);
+        layout->scrollAbs(pos.x, pos.y);
     }
 
     Document* doc = control->getDocument();

src/core/gui/inputdevices/TouchInputHandler.cpp

@@ -47,13 +47,13 @@ auto TouchInputHandler::handleImpl(InputEvent const& event) -> bool {
             // Set sequence data
             sequenceStart(event);
 
-            startZoomReady = true;
+            this->startZoomReady = true;
         }
     }
 
     if (event.type == MOTION_EVENT && this->primarySequence) {
         if (this->primarySequence && this->secondarySequence && zoomGesturesEnabled) {
-            if (startZoomReady) {
+            if (this->startZoomReady) {
                 if (this->primarySequence == event.sequence) {
                     sequenceStart(event);
                     zoomStart();
@@ -121,8 +121,6 @@ void TouchInputHandler::scrollMotion(InputEvent const& event) {
 }
 
 void TouchInputHandler::zoomStart() {
-    auto center = (this->priLastRel + this->secLastRel) / 2.0;
-
     this->startZoomDistance = this->priLastAbs.distance(this->secLastAbs);
 
     if (this->startZoomDistance == 0.0) {
@@ -133,8 +131,6 @@ void TouchInputHandler::zoomStart() {
     // hasn't changed enough).
     this->canBlockZoom = true;
 
-    lastZoomScrollCenter = (this->priLastAbs + this->secLastAbs) / 2.0;
-
     ZoomControl* zoomControl = this->inputContext->getView()->getControl()->getZoomControl();
 
     // Disable zoom fit as we are zooming currently
@@ -143,16 +139,19 @@ void TouchInputHandler::zoomStart() {
         zoomControl->setZoomFitMode(false);
     }
 
-    auto* mainWindow = inputContext->getView()->getControl()->getWindow();
     // use screen pixel coordinates for the zoom center
     // as relative coordinates depend on the changing zoom level
-    utl::Point<double> windowTopLeft = mainWindow->getScreenPos();
-    center.x -= windowTopLeft.x;
-    center.y -= windowTopLeft.y;
+    auto center = (this->priLastAbs + this->secLastAbs) / 2.0;
+    this->lastZoomScrollCenter = center;
+
+    // translate absolute window coordinates to the widget-local coordinates
+    const auto* mainWindow = inputContext->getView()->getControl()->getWindow();
+    const auto translation = mainWindow->getNegativeXournalWidgetPos();
+    center += translation;
 
     zoomControl->startZoomSequence(center);
 
-    startZoomReady = false;
+    this->startZoomReady = false;
 }
 
 void TouchInputHandler::zoomMotion(InputEvent const& event) {
@@ -178,7 +177,7 @@ void TouchInputHandler::zoomMotion(InputEvent const& event) {
     }
 
     ZoomControl* zoomControl = this->inputContext->getView()->getControl()->getZoomControl();
-    auto center = (this->priLastAbs + this->secLastAbs) / 2;
+    const auto center = (this->priLastAbs + this->secLastAbs) / 2;
     zoomControl->zoomSequenceChange(zoom, true, center - lastZoomScrollCenter);
     lastZoomScrollCenter = center;
 }

src/util/Util.cpp

@@ -48,12 +48,14 @@ auto Util::paintBackgroundWhite(GtkWidget* widget, cairo_t* cr, void*) -> gboole
     return false;
 }
 
-utl::Point<double> Util::toScreenCoords(GtkWidget* widget, utl::Point<double> point) {
-    // use screen pixel coordinates for the zoom center
-    // as relative coordinates depend on the changing zoom level
+utl::Point<double> Util::toWidgetCoords(GtkWidget* widget, utl::Point<double> absolute_coords) {
     int rx, ry;
-    gdk_window_get_root_origin(gtk_widget_get_window(widget), &rx, &ry);
-    return utl::Point<double>{point.x - rx, point.y - ry};
+    // X11 uses absolute screen coordinates while Wayland uses absolute window coordinates.
+    // Converting them to widget-local coordinates will cancel out this difference.
+    // `gtk_widget_get_window` doesn't return the actual window, but the local widget-window.
+    // GTK4 renames `gdk_window_get_root_coords()` to `gdk_surface_get_root_coords()`
+    gdk_window_get_root_coords(gtk_widget_get_window(widget), 0, 0, &rx, &ry);
+    return utl::Point<double>{absolute_coords.x - rx, absolute_coords.y - ry};
 }
 
 void Util::cairo_set_dash_from_vector(cairo_t* cr, const std::vector<double>& dashes, double offset) {

src/util/include/util/Util.h

@@ -69,7 +69,11 @@ gboolean paintBackgroundWhite(GtkWidget* widget, cairo_t* cr, void* unused);
 
 void cairo_set_dash_from_vector(cairo_t* cr, const std::vector<double>& dashes, double offset);
 
-utl::Point<double> toScreenCoords(GtkWidget* widget, utl::Point<double> point);
+/**
+ * Transform absolute coordinates into coordinates local to the specified widget.
+ * The top left corner of `widget` will have coordinates (0, 0).
+ */
+utl::Point<double> toWidgetCoords(GtkWidget* widget, utl::Point<double> absolute_coords);
 
 /**
  * Format coordinates to use 8 digits of precision https://m.xkcd.com/2170/