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

Fix smooth scrolling Linux Wayland. #205122

Merged
merged 4 commits into from Mar 14, 2024

Conversation

AndreasBackx
Copy link
Contributor

@AndreasBackx AndreasBackx commented Feb 13, 2024

Fixes #182499.

MouseWheelClassifier is trying to assess whether the scrolling device is actually a physical scroll wheel and not a touchpad (or touch screen?). There is an _computeScore(...) function that tries to assess a particular scroll event and give it a likelihood of whether it's a physical device or not. This particular part was the reason why the identification failed on Wayland Linux:

if (!this._isAlmostInt(item.deltaX) || !this._isAlmostInt(item.deltaY)) {
	// non-integer deltas => indicator that this is not a physical mouse wheel
	score += 0.25;
}

This is saying that when the delta movement is not a round number, it's more likely to be a touchpad.

The problem however is that on Linux the scroll delta is not a round number like on MacOS and Windows, its base is 1.5 for some reason instead of for example 1 or 2. To replace this, I've found a more reliable way to determine this particular part, see the changed files.

What I'm doing is verifying whether the delta from the current event is a modulo of the previous event or vice versa. This seems to be a more reliable way of detecting a physical scroll wheel. I am keeping the original implementation however because this new implementation won't work on the first event. I've given it a higher score deduction instead and MacOS / Windows still have original behaviour.

I've also added updated the tests with test data from my machine.

I was considering making a newer version of the classifier, see below. Though it wasn't entirely reliable. This seems to be a rabbit hole that I would rather stay out of so I opted for the approach above. It's a bit of a bummer that the type of input device is not available to JS. This has the code I was considering on adding...
/**
 * Singleton that determines whether the currently used scrolling device is a
 * physical mouse wheel or not. Important to determine whether or not smooth
 * scrolling should be used. A touchpad does not want smooth scrolling as it
 * should be smooth by itself.
 *
 * Because there is no easy way to determine whether the attached device is
 * a scroll wheel or not. We depend on the heuristic that the amount of events
 * a touchpad produces in a certain time is way more than a mouse wheel does as
 * that amount of events leads to the smooth scrolling thus it not needing
 * additional smooth scrolling.
 *
 * It therefore records every `TIMESTAMP_INTERVAL` and if the amoun of time
 * between each interval is less than `INVERVAL_DURATION_MS` it is considered
 * to be a touchpad.
 */
export class MouseWheelClassifierV2 {
	/**
	 * Value of the singleton.
	 */
	public static readonly INSTANCE = new MouseWheelClassifierV2();

	/**
	 * How many events to skip before checking the time difference between the
	 * last event and the current event.
	 */
	public static readonly TIMESTAMP_INTERVAL = 5;
	/**
	 * The amount of time in milliseconds between each `TIMESTAMP_INTERVAL` to
	 * consider the device to be a touchpad.
	 */
	public static readonly INVERVAL_DURATION_MS = 50;

	/**
	 * Last recorded timestamp.
	 */
	public lastTimestamp: number = 0;

	/**
	 * Default it to -1 so the first time an event comes in, it immediately sets
	 * `lastElapsedAmountTimestamp` to the current timestamp.
	 */
	public eventCounter: number = -1;
	public minDeltaX: number = Number.MAX_VALUE;
	public maxDeltaX: number = 0;
	public minDeltaY: number = Number.MAX_VALUE;
	public maxDeltaY: number = 0;
	/**
	 * We start out assuming that the device is a physical mouse wheel as then
	 * we would smooth scroll for the first `INVERVAL_DURATION_MS` milliseconds.
	 * Which will barely be perceptible for touchpad users but that first
	 * scroll that is not smooth will be perceptible to mouse wheel users.
	 */
	private _isPhysicalMouseWheel: boolean = true;

	/**
	 * Whether the device is a physical mouse wheel or not.
	 */
	public isPhysicalMouseWheel(): boolean {
		return this._isPhysicalMouseWheel;
	}

	public acceptStandardWheelEvent(e: StandardWheelEvent): void {
		// console.log(`acceptStandardWheelEvent: [${Date.now()}, ${e.deltaY}, ${e.deltaX}]`);
		this.accept(Date.now(), e.deltaX, e.deltaY);
	}


