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

Add scroll fix flags #2034

Closed
1 task
ghost opened this issue Aug 5, 2022 · 20 comments
Closed
1 task

Add scroll fix flags #2034

ghost opened this issue Aug 5, 2022 · 20 comments

Comments

@ghost
Copy link

ghost commented Aug 5, 2022

Description

Let us have a flags to fix scrolling issue on wayland (linux)

Who's implementing?

  • I'm willing to implement this feature myself

The problem

A known but for many many many year on chromium with wayland slow down heavily the scroll speed on chromium based software (electron too), and google don't fix it at all (more than 5 year that the bug is known).

Possible solutions

Add a flags in chromium://flags who let us speed up this wheel speed on all page from 1.0 (normal speed) with 0.1 increment : 1.1, 1.2, 1.3, ... 2.0

Alternatives

No response

Additional context

This actual bug cause nightmarish navigation for nearly all ungoogled chromium (and other software based on chromium) for year, but since last year where first linux started to use wayland native, it become even more nightmare (and using a fix for X will stop to work when xwayland will be less and less used.

I hope to see this fix, because using chromium now on wayland is a nightmare and i will not go back to X (and loose all benefit of wayland on multi display, security, HIDPi, etc)

i have found issue related to this bugs more than ... 9 Years ago another proof the chromium team will not fix it soon (why i ask you) : https://askubuntu.com/questions/254367/permanently-fix-chrome-scroll-speed

Thanks

@daudix
Copy link

daudix commented Sep 14, 2022

There is PR on chromium repo: chromium/chromium#128

@do-sch
Copy link

do-sch commented Sep 24, 2022

There is PR on chromium repo: chromium/chromium#128

The pull request does not change the scroll speed. It fixes the touchpad gestures to go forward and backward in the browser history and it fixes the kinetic scrolling with the touchpad.

@daudix
Copy link

daudix commented Sep 24, 2022

Oh, sorry

@do-sch
Copy link

do-sch commented Sep 24, 2022

I don't think the problem is with Chromium, but with Mutter. When scrolling, the mouse sends how many degrees the mouse wheel has moved. In the past, Mutter ignored the value and always sent the value 10°, even though most scroll wheels scroll with 15°. This can be easily examined if you start Chromium with the environment variable WAYLAND_DEBUG=1 and compare the y-value of wl_pointer@3.axis with the value that sudo libinput debug-events prints when scrolling. I haven't tried it yet, but this merge, which is included in Gnome 43 might fix it. However, this should always have been correctly implemented in KDE. You can also set the scroll speed there. The window manager scales the degree value. But there is also a pull request for it in Mutter.

If it was just Mutter all the time, the Chromium developers are right. Bugs in the window manager shouldn't have to be worked around in the applications.

@ghost
Copy link
Author

ghost commented Sep 24, 2022

I don't think the problem is with Chromium, but with Mutter. When scrolling, the mouse sends how many degrees the mouse wheel has moved. In the past, Mutter ignored the value and always sent the value 10°, even though most scroll wheels scroll with 15°. This can be easily examined if you start Chromium with the environment variable WAYLAND_DEBUG=1 and compare the y-value of wl_pointer@3.axis with the value that sudo libinput debug-events prints when scrolling. I haven't tried it yet, but this merge, which is included in Gnome 43 might fix it. However, this should always have been correctly implemented in KDE. You can also set the scroll speed there. The window manager scales the degree value. But there is also a pull request for it in Mutter.

If it was just Mutter all the time, the Chromium developers are right. Bugs in the window manager shouldn't have to be worked around in the applications.

but i do think the bug can also happen on KDE (or it's fixed someone can confirm that (or not)) ?
But my points it's a temporary fix, because the bug is so old that it's unberable at this point ...

@do-sch
Copy link

do-sch commented Sep 25, 2022

but i do think the bug can also happen on KDE (or it's fixed someone can confirm that (or not)) ? But my points it's a temporary fix, because the bug is so old that it's unberable at this point ...

I have now tested the following with my mouse, which scrolls at 15°:
On Fedora 36 with Gnome 42, Chromium shows 10° with WAYLAND_DEBUG=1. The same with Fedora 37 Beta with Gnome 43. Unfortunately, nothing seems be fixed.
Under Plasma 5.25.5, however, WAYLAND_DEBUG=1 shows the value 15° when scrolling. So Chromium scrolls 50% faster in KDE. If I move the slider for scrolling speed in plasmas settings, values of 22.5° and 30° are even displayed. Chromium then scrolls even much faster.

For me it's apparently a bug in Mutter. If you still want to patch it in Chromium, under ui/ozone/platform/wayland/host/wayland_event_source.cc in the method WaylandEventSource::OnPointerAxisEvent you should be able to just multiply offset.y and offset.x with a float.
But I would exclude touchpad events from the multiplication. An extra flag would then have to be introduced for this. Otherwise, people who depend on both only have the choice of what scrolls at the correct speed now.

An alternative approach would be to use the WaylandPointer::AxisDiscrete method in ui/ozone/platform/wayland/host/wayland_pointer.cc. Chromium currently ignores discrete scroll events, which are also sent by Wayland. You could introduce a flag that ignores the associated axis events for discrete events and then generates axis events itself with a value that can be configured in the flag.

@do-sch
Copy link

do-sch commented Oct 15, 2022

I have to correct my statement. When trying to fix the bug in Mutter, I found that it's not implemented incorrectly in Mutter, but rather in Chromium and KWin.

In Weston, the Wayland reference implementation, the axis events are also 10 for me when my mouse wheel scrolls 15°. So I assume 10 is the correct value.

I've also noticed that touchpad scrolling is also far slower than Firefox, although it's not that noticeable.

@do-sch
Copy link

do-sch commented Oct 22, 2022

I added scroll flags and command line arguments to manipulate scroll speed in a fork of main

I also made a patch for Chromium 106. But since this takes all day to build on my computer, I can't manage to make a merge request this weekend. If anyone wants to do it themselves, here is the patch:

From 090bd75631e9b8858a7babe9831a013a8ec8727a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dominik=20Sch=C3=BCtz?= <do.sch.dev@gmail.com>
Date: Sat, 22 Oct 2022 10:20:41 +0200
Subject: [PATCH] Allow configuration of scroll speed in Wayland

This commit allows specifying a value by which the scroll speed in Wayland is multiplied. This can be done either with "--wayland-scroll-speed-multiplier=xx" or under "chrome://flags/#wayland-scroll-speed-multiplier"
---
 chrome/browser/about_flags.cc                 | 35 +++++++++++++++++++
 chrome/browser/flag_descriptions.cc           | 19 ++++++++++
 chrome/browser/flag_descriptions.h            | 18 ++++++++++
 .../wayland/host/wayland_event_source.cc      | 22 ++++++++++--
 .../wayland/host/wayland_event_source.h       |  3 ++
 ui/ozone/public/ozone_switches.cc             |  3 ++
 ui/ozone/public/ozone_switches.h              |  3 ++
 7 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 9a0c47ab605..60ba9a17a3e 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -406,6 +406,36 @@ const FeatureEntry::Choice kOzonePlatformHintRuntimeChoices[] = {
      switches::kOzonePlatformHint, "wayland"},
 #endif
 };
+
+const FeatureEntry::Choice kWaylandScrollSpeedMultiplierChoices[] = {
+    {flag_descriptions::kWaylandScrollSpeedMultiplier1, "", ""},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier1_2,
+     switches::kWaylandScrollSpeedMultiplier, "1.2"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier1_4,
+     switches::kWaylandScrollSpeedMultiplier, "1.4"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier1_6,
+     switches::kWaylandScrollSpeedMultiplier, "1.6"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier1_8,
+     switches::kWaylandScrollSpeedMultiplier, "1.8"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier2,
+     switches::kWaylandScrollSpeedMultiplier, "2.0"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier2_5,
+     switches::kWaylandScrollSpeedMultiplier, "2.5"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier3,
+     switches::kWaylandScrollSpeedMultiplier, "3.0"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier3_5,
+     switches::kWaylandScrollSpeedMultiplier, "3.5"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier4,
+     switches::kWaylandScrollSpeedMultiplier, "4.0"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier5,
+     switches::kWaylandScrollSpeedMultiplier, "5.0"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier6,
+     switches::kWaylandScrollSpeedMultiplier, "6.0"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier8,
+     switches::kWaylandScrollSpeedMultiplier, "8.0"},
+    {flag_descriptions::kWaylandScrollSpeedMultiplier10,
+     switches::kWaylandScrollSpeedMultiplier, "10.0"},
+};
 #endif
 
 #if BUILDFLAG(ENABLE_VR)
@@ -4554,6 +4584,11 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kOzonePlatformHintDescription, kOsLinux,
      MULTI_VALUE_TYPE(kOzonePlatformHintRuntimeChoices)},
 
