Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Web trackpad pan #36346

Merged
auto-submit merged 13 commits into from
Dec 12, 2022
Merged

Web trackpad pan #36346

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8f04c00
Guess at trackpad pans on web
moffatman Sep 22, 2022
ad18dee
Add test
moffatman Sep 22, 2022
77c8347
Update comment
moffatman Oct 2, 2022
34f091b
Handle macOS accelerated scroll wheel
moffatman Oct 27, 2022
41a7f2e
Fix test after last commit
moffatman Oct 28, 2022
c5b987e
Disable on firefox
moffatman Oct 28, 2022
50c21a0
Pull out _isTrackpadEvent and add doc links
moffatman Nov 1, 2022
0f2400e
Fix issue with floating point / integer conversion error.
moffatman Nov 12, 2022
31d04af
Workaround for magic mouse events which happen to be divisible by 120.
moffatman Nov 20, 2022
85a0a58
Refactor to handle bad luck in accelerated mouse deltas.
moffatman Nov 22, 2022
426cac0
Use 120 wheelDelta to identify mouse-accelerated events instead of 240
moffatman Nov 29, 2022
a07c922
Handle multiple bad-luck events in a row.
moffatman Dec 4, 2022
48b93bd
Cleanup parameters
moffatman Dec 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/web_ui/lib/src/engine/pointer_binding.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ abstract class _BaseAdapter {
final PointerDataConverter _pointerDataConverter;
final KeyboardConverter _keyboardConverter;
DomWheelEvent? _lastWheelEvent;
bool? _lastWheelEventWasTrackpad;
Copy link
Contributor

@justinmc justinmc Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: What do you think about making this non-nullable and initializing it to false? Currently it probably represents reality more accurately as-is, because a value of null means that there was no last wheel event. However, that null state is not used anywhere.

Up to you what you think is better.

Copy link
Contributor Author

@moffatman moffatman Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have it this way as in some previous iterations the null state did matter.


/// Each subclass is expected to override this method to attach its own event
/// listeners and convert events into pointer events.
Expand Down Expand Up @@ -349,7 +350,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter {
return (wheelDelta - (-3 * delta)).abs() > 1;
}

bool _isTrackpadEvent(DomWheelEvent event, DomWheelEvent? lastEvent) {
bool _isTrackpadEvent(DomWheelEvent event, DomWheelEvent? lastEvent, bool? lastEventWasTrackpad) {
Copy link
Contributor

@justinmc justinmc Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use _lastWheelEventWasTrackpad directly instead of passing it in?

Copy link
Contributor

@justinmc justinmc Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, now that _isTrackpadEvent doesn't call itself, maybe you don't need to pass in _lastWheelEvent either.

Copy link
Contributor Author

@moffatman moffatman Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, I'll change it to just use the members directly.

// This function relies on deprecated and non-standard implementation
// details. Useful reference material can be found below.
//
Expand Down Expand Up @@ -389,7 +390,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter {
// it was preceded within 50 milliseconds by a trackpad event. This
// handles unlucky 120-delta trackpad events during rapid movement.
final num diffMs = event.timeStamp! - lastEvent!.timeStamp!;
if (diffMs < 50 && _isTrackpadEvent(lastEvent, null)) {
if (diffMs < 50 && (lastEventWasTrackpad ?? false)) {
return true;
}
}
Expand All @@ -407,7 +408,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter {
const int domDeltaPage = 0x02;

ui.PointerDeviceKind kind = ui.PointerDeviceKind.mouse;
if (_isTrackpadEvent(event, _lastWheelEvent)) {
if (_isTrackpadEvent(event, _lastWheelEvent, _lastWheelEventWasTrackpad)) {
kind = ui.PointerDeviceKind.trackpad;
}

Expand Down Expand Up @@ -454,6 +455,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter {
scrollDeltaY: deltaY,
);
_lastWheelEvent = event;
_lastWheelEventWasTrackpad = kind == ui.PointerDeviceKind.trackpad;
return data;
}

Expand Down
90 changes: 68 additions & 22 deletions lib/web_ui/test/engine/pointer_binding_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,7 @@ void testMain() {
deltaY: 119,
wheelDeltaX: -357,
wheelDeltaY: -357,
timeStamp: 0,
));

glassPane.dispatchEvent(context.wheel(
Expand All @@ -1183,6 +1184,18 @@ void testMain() {
deltaY: 120,
wheelDeltaX: -360,
wheelDeltaY: -360,
timeStamp: 10,
));

glassPane.dispatchEvent(context.wheel(
buttons: 0,
clientX: 10,
clientY: 10,
deltaX: 120,
deltaY: 120,
wheelDeltaX: -360,
wheelDeltaY: -360,
timeStamp: 20,
));

glassPane.dispatchEvent(context.wheel(
Expand All @@ -1193,7 +1206,7 @@ void testMain() {
deltaY: 119,
wheelDeltaX: -357,
wheelDeltaY: -357,
timeStamp: 10,
timeStamp: 1000,
));

glassPane.dispatchEvent(context.wheel(
Expand All @@ -1204,7 +1217,7 @@ void testMain() {
deltaY: -120,
wheelDeltaX: 360,
wheelDeltaY: 360,
timeStamp: 20,
timeStamp: 1010,
));

glassPane.dispatchEvent(context.wheel(
Expand All @@ -1215,6 +1228,7 @@ void testMain() {
deltaY: -120,
wheelDeltaX: 0,
wheelDeltaY: 360,
timeStamp: 2000,
));

glassPane.dispatchEvent(context.wheel(
Expand All @@ -1225,9 +1239,10 @@ void testMain() {
deltaY: 40,
wheelDeltaX: 0,
wheelDeltaY: -360,
timeStamp: 3000,
));

expect(packets, hasLength(6));
expect(packets, hasLength(7));

// An add will be synthesized.
expect(packets[0].data, hasLength(2));
Expand Down Expand Up @@ -1270,8 +1285,8 @@ void testMain() {
expect(packets[1].data[0].scrollDeltaX, equals(120.0));
expect(packets[1].data[0].scrollDeltaY, equals(120.0));

// Because the delta is not in increments of 120 and has matching wheelDelta,
// it will be a trackpad event.
// Because the delta is in increments of 120, but is again similar to the
// previous event, it will be a trackpad event.
expect(packets[2].data[0].change, equals(ui.PointerChange.hover));
expect(
packets[2].data[0].signalKind, equals(ui.PointerSignalKind.scroll));
Expand All @@ -1283,12 +1298,11 @@ void testMain() {
expect(packets[2].data[0].physicalY, equals(10.0 * dpi));
expect(packets[2].data[0].physicalDeltaX, equals(0.0));
expect(packets[2].data[0].physicalDeltaY, equals(0.0));
expect(packets[2].data[0].scrollDeltaX, equals(119.0));
expect(packets[2].data[0].scrollDeltaY, equals(119.0));
expect(packets[2].data[0].scrollDeltaX, equals(120.0));
expect(packets[2].data[0].scrollDeltaY, equals(120.0));

// Because the delta is in increments of 120, and is not similar to the
// previous event, but occured soon after the previous event, it will be
// a trackpad event.
// Because the delta is not in increments of 120 and has matching wheelDelta,
// it will be a trackpad event.
expect(packets[3].data[0].change, equals(ui.PointerChange.hover));
expect(
packets[3].data[0].signalKind, equals(ui.PointerSignalKind.scroll));
Expand All @@ -1300,28 +1314,28 @@ void testMain() {
expect(packets[3].data[0].physicalY, equals(10.0 * dpi));
expect(packets[3].data[0].physicalDeltaX, equals(0.0));
expect(packets[3].data[0].physicalDeltaY, equals(0.0));
expect(packets[3].data[0].scrollDeltaX, equals(-120.0));
expect(packets[3].data[0].scrollDeltaY, equals(-120.0));
expect(packets[3].data[0].scrollDeltaX, equals(119.0));
expect(packets[3].data[0].scrollDeltaY, equals(119.0));

// Because the delta is in increments of 120, and is not similar to
// the previous event, and does not have a timestamp, it will be a mouse event.
expect(packets[4].data, hasLength(1));
// Because the delta is in increments of 120, and is not similar to the
// previous event, but occured soon after the previous event, it will be
// a trackpad event.
expect(packets[4].data[0].change, equals(ui.PointerChange.hover));
expect(
packets[4].data[0].signalKind, equals(ui.PointerSignalKind.scroll));
expect(
packets[4].data[0].kind, equals(ui.PointerDeviceKind.mouse));
packets[4].data[0].kind, equals(ui.PointerDeviceKind.trackpad));
expect(packets[4].data[0].pointerIdentifier, equals(0));
expect(packets[4].data[0].synthesized, isFalse);
expect(packets[4].data[0].physicalX, equals(10.0 * dpi));
expect(packets[4].data[0].physicalY, equals(10.0 * dpi));
expect(packets[4].data[0].physicalDeltaX, equals(0.0));
expect(packets[4].data[0].physicalDeltaY, equals(0.0));
expect(packets[4].data[0].scrollDeltaX, equals(0.0));
expect(packets[4].data[0].scrollDeltaX, equals(-120.0));
expect(packets[4].data[0].scrollDeltaY, equals(-120.0));

// Because the delta is not in increments of 120 and has non-matching
// wheelDelta, it will be a mouse event.
// Because the delta is in increments of 120, and is not similar to
// the previous event, and occured long after the previous event, it will be a mouse event.
expect(packets[5].data, hasLength(1));
expect(packets[5].data[0].change, equals(ui.PointerChange.hover));
expect(
Expand All @@ -1335,7 +1349,24 @@ void testMain() {
expect(packets[5].data[0].physicalDeltaX, equals(0.0));
expect(packets[5].data[0].physicalDeltaY, equals(0.0));
expect(packets[5].data[0].scrollDeltaX, equals(0.0));
expect(packets[5].data[0].scrollDeltaY, equals(40.0));
expect(packets[5].data[0].scrollDeltaY, equals(-120.0));

// Because the delta is not in increments of 120 and has non-matching
// wheelDelta, it will be a mouse event.
expect(packets[6].data, hasLength(1));
expect(packets[6].data[0].change, equals(ui.PointerChange.hover));
expect(
packets[6].data[0].signalKind, equals(ui.PointerSignalKind.scroll));
expect(
packets[6].data[0].kind, equals(ui.PointerDeviceKind.mouse));
expect(packets[6].data[0].pointerIdentifier, equals(0));
expect(packets[6].data[0].synthesized, isFalse);
expect(packets[6].data[0].physicalX, equals(10.0 * dpi));
expect(packets[6].data[0].physicalY, equals(10.0 * dpi));
expect(packets[6].data[0].physicalDeltaX, equals(0.0));
expect(packets[6].data[0].physicalDeltaY, equals(0.0));
expect(packets[6].data[0].scrollDeltaX, equals(0.0));
expect(packets[6].data[0].scrollDeltaY, equals(40.0));
},
);

Expand Down Expand Up @@ -3061,13 +3092,28 @@ mixin _ButtonedEventMixin on _BasicEventContext {
'deltaY': deltaY,
'wheelDeltaX': wheelDeltaX,
'wheelDeltaY': wheelDeltaY,
'timeStamp': timeStamp,
}
];
return js_util.callConstructor<DomEvent>(
final DomEvent event = js_util.callConstructor<DomEvent>(
jsWheelEvent,
js_util.jsify(eventArgs) as List<Object?>,
);
// timeStamp can't be set in the constructor, need to override the getter.
if (timeStamp != null) {
js_util.callMethod(
objectConstructor,
'defineProperty',
<dynamic>[
event,
'timeStamp',
js_util.jsify(<String, dynamic>{
'value': timeStamp,
'configurable': true
})
]
);
}
return event;
}
}

Expand Down