	public accept(timestamp: number, deltaX: number, deltaY: number): void {
		this.eventCounter = (this.eventCounter + 1) % MouseWheelClassifierV2.TIMESTAMP_INTERVAL;
		if (this.eventCounter === 0) {
			// console.log(`this.maxDeltaX: ${this.maxDeltaX}`);
			// console.log(`this.minDeltaX: ${this.minDeltaX}`);
			// console.log(`this.maxDeltaX % this.minDeltaX: ${this.maxDeltaX % this.minDeltaX}`);
			// console.log(`(this.maxDeltaX % this.minDeltaX) === 0: ${(this.maxDeltaX % this.minDeltaX) === 0}`);

			// console.log(`this.maxDeltaY: ${this.maxDeltaY}`);
			// console.log(`this.minDeltaY: ${this.minDeltaY}`);
			// console.log(`this.maxDeltaY % this.minDeltaY: ${this.maxDeltaY % this.minDeltaY}`);
			// console.log(`(this.maxDeltaY % this.minDeltaY) === 0: ${(this.maxDeltaY % this.minDeltaY) === 0}`);

			const eventSpeedLikelyScrollwheel = timestamp - this.lastTimestamp > MouseWheelClassifierV2.INVERVAL_DURATION_MS;
			const isXSameOrder = (this.maxDeltaX % this.minDeltaX) === 0;
			const isYSameOrder = (this.maxDeltaY % this.minDeltaY) === 0;

			// console.log(`eventSpeedLikelyScrollwheel: ${eventSpeedLikelyScrollwheel}`);
			// console.log(`isXSameOrder: ${isXSameOrder}`);
			// console.log(`isYSameOrder: ${isYSameOrder}`);
			this._isPhysicalMouseWheel = eventSpeedLikelyScrollwheel && (
				isXSameOrder && isYSameOrder
			);
			this.lastTimestamp = timestamp;


			this.minDeltaX = Number.MAX_VALUE;
			this.maxDeltaX = 0;
			this.minDeltaY = Number.MAX_VALUE;
			this.maxDeltaY = 0;
		}
		const absDeltaX = Math.abs(deltaX);
		this.minDeltaX = Math.max(Math.min(this.minDeltaX, absDeltaX), 1);
		this.maxDeltaX = Math.max(this.maxDeltaX, absDeltaX);
		// console.log(`absDeltaX: ${absDeltaX}`);
		// console.log(`this.maxDeltaX: ${this.maxDeltaX}`);
		// console.log(`this.minDeltaX: ${this.minDeltaX}`);

		const absDeltaY = Math.abs(deltaY);
		this.minDeltaY = Math.max(Math.min(this.minDeltaY, absDeltaY), 1);
		this.maxDeltaY = Math.max(this.maxDeltaY, absDeltaY);
		// console.log(`absDeltaY: ${absDeltaY}`);
		// console.log(`this.maxDeltaY: ${this.maxDeltaY}`);
		// console.log(`this.minDeltaY: ${this.minDeltaY}`);
		// console.log('');
	}
}

@AndreasBackx AndreasBackx changed the title Fix smooth scrolling Linux. Fix smooth scrolling Linux Wayland. Feb 13, 2024
@deepak1556
Copy link
Contributor

Thanks for working on this, I have a follow-up question

its base is 1.5 for some reason instead of for example 1 or 2

Would it be related to the fractional scaling support on wayland ? Does the current integer based smooth scrolling work fine in your setup when the scale factor is forced via code --ozone-platform=wayland --force-device-scale-factor=1

How does the scroll delta values look like in x11 for comparison, does it also support fractional scaling ?

@alexdima alexdima enabled auto-merge (squash) March 14, 2024 07:54
@alexdima alexdima added this to the March 2024 milestone Mar 14, 2024
@alexdima
Copy link
Member

Thank you!

@alexdima alexdima merged commit 5bc9d1d into microsoft:main Mar 14, 2024
6 checks passed
@AndreasBackx
Copy link
Contributor Author

Thank you for merging this @alexdima and @sandy081! Apologies @deepak1556 for not replying earlier, life has been busy, but when is it not?

Thanks for working on this, I have a follow-up question

its base is 1.5 for some reason instead of for example 1 or 2

Would it be related to the fractional scaling support on wayland ? Does the current integer based smooth scrolling work fine in your setup when the scale factor is forced via code --ozone-platform=wayland --force-device-scale-factor=1

It might, though my fractional scaling is set to 1.25, not 1.5. 🤔 I'm currently leaving to visit family for a short while so wouldn't be able to test this unfortunately. I'd be interested myself if you were to have the time.

How does the scroll delta values look like in x11 for comparison, does it also support fractional scaling ?

The X11 scroll deltas were rounded like the other test values. I was going to add some test values for those, but did not end up having the time unfortunately.

@AndreasBackx AndreasBackx deleted the fix/wayland-smooth-scroll branch March 16, 2024 01:55
@caleb-allen
Copy link

@AndreasBackx does this modify mouse-wheel scrolling only?

When scrolling with a touchpad, vs-code does not have "momentum" like with chrome, firefox, etc. but perhaps that's not related to this smooth-scrolling? At least, scrolling with the touchpad has the same behavior before and after this PR.

@caleb-allen
Copy link

Ah yes, I am talking about "kinetic scrolling", there's an issue here: #201937

Sorry for the noise!

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

Successfully merging this pull request may close these issues.

Wayland smooth scrolling enabled not respected
5 participants