Rawtherapee: Touchpad scrolling not working in thumbnail browser

[Barugon]

Touchpad scrolling doesn't work in the thumbnail browser and also doesn't zoom the image view.

Version: 5.1
Branch: 5.1
Commit: f23d70fd
Commit date: 2017-05-15
Compiler: cc 6.2.1
Processor: x86_64
System: Linux
Bit depth: 64 bits
Gtkmm: V3.22.0
Build type: release
Build flags: -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -std=c++11 -Werror=unused-label -fopenmp -Werror=unknown-pragmas -Wall -Wno-unused-result -Wno-deprecated-declarations -O2 -DNDEBUG
Link flags:
OpenMP support: ON
MMAP support: ON

OS: Fedora 25, x86_64, GNOME 3.22.2
Memory: 16GB
Processor: Intel i7-4720HQ
Graphics: Intel HD 4600

[heckflosse]

Can't reproduce because I don't have a touchpad.
But why are the rt fedora builds made using -O2 instead of -O3?
Using -O2 does not generate auto-vectorized code ...

[Floessie]

But why are the rt fedora builds made using -O2 instead of -O3?
Using -O2 does not generate auto-vectorized code ...

That's a question for @mattiaverga, I guess.

[mattiaverga]

+ CFLAGS='-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic'
+ export CFLAGS
+ CXXFLAGS='-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic'
+ export CXXFLAGS

These are the default build flags used by Fedora build system.

I also can't reproduce the problem, because I don't have a touchpad. But using a mouse all is working fine, so maybe it's a problem between gtk+ and touchpad(s) input.

[Barugon]

Mousewheel works fine. This is definitely a touchpad only issue.

[Beep6581]

Touchpad scrolling works fine here in both 5.1 and dev.

Version: 5.1-77-g8aff354d
Branch: dev
Commit: 8aff354d
Commit date: 2017-06-18
Compiler: cc 4.9.3
Processor: Intel(R)\ Core(TM)\ m3-6Y30\ CPU\ @\ 0.90GHz
System: Linux
Bit depth: 64 bits
Gtkmm: V3.22.0
Build type: release
Build flags:  -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -Wall -Wno-unused-result -Wno-deprecated-declarations -O3 -DNDEBUG
Link flags:  -march=native
OpenMP support: ON
MMAP support: ON

[xaxxon]

Just downloaded latest RT on macbook air mid 2011 running os x 10.11.6. Two-finger scrolling in thumbnails always scrolls left, whether I swipe left or right with my fingers.

[Beep6581]

@xaxxon what happens with this version?
https://filebin.net/fu841fry89ozve0b/RawTherapee_OSX_10.9_64_5.3-643-gfc751a0cc.zip

[xaxxon]

Same behavior. up/down scrolling scrolls left and right but left/right scrolling always scrolls left.

https://www.youtube.com/watch?v=CSW8SOXzbt8

When you see the thumbnails scrolling right, that's me using up/down scrolling.

Any work being done on the RT UI responsiveness? Lightroom is almost perfectly smooth on my laptop, but RT is pretty much just a beachball whenever I do anything at all.

[Beep6581]

Confirmed in Sabayon/KDE.
I had to enable horizontal scrolling:

[xaxxon]

Well, this seems a pretty obvious UI bug.

[xaxxon]

Honestly, I don't even know where I'd go to find that, as there's no "preferences" menu option which is 100% standard in mac apps.. Or I guess 99.999%

edit: I've now spent 10+ minutes in RT and on google trying to search for how to find something that looks like what you posted. I've found nothing.

[Hombre57]

@xaxxon I think it's a snapshot of the touchpad driver, it's not an RT window.

I'll check tonight if there are some special events to handle for touchpad device.

[Beep6581]

This patch fixes it for horizontal scrolling, while breaking vertical scrolling, so it's only a starting point:

diff --git a/rtgui/thumbbrowserbase.cc b/rtgui/thumbbrowserbase.cc
index abb8ec68..5c2ab315 100644
--- a/rtgui/thumbbrowserbase.cc
+++ b/rtgui/thumbbrowserbase.cc
@@ -74,7 +74,7 @@ void ThumbBrowserBase::scroll (int direction)
     if (arrangement == TB_Vertical) {
         vscroll.set_value (vscroll.get_value() + (direction == GDK_SCROLL_DOWN ? +1 : -1) * vscroll.get_adjustment()->get_step_increment());
     } else {
-        hscroll.set_value (hscroll.get_value() + (direction == GDK_SCROLL_DOWN ? +1 : -1) * hscroll.get_adjustment()->get_step_increment());
+        hscroll.set_value (hscroll.get_value() + (direction == GDK_SCROLL_RIGHT ? +1 : -1) * hscroll.get_adjustment()->get_step_increment());
     }
 }

