Enabling touch navigation

[gg-rewrite]

Adds the touch navigation functionality as proposed in #138 .

• tap to left/right side to navigate to previous/next image
• tap into the middle to show overlay
• move single point to pan the image
• pinch in/out to zoom

[WhyNotHugo]

It doesn't seem to be working for me on Wayland (swaywm). Does "touch" imply "touchpad" or "touchscreen"? (might be best to clarify)

[gg-rewrite]

It doesn't seem to be working for me on Wayland (swaywm). Does "touch" imply "touchpad" or "touchscreen"? (might be best to clarify)

Touchscreen specifically, my apologies for ambiguity.

[WhyNotHugo]

No worries! Ignore my feedback then, I tried with a touchpad. 😅

[NDagestad]

I have tested this and it mostly seems to work but there are some kinda strange behaviour.
If you start one gesture and then transition to another one without taking both finger off the screen, the image will jump around.
So if zoom by pinching, lift a finger and start panning the image, it will end up at a seemingly unpredictable position in the viewport before starting to pan.

[gg-rewrite]

I have tested this and it mostly seems to work but there are some kinda strange behaviour.
If you start one gesture and then transition to another one without taking both finger off the screen, the image will jump around.
So if zoom by pinching, lift a finger and start panning the image, it will end up at a seemingly unpredictable position in the viewport before starting to pan.

Seems like incorrect pan distance tracking in this particular case. I'll fix that.

[NDagestad]

Any news on this ?

[gg-rewrite]

My apologies, other stuff got in the way. Still on it.

[NDagestad]

No worries, it's already quite usable 👍

[eXeC64]

Hey, thanks for the work you've put in for this. I'm sorry the review's taking so long. I've not got the right hardware to be able to test this though, so I'm reliant on volunteers to see that it works for them.

I've only given the code a light skim but I've noticed a few tabs have snuck in. Could you clean those up please, and I'll try and get a proper code review done soon.

Thanks again

[gg-rewrite]

Thaaat was one hell of a vacation leave, my apologies for that. Fixed the tabs, please check.

[eXeC64]

Will review this once I've put out a smaller bugfix release. Sorry about the delay.

[mlngh]

Hi,

I merged master into this branch locally and needed to change references to imv->overlay_enabled to imv->overlay.enabled before it would compile.

I can verify that the basic functionality works, but with two caveats:

• it seems that scrolling and zooming do not "stay with" your fingers as one would expect--there may be a missing/wrong scaling factor somewhere?
• when moving with one finger and then placing another to begin zooming, or vice versa (lifting one finger to move rather than zoom), the image view jumps significantly. This seems to be what @NDagestad described.

I'll see if I can find a solution to these issues.

[mlngh]

The issue with the image not "staying with" fingers (when panning) seems to be a missing multiplication of pan distances by the scale factor of the buffer--the image moves exactly half as far as my fingers when using a Wayland environment with scale 2.

Zooming, on the other hand, seems to be correctly proportional (because dividing initial by current distance would cancel out any scale factor), but is unintuitive because it doesn't seem to properly respect the zoom position. I think this is because the logic for keeping as much of the image as possible onscreen when zooming with the mouse interferes.

In addition, zooming in and out with the mouse seems to make the viewport slowly drift. This seems to be because viewport.c should use doubles, not ints, for its x/y coordinates.

I haven't figured out the zoom/pan interaction yet.

Current patch:

diff --git a/src/imv.c b/src/imv.c
index c88e3c4..4abeafa 100644
--- a/src/imv.c
+++ b/src/imv.c
@@ -471,7 +471,7 @@ static void event_handler(void *data, const struct imv_event *e)
         double x, y;
         imv_window_get_mouse_position(imv->window, &x, &y);
         imv_viewport_zoom(imv->view, imv->current_image, IMV_ZOOM_MOUSE,
-            x, y, -e->data.mouse_scroll.dy);
+            x, y, -e->data.mouse_scroll.dy, false);
       }
       break;
     case IMV_EVENT_CUSTOM:
@@ -493,7 +493,7 @@ static void event_handler(void *data, const struct imv_event *e)
 	  } 
         } 
 	if (x > width / 4 && x < width * 3 / 4) {
-	  imv->overlay_enabled = !imv->overlay_enabled;
+	  imv->overlay.enabled = !imv->overlay.enabled;
 	  imv->need_redraw = true;
 	}
       }
@@ -511,7 +511,7 @@ static void event_handler(void *data, const struct imv_event *e)
 	int new_zoom_int = (int) (new_zoom * 100);
 	imv_viewport_zoom(imv->view, imv->current_image, IMV_ZOOM_TOUCH,
 			  e->data.touch_zoom.x, e->data.touch_zoom.y,
-			  new_zoom_int);
+			  new_zoom_int, true);
       }
       break;
     case IMV_EVENT_TOUCH_PAN_START:
@@ -1717,7 +1717,7 @@ static void command_zoom(struct list *args, const char *argstr, void *data)
       imv_viewport_scale_to_actual(imv->view, imv->current_image);
     } else {
       long int amount = strtol(args->items[1], NULL, 10);
-      imv_viewport_zoom(imv->view, imv->current_image, IMV_ZOOM_KEYBOARD, 0, 0, amount);
+      imv_viewport_zoom(imv->view, imv->current_image, IMV_ZOOM_KEYBOARD, 0, 0, amount, false);
     }
   }
 }
