Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scroll_factor 0.5 does not reduce speed, instead skips events (app-dependent, regression from 1.7) #7338

Open
amarshall opened this issue Dec 23, 2022 · 4 comments
Labels
bug Not working as intended

Comments

@amarshall
Copy link

amarshall commented Dec 23, 2022

  • Sway Version: 1.8-rc4 (update: still occurring with 1.8 release)

  • Debug Log: Here. Note that this was from a nested sway session, and I just ctrl-c-ed it at the end. In it, I opened Kitty, ran a command to print a bunch of output, and scrolled.

  • Configuration File: (cat /etc/sway/config ; echo 'input * scroll_factor 0.5') > sway.conf

  • Stack Trace: n/a

  • Description:

    1. Set scroll_factor to 0.5 e.g. swaymsg input '*' scroll_factor 0.5 (note if testing in nested Sway, likely need to set scroll_factor 1 in the outer Sway.
    2. Expect: Scroll speed to be halved, scroll still occurs on each wheel scroll event (this is the behavior I observe sway 1.7). Actual: Scroll speed is unchanged depending on the app, but every other scroll input is ignored.

    Does not seem to affect touchpad, only wheel with discrete steps.

    Applications that are affected include Firefox and Kitty. Applications not affected include Nautilus and Gnome Text Editor; I have not tested any others yet. While I suspect the affected applications are partly at fault, this is still a change from Sway 1.7 while the application versions have been unchanged. Possibly related issue is broken scroll_factor behavior in sway and xwayland clients #4679, however it does seem to be different. I would suspect that this is perhaps a result of the change to high-resolution scroll events—maybe?

@amarshall amarshall added the bug Not working as intended label Dec 23, 2022
@rpigott
Copy link
Member

rpigott commented Dec 30, 2022

I think this is intended behavior.

For the record, nautilus binds wl_seat v8 (w/ value_120) while it seems firefox and kitty take v1 (!) and v5 respectively. Sway tabs (for tabbed containers) behavior is similar to the legacy clients. It's actually not clear nautilus' behavior is even correct here if it tabs on each value120 event. It might make sense to just have a separate scroll factor for discrete events, especially since the mouse may not do hi-res scrolling.

The linked issues are outdated, so I will close them.

@amarshall
Copy link
Author

amarshall commented Dec 30, 2022

I’ve spent a bit going through the sway and wlroots code and am starting to grok things a bit.

My initial angle to attack this was from the wlroots side, and applying the factor to its step in the accumulator seems to do the job:

diff --git a/types/seat/wlr_seat_pointer.c b/types/seat/wlr_seat_pointer.c
index 8d6154b0..89b68044 100644
--- a/types/seat/wlr_seat_pointer.c
+++ b/types/seat/wlr_seat_pointer.c
@@ -285,6 +285,7 @@ static void update_value120_accumulators(struct wlr_seat_client *client,
                return;
        }
 
+       int32_t step = WLR_POINTER_AXIS_DISCRETE_STEP * 0.5;
        int32_t *acc_discrete = &client->value120.acc_discrete[orientation];
        int32_t *last_discrete = &client->value120.last_discrete[orientation];
        double *acc_axis = &client->value120.acc_axis[orientation];
@@ -299,11 +300,11 @@ static void update_value120_accumulators(struct wlr_seat_client *client,
 
        // Compute low resolution event values for older clients and reset
        // the accumulators if needed
-       *low_res_value_discrete = *acc_discrete / WLR_POINTER_AXIS_DISCRETE_STEP;
+       *low_res_value_discrete = *acc_discrete / step;
        if (*low_res_value_discrete == 0) {
                *low_res_value = 0;
        } else {
-               *acc_discrete -= *low_res_value_discrete * WLR_POINTER_AXIS_DISCRETE_STEP;
+               *acc_discrete -= *low_res_value_discrete * step;
                *low_res_value = *acc_axis;
                *acc_axis = 0;
        }

Doing that properly would, as far as I can tell, require that sway etc. pass the discrete step factor in. Not really sure if that’s the best though.

I tried reverting the accumulator bits, but I couldn’t get it to work (wouldn’t scroll at all)—may play with it a bit more.


However, as you hint at, modifying the scroll factor for the discrete value in sway instead may also work, so I tried this:

diff --git a/sway/input/seatop_default.c b/sway/input/seatop_default.c
index 84acefdf..314c42c6 100644
--- a/sway/input/seatop_default.c
+++ b/sway/input/seatop_default.c
@@ -761,7 +761,7 @@ static void handle_pointer_axis(struct sway_seat *seat,
        if (!handled) {
                wlr_seat_pointer_notify_axis(cursor->seat->wlr_seat, event->time_msec,
                        event->orientation, scroll_factor * event->delta,
-                       round(scroll_factor * event->delta_discrete), event->source);
+                       round(event->delta_discrete), event->source);
        }
 }

Doing that resulted in Nautilus (and perhaps all other v8+ clients that take this code path) not having the scroll speed adjusted at all. As you say, though, maybe Nautilus is doing something wrong itself (though I think you’re only talking about scrolling the tab list, whereas I’m mostly testing scrolling the file list), and I don’t know any other v8+ clients off-hand to test. (Personally I found after Nautilus went to GTK 4 that the scroll speed got way too slow.)

@paolomainardi
Copy link

paolomainardi commented Jan 20, 2024

I am experiencing the same issue on Firefox. Even with the scroll_factor set to 0.2 and using an MX Master 3s, I need to scroll the mouse wheel five times before Firefox begins scrolling the lines. This works seamlessly on Chrome.

@GalaxySnail
Copy link

GalaxySnail commented Jan 25, 2024

This issue can still reproduce on sway 1.8.1. When scroll_factor is set to 0.5, wev shows that the compositor alternately sends an empty frame and a scroll frame in a loop. When scroll_factor is set to 0.33 or 0.333333333333333, the compositor alternately sends two empty frames and a scroll frame in a loop.

terminal recording: https://asciinema.org/a/fvlDGLNmKeRmRc2nPrC7XeK13

Interestingly, when scroll_factor is set to 0.33, the axis value is 14.851562 instead of 15, which looks close to 15 * 0.33 * 3 = 14.85. (Update: It should be the 24.8 bit fixed-point form of 14.85

>>> round(15 * 0.33 * 3 * 2**8) / 2**8
14.8515625

And only the last scroll in a continuous sequence in the same direction will trigger an event. Scrolling alternately up and down will only result in empty events.

terminal recording: https://asciinema.org/a/sR6NaZBdZcwAeACa4swGLbaKx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended
Development

No branches or pull requests

4 participants