@@ -84,7 +84,7 @@ void ThumbBrowserBase::scrollPage (int direction)
     if (arrangement == TB_Vertical) {
         vscroll.set_value (vscroll.get_value() + (direction == GDK_SCROLL_DOWN ? +1 : -1) * vscroll.get_adjustment()->get_page_increment());
     } else {
-        hscroll.set_value (hscroll.get_value() + (direction == GDK_SCROLL_DOWN ? +1 : -1) * hscroll.get_adjustment()->get_page_increment());
+        hscroll.set_value (hscroll.get_value() + (direction == GDK_SCROLL_RIGHT ? +1 : -1) * hscroll.get_adjustment()->get_page_increment());
     }
 }

[Beep6581]

And this seems to only handle "if not down then up"
https://github.com/Beep6581/RawTherapee/blob/dev/rtgui/guiutils.cc#L948

[Beep6581]

Here's a working patch:

diff --git a/rtgui/cropwindow.cc b/rtgui/cropwindow.cc
index 2b20de00..94d0bd5e 100644
--- a/rtgui/cropwindow.cc
+++ b/rtgui/cropwindow.cc
@@ -287,7 +287,7 @@ void CropWindow::scroll (int state, GdkScrollDirection direction, int x, int y)

         screenCoordToImage(newCenterX, newCenterY, newCenterX, newCenterY);

-        if (direction == GDK_SCROLL_UP && !isMaxZoom()) {
+        if ((direction == GDK_SCROLL_UP || direction == GDK_SCROLL_LEFT) && !isMaxZoom()) {
             zoomIn (true, newCenterX, newCenterY);
         } else if (!isMinZoom()) {
             zoomOut (true, newCenterX, newCenterY);
diff --git a/rtgui/thumbbrowserbase.cc b/rtgui/thumbbrowserbase.cc
index abb8ec68..cb6c0561 100644
--- a/rtgui/thumbbrowserbase.cc
+++ b/rtgui/thumbbrowserbase.cc
@@ -71,20 +71,58 @@ void ThumbBrowserBase::scrollChanged ()
 void ThumbBrowserBase::scroll (int direction)
 {
     // GUI already acquired when here
+    double v = 0.;
+
+    switch(direction) {
+        case GDK_SCROLL_UP :
+            v=-1;
+            break;
+        case GDK_SCROLL_DOWN :
+            v=1;
+            break;
+        case GDK_SCROLL_LEFT :
+            v=-1;
+            break;
+        case GDK_SCROLL_RIGHT :
+            v=1;
+            break;
+        case GDK_SCROLL_SMOOTH :
+            break;
+    }
+
     if (arrangement == TB_Vertical) {
-        vscroll.set_value (vscroll.get_value() + (direction == GDK_SCROLL_DOWN ? +1 : -1) * vscroll.get_adjustment()->get_step_increment());
+        vscroll.set_value (vscroll.get_value() + v * vscroll.get_adjustment()->get_step_increment());
     } else {
-        hscroll.set_value (hscroll.get_value() + (direction == GDK_SCROLL_DOWN ? +1 : -1) * hscroll.get_adjustment()->get_step_increment());
+        hscroll.set_value (hscroll.get_value() + v * hscroll.get_adjustment()->get_step_increment());
     }
 }

 void ThumbBrowserBase::scrollPage (int direction)
 {
     // GUI already acquired when here
+    double v = 0.;
+
+    switch(direction) {
+        case GDK_SCROLL_UP :
+            v=-1;
+            break;
+        case GDK_SCROLL_DOWN :
+            v=1;
+            break;
+        case GDK_SCROLL_LEFT :
+            v=-1;
+            break;
+        case GDK_SCROLL_RIGHT :
+            v=1;
+            break;
+        case GDK_SCROLL_SMOOTH :
+            break;
+    }
+
     if (arrangement == TB_Vertical) {
-        vscroll.set_value (vscroll.get_value() + (direction == GDK_SCROLL_DOWN ? +1 : -1) * vscroll.get_adjustment()->get_page_increment());
+        vscroll.set_value (vscroll.get_value() + v * vscroll.get_adjustment()->get_page_increment());
     } else {
-        hscroll.set_value (hscroll.get_value() + (direction == GDK_SCROLL_DOWN ? +1 : -1) * hscroll.get_adjustment()->get_page_increment());
+        hscroll.set_value (hscroll.get_value() + v * hscroll.get_adjustment()->get_page_increment());
     }
 }

What is the expected behavior when:

1. focus is on a horizontal panel and the user scrolls up/down?
2. focus is on a vertical panel and the user scrolls left/right?
3. focus is on the image preview and the user scrolls left/right?
   i.e. in those cases, should the scroll be ignored, or should it behave as if the user scrolled along the other axis?

[Beep6581]

Do the thumbs-up mean the patch should be committed? i.e. should I could this as a personal C++ success? :smiley:

[Hombre57]

A C++ success, yes, but would prefer to wait for the answers of your questions by @Barugon and @xaxxon , unless you'd go for two commits.

If I could nitpicking, I'd remove the test of direction == GDK_SCROLL_LEFT on your first change and add a test of direction == GDK_SCROLL_DOWN in the else if, as I'm not sure it makes sense to use left and right for zooming (additionally, some user have different text direction and may find the left direction to zoom in not that obvious). As a mouse user, I don't mind if it's committed as is.

[Floessie]

@Beep6581

Do the thumbs-up mean the patch should be committed? i.e. should I could this as a personal C++ success?

I just like to see you code C++. :+1:

The cases could be grouped, v could carry a better name, the whole thing could have been solved with ||, but I just like that you did it. :+2:

[xaxxon]

focus is on a horizontal panel and the user scrolls up/down?

No behavior on a touchpad. Perhaps on a mouse wheel up/down it should scroll left/right, but on a device that specifically has input for left/right it should use the most obvious input.

focus is on a vertical panel and the user scrolls left/right?

Nothing, ever. I don't know of any devices where there is horizontal scrolling but not vertical.

focus is on the image preview and the user scrolls left/right?

I don't know what exactly "image preview" means, but on a canvas-type area, the movement should mirror the fluid directional movement of the drag in all directions, not just cardinal directions.

i.e. in those cases, should the scroll be ignored, or should it behave as if the user scrolled along the other axis?

The problem becomes what happens if I'm scrolling "mostly left but somewhat up" if up is mapped to right. The imprecision of my left-ward scroll should not be counteracted by my guaranteed slight vertical imprecision.

It's important to remember that unlike a mouse wheel, a touchpad is inherently imprecise and when the intended behavior is known, the imprecision should be ignored.

[Beep6581]

Thank you very much for the kind words and good advice :)
As I'm already juggling several tasks and I'd like to release 5.4 tonight or tomorrow, I agreed with @Hombre57 on IRC that he would take this. However, I promise to write some more C++ patches when possible :)

[Beep6581]

Summary: due to Gtk+ not distinguishing between a mouse and a touchpad device (it always reports the device as a mouse, tested in Linux and maybe @Hombre57 tested in Windows), scrolling is limited to up/down (always present when he device is a mouse), and nothing will happen for left/right (touchpad only) even though it would make sense for horizontally-scrollable windows, due to what @xaxxon wrote,

The problem becomes what happens if I'm scrolling "mostly left but somewhat up" if up is mapped to right. The imprecision of my left-ward scroll should not be counteracted by my guaranteed slight vertical imprecision.

Closing as fixed, as far as possible.

[Beep6581]

Ah this wasn't merged yet - reopening.

[Hombre57]

@Beep6581 Maybe I'm getting old :scream: but I though that the scrolling branch was already deleted, so I applied the patch to dev and committed it (in commit bf6702f ). Then realized it was still existing, so I've deleted the scrolling branch.

I just wanted to find a proper solution and explore the input methods and devices a bite, time mostly spent on looking for documentation... You can close this issue if you want, I'll reopen it if by any chance I can find my way through device handling.

[heckflosse]

@Hombre57

Maybe I'm getting old

you're kidding ...

[Barugon]

@Beep6581
At what version will this show up? I installed rawtherapee-unstable (5.4-704-g3e7f19c81) on my notebook and I still cannot use two finger scrolling in the thumbnail view.

[Beep6581]

@Barugon 5.5, and in the latest dev which is what you're using.

[Beep6581]

Touchpad scrolling works fine here in the File Browser, Filmstrip and Toolbox.
Sabayon/KDE.
5.4-745-gf919b0b8

[Barugon]

I tried this in RawTherapee-dev-5.4-948-g8a18300.AppImage and when I scroll to the bottom of the thumbnails, it always jumps back up at least one row as soon as I remove my fingers from the touchpad (using two finger, natural scrolling). Also the sensitivity is way too high. Barely any movement causes it to scroll multiple pages.

[Thanatomanic]

Let's leave this thread closed, because other things have changed in the meanwhile. Please see #4663