diff --git a/src/viewport.c b/src/viewport.c
index c9d7378..cb7df71 100644
--- a/src/viewport.c
+++ b/src/viewport.c
@@ -13,7 +13,7 @@ struct imv_viewport {
   struct {
     int width, height;
   } buffer; /* rendering buffer dimensions */
-  int x, y;
+  double x, y;
   double pan_factor_x, pan_factor_y;
   int redraw;
   int playing;
@@ -152,7 +152,8 @@ void imv_viewport_move_relative(struct imv_viewport *view,
 }
 
 void imv_viewport_zoom(struct imv_viewport *view, const struct imv_image *image,
-                       enum imv_zoom_source src, int mouse_x, int mouse_y, int amount)
+                       enum imv_zoom_source src, int mouse_x, int mouse_y, int amount,
+                       bool ignore_edges)
 {
   double prev_scale = view->scale;
   int x, y;
@@ -192,19 +193,21 @@ void imv_viewport_zoom(struct imv_viewport *view, const struct imv_image *image,
     view->scale = min_scale;
   }
 
-  if(view->scale < prev_scale) {
-    if(scaled_width < view->buffer.width) {
-      x = scaled_width/2 - (ic_x - wc_x)*2;
-    }
-    if(scaled_height < view->buffer.height) {
-      y = scaled_height/2 - (ic_y - wc_y)*2;
-    }
-  } else {
-    if(scaled_width < view->buffer.width) {
-      x = scaled_width/2;
-    }
-    if(scaled_height < view->buffer.height) {
-      y = scaled_height/2;
+  if(!ignore_edges) {
+    if(view->scale < prev_scale) {
+      if(scaled_width < view->buffer.width) {
+        x = scaled_width/2 - (ic_x - wc_x)*2;
+      }
+      if(scaled_height < view->buffer.height) {
+        y = scaled_height/2 - (ic_y - wc_y)*2;
+      }
+    } else {
+      if(scaled_width < view->buffer.width) {
+        x = scaled_width/2;
+      }
+      if(scaled_height < view->buffer.height) {
+        y = scaled_height/2;
+      }
     }
   }
 
diff --git a/src/viewport.h b/src/viewport.h
index b54cf06..024ed89 100644
--- a/src/viewport.h
+++ b/src/viewport.h
@@ -64,7 +64,8 @@ void imv_viewport_move_relative(struct imv_viewport *view, int initial_x,
 /* Zoom the view by the given amount. imv_image* is used to get the image
  * dimensions */
 void imv_viewport_zoom(struct imv_viewport *view, const struct imv_image *image,
-                       enum imv_zoom_source, int mouse_x, int mouse_y, int amount);
+                       enum imv_zoom_source, int mouse_x, int mouse_y, int amount,
+                       bool ignore_edges);
 
 /* Rotate the view by the given number of degrees */
 void imv_viewport_rotate_by(struct imv_viewport *view, double degrees);
diff --git a/src/wl_window.c b/src/wl_window.c
index bcc4459..fd08a76 100644
--- a/src/wl_window.c
+++ b/src/wl_window.c
@@ -559,10 +559,10 @@ static void touch_motion(void *data, struct wl_touch *touch, uint32_t time,
 	    .type = event_type,
 	    .data = {
 	      .touch_pan = {
-		.initial_x = (int) point->initial.x,
-	        .initial_y = (int) point->initial.y,
-		.current_x = (int) point->current.x,
-		.current_y = (int) point->current.y
+		.initial_x = (int) point->initial.x * window->scale,
+	        .initial_y = (int) point->initial.y * window->scale,
+		.current_x = (int) point->current.x * window->scale,
+		.current_y = (int) point->current.y * window->scale
 	      }
 	    }
 	  };

[mlngh]

Here's a patch that fixes pan/zoom interaction:

diff --git a/src/wl_window.c b/src/wl_window.c
index bcc4459..7bbd5cf 100644
--- a/src/wl_window.c
+++ b/src/wl_window.c
@@ -509,6 +509,26 @@ static void touch_up(void *data, struct wl_touch *touch, uint32_t serial,
       wl_list_remove(&point->link);
     }
   }
+  /* if after touch up we have one point, reinitiate pan */
+  if (wl_list_length(&window->touch.points) == 1) {
+    wl_list_for_each_safe(point, tmp, &window->touch.points, link) {
+        point->initial.x = point->current.x;
+        point->initial.y = point->current.y;
+       struct imv_event e = {
+         .type = IMV_EVENT_TOUCH_PAN_START,
+         .data = {
+           .touch_pan = {
+             .initial_x = (int) point->initial.x * window->scale,
+             .initial_y = (int) point->initial.y * window->scale,
+             .current_x = (int) point->current.x * window->scale,
+             .current_y = (int) point->current.y * window->scale
+           }
+         }
+       };
+       imv_window_push_event(window, &e);
+    }
+  }
+
 }