+    {"wayland-scroll-speed-multiplier",
+     flag_descriptions::kWaylandScrollSpeedMultiplierName,
+     flag_descriptions::kWaylandScrollSpeedMultiplierDescription, kOsLinux,
+     MULTI_VALUE_TYPE(kWaylandScrollSpeedMultiplierChoices)},
+
     {"clean-undecryptable-passwords",
      flag_descriptions::kCleanUndecryptablePasswordsLinuxName,
      flag_descriptions::kCleanUndecryptablePasswordsLinuxDescription, kOsLinux,
diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
index 2998fc1683b..67ae697385d 100644
--- a/chrome/browser/flag_descriptions.cc
+++ b/chrome/browser/flag_descriptions.cc
@@ -6259,6 +6259,25 @@ const char kOzonePlatformHintName[] = "Preferred Ozone platform";
 const char kOzonePlatformHintDescription[] =
     "Selects the preferred platform backend used on Linux. The default one is "
     "\"X11\". \"Auto\" selects Wayland if possible, X11 otherwise. ";
+
+const char kWaylandScrollSpeedMultiplier1[] = "Default";
+const char kWaylandScrollSpeedMultiplier1_2[] = "1.2";
+const char kWaylandScrollSpeedMultiplier1_4[] = "1.4";
+const char kWaylandScrollSpeedMultiplier1_6[] = "1.6";
+const char kWaylandScrollSpeedMultiplier1_8[] = "1.8";
+const char kWaylandScrollSpeedMultiplier2[] = "2.0";
+const char kWaylandScrollSpeedMultiplier2_5[] = "2.5";
+const char kWaylandScrollSpeedMultiplier3[] = "3.0";
+const char kWaylandScrollSpeedMultiplier3_5[] = "3.5";
+const char kWaylandScrollSpeedMultiplier4[] = "4.0";
+const char kWaylandScrollSpeedMultiplier5[] = "5.0";
+const char kWaylandScrollSpeedMultiplier6[] = "6.0";
+const char kWaylandScrollSpeedMultiplier8[] = "8.0";
+const char kWaylandScrollSpeedMultiplier10[] = "10.0";
+const char kWaylandScrollSpeedMultiplierName[] =
+    "Changes the scrolling speed in Wayland";
+const char kWaylandScrollSpeedMultiplierDescription[] =
+    "Sets a multiplier by which the scroll distance is multiplied.";
 #endif  // BUILDFLAG(IS_LINUX) && !BUILDFLAG(IS_CHROMEOS)
 
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
index d0950892428..af0c74a43d7 100644
--- a/chrome/browser/flag_descriptions.h
+++ b/chrome/browser/flag_descriptions.h
@@ -3600,6 +3600,24 @@ extern const char kOzonePlatformHintChoiceWayland[];
 extern const char kOzonePlatformHintName[];
 extern const char kOzonePlatformHintDescription[];
 
+extern const char kWaylandScrollSpeedMultiplier1[];
+extern const char kWaylandScrollSpeedMultiplier1_2[];
+extern const char kWaylandScrollSpeedMultiplier1_4[];
+extern const char kWaylandScrollSpeedMultiplier1_6[];
+extern const char kWaylandScrollSpeedMultiplier1_8[];
+extern const char kWaylandScrollSpeedMultiplier2[];
+extern const char kWaylandScrollSpeedMultiplier2_5[];
+extern const char kWaylandScrollSpeedMultiplier3[];
+extern const char kWaylandScrollSpeedMultiplier3_5[];
+extern const char kWaylandScrollSpeedMultiplier4[];
+extern const char kWaylandScrollSpeedMultiplier5[];
+extern const char kWaylandScrollSpeedMultiplier6[];
+extern const char kWaylandScrollSpeedMultiplier8[];
+extern const char kWaylandScrollSpeedMultiplier10[];
+
+extern const char kWaylandScrollSpeedMultiplierName[];
+extern const char kWaylandScrollSpeedMultiplierDescription[];
+
 extern const char kCleanUndecryptablePasswordsLinuxName[];
 extern const char kCleanUndecryptablePasswordsLinuxDescription[];
 
diff --git a/ui/ozone/platform/wayland/host/wayland_event_source.cc b/ui/ozone/platform/wayland/host/wayland_event_source.cc
index 11acbefd60d..04f148397ec 100644
--- a/ui/ozone/platform/wayland/host/wayland_event_source.cc
+++ b/ui/ozone/platform/wayland/host/wayland_event_source.cc
@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/check.h"
+#include "base/command_line.h"
 #include "base/containers/cxx20_erase.h"
 #include "base/logging.h"
 #include "base/memory/raw_ptr.h"
@@ -34,11 +35,24 @@
 #include "ui/ozone/platform/wayland/host/wayland_keyboard.h"
 #include "ui/ozone/platform/wayland/host/wayland_window.h"
 #include "ui/ozone/platform/wayland/host/wayland_window_manager.h"
+#include "ui/ozone/public/ozone_switches.h"
 
 namespace ui {
 
 namespace {
 
+float axisScaleFromCmdLineSwitch() {
+  const std::string multiplier =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kWaylandScrollSpeedMultiplier);
+  double result = 1.0;
+
+  if (base::StringToDouble(multiplier, &result))
+    return result;
+
+  return 1;
+}
+
 bool HasAnyPointerButtonFlag(int flags) {
   return (flags & (EF_LEFT_MOUSE_BUTTON | EF_MIDDLE_MOUSE_BUTTON |
                    EF_RIGHT_MOUSE_BUTTON | EF_BACK_MOUSE_BUTTON |
@@ -155,6 +169,10 @@ WaylandEventSource::WaylandEventSource(wl_display* display,
 
   // Observes remove changes to know when touch points can be removed.
   window_manager_->AddObserver(this);
+
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kWaylandScrollSpeedMultiplier))
+    axis_scale_ = axisScaleFromCmdLineSwitch();
 }
 
 WaylandEventSource::~WaylandEventSource() = default;
@@ -328,8 +346,8 @@ void WaylandEventSource::OnPointerMotionEvent(const gfx::PointF& location) {
 }
 
 void WaylandEventSource::OnPointerAxisEvent(const gfx::Vector2dF& offset) {
-  EnsurePointerScrollData().dx += offset.x();
-  EnsurePointerScrollData().dy += offset.y();
+  EnsurePointerScrollData().dx += offset.x() * axis_scale_;
+  EnsurePointerScrollData().dy += offset.y() * axis_scale_;
 }
 
 void WaylandEventSource::OnResetPointerFlags() {
diff --git a/ui/ozone/platform/wayland/host/wayland_event_source.h b/ui/ozone/platform/wayland/host/wayland_event_source.h
index 441c150c118..bcec24553eb 100644
--- a/ui/ozone/platform/wayland/host/wayland_event_source.h
+++ b/ui/ozone/platform/wayland/host/wayland_event_source.h
@@ -241,6 +241,9 @@ class WaylandEventSource : public PlatformEventSource,
   // See ui/events/event_constants.h for examples and details.
   int keyboard_modifiers_ = 0;
 
+  // Scroll scale that is multiplied with scroll amount
+  float axis_scale_ = 1;
+
   // Last known pointer location.
   gfx::PointF pointer_location_;
 
diff --git a/ui/ozone/public/ozone_switches.cc b/ui/ozone/public/ozone_switches.cc
index 1f6ccc48ff3..eeb23abb5fd 100644
--- a/ui/ozone/public/ozone_switches.cc
+++ b/ui/ozone/public/ozone_switches.cc
@@ -13,6 +13,9 @@ const char kOzonePlatform[] = "ozone-platform";
 // chrome://flags.  See https://crbug.com/1246928.
 const char kOzonePlatformHint[] = "ozone-platform-hint";
 
+// Scroll speed multiplier for Wayland.
+const char kWaylandScrollSpeedMultiplier[] = "wayland-scroll-speed-multiplier";
+
 // Specify location for image dumps.
 const char kOzoneDumpFile[] = "ozone-dump-file";
 
diff --git a/ui/ozone/public/ozone_switches.h b/ui/ozone/public/ozone_switches.h
index ca63b0d2626..caa351522d3 100644
--- a/ui/ozone/public/ozone_switches.h
+++ b/ui/ozone/public/ozone_switches.h
@@ -15,6 +15,9 @@ COMPONENT_EXPORT(OZONE_SWITCHES) extern const char kOzonePlatform[];
 
 COMPONENT_EXPORT(OZONE_SWITCHES) extern const char kOzonePlatformHint[];
 
+COMPONENT_EXPORT(OZONE_SWITCHES)
+extern const char kWaylandScrollSpeedMultiplier[];
+
 COMPONENT_EXPORT(OZONE_SWITCHES) extern const char kOzoneDumpFile[];
 
 COMPONENT_EXPORT(OZONE_SWITCHES) extern const char kEnableWaylandIme[];
-- 
2.37.3

@KingRayleigh
Copy link

@do-sch Thanks for this patch brother but can you provide it as a chromium.rpm or flatpak something like this?? am a very non technical person who dont know how to build these things .. once again i wanna appreciate your contribution..

@do-sch
Copy link

do-sch commented Nov 6, 2022

@KingRayleigh I'd love to make a pull request, but I can't seem to build ungoogled-chromium. It breaks off at this point:

../../../components/signin/internal/identity_manager/primary_account_manager.cc:151:48: error: no member named 'kGoogleServicesConsentedToSync' in namespace 'prefs'
        client_->GetPrefs()->GetBoolean(prefs::kGoogleServicesConsentedToSync);

It's very depressing because if I change something it builds half the day on my hardware before it stops building again.

I can build the normal Chromium without any problems. Can anyone confirm that master is currently building?

@networkException
Copy link
Member

Multiple releases have been built from the lastest commit on master, so it should work. Maybe the build scripts from this repository are outdated, at least I only built the arch package.

@PF4Public
Copy link
Contributor

Is this still an issue?

@PF4Public PF4Public added the need info Need feedback to proceed label Jun 19, 2023
@github-actions
Copy link

This issue has been automatically marked as stale as there has been no recent activity in response to our request for more information. Please respond so that we can proceed with this issue.

@github-actions github-actions bot added the Stale label Jul 20, 2023
@github-actions
Copy link

This issue has been automatically closed as sufficient information hasn't been provided on the issue for further actions to be taken. Feel free to add more information.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 28, 2023
@gazhay
Copy link

gazhay commented Oct 25, 2023

Is this still an issue?

Yes

@gringus
Copy link

gringus commented Nov 5, 2023

You can run into this issue when eg. debug is enabled as we have:

#if DCHECK_IS_ON()
  {
    std::string pref_account_id;
    bool consented_to_sync =
        client_->GetPrefs()->GetBoolean(prefs::kGoogleServicesConsentedToSync); << error occurs here

and in other place

#if defined(NDEBUG) && !defined(DCHECK_ALWAYS_ON)
#define DCHECK_IS_ON() false
#else
#define DCHECK_IS_ON() true
#endif

I just ran into this error (../../components/signin/internal/identity_manager/primary_account_manager.cc:257:48: error: no member named 'kGoogleServicesConsentedToSync' in namespace 'prefs' when building ungoogled-chromium-119.0.6045.105)

@PF4Public
Copy link
Contributor

Should this code be also removed in ungoogled-chromium?

@gringus
Copy link

gringus commented Nov 6, 2023

I have worked around this by setting variable to false as I saw in other places in the patch (not sure if this was the right thing to do):

--- chromium-119.0.6045.105/components/signin/internal/identity_manager/primary_account_manager.cc.orig	2023-11-06 09:01:05.307406151 +0100
+++ chromium-119.0.6045.105/components/signin/internal/identity_manager/primary_account_manager.cc	2023-11-06 09:01:22.814723221 +0100
@@ -253,8 +253,7 @@
 #if DCHECK_IS_ON()
   {
     std::string pref_account_id;
-    bool consented_to_sync =
-        client_->GetPrefs()->GetBoolean(prefs::kGoogleServicesConsentedToSync);
+    bool consented_to_sync = false;
 
     DCHECK(pref_account_id.empty() || !consented_to_sync ||
            pref_account_id == account_info.account_id.ToString())

@PF4Public
Copy link
Contributor

@Ahrotahn Please have a look at the message above. We should't break debug, right?

@Ahrotahn
Copy link
Contributor

Ahrotahn commented Nov 7, 2023

The changes in ungoogled-chromium have historically been made without regard to building with DCHECKs. Instead of building with is_debug you would need to build with symbol_level=2 and/or blink_symbol_level=2 & v8_symbol_level=2 depending on what you're trying to debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants