From e69319216729b5af01d642fc78e502058737cc99 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 9 Oct 2019 12:43:06 -0700 Subject: [PATCH] [web_ui] Check if a pointer is already down for the specific device (#12470) * WIP on fixing pointer event clicks * Don't set canvaskit as enabled by default * Use a Set of pressed buttons rather than a Map of button status to avoid memory leaks * Add test for pointer binding Fix bug where event listeners weren't properly removed --- .../lib/src/engine/pointer_binding.dart | 80 +++++++++++---- .../test/engine/pointer_binding_test.dart | 98 +++++++++++++++++++ 2 files changed, 159 insertions(+), 19 deletions(-) create mode 100644 lib/web_ui/test/engine/pointer_binding_test.dart diff --git a/lib/web_ui/lib/src/engine/pointer_binding.dart b/lib/web_ui/lib/src/engine/pointer_binding.dart index ef25b617443..7a67c14588f 100644 --- a/lib/web_ui/lib/src/engine/pointer_binding.dart +++ b/lib/web_ui/lib/src/engine/pointer_binding.dart @@ -101,6 +101,24 @@ class PointerSupportDetector { 'pointers:$hasPointerEvents, touch:$hasTouchEvents, mouse:$hasMouseEvents'; } +class _PressedButton { + const _PressedButton(this.deviceId, this.button); + + // The id of the device pressing the button. + final int deviceId; + + // The id of the button being pressed. + final int button; + + bool operator ==(other) { + if (other is! _PressedButton) return false; + final _PressedButton otherButton = other; + return deviceId == otherButton.deviceId && button == otherButton.button; + } + + int get hashCode => ((13801 + deviceId) * 37) + button; +} + /// Common functionality that's shared among adapters. abstract class BaseAdapter { static final Map _listeners = @@ -108,13 +126,20 @@ abstract class BaseAdapter { final DomRenderer domRenderer; PointerDataCallback _callback; - Map _isDownMap = {}; - bool _isButtonDown(int button) { - return _isDownMap[button] == true; + + // A set of the buttons that are currently being pressed. + Set<_PressedButton> _pressedButtons = Set<_PressedButton>(); + + bool _isButtonDown(int device, int button) { + return _pressedButtons.contains(_PressedButton(device, button)); } - void _updateButtonDownState(int button, bool value) { - _isDownMap[button] = value; + void _updateButtonDownState(int device, int button, bool value) { + if (value) { + _pressedButtons.add(_PressedButton(device, button)); + } else { + _pressedButtons.remove(_PressedButton(device, button)); + } } BaseAdapter(this._callback, this.domRenderer) { @@ -129,7 +154,7 @@ abstract class BaseAdapter { void clearListeners() { final html.Element glassPane = domRenderer.glassPaneElement; _listeners.forEach((String eventName, html.EventListener listener) { - glassPane.removeEventListener(eventName, listener); + glassPane.removeEventListener(eventName, listener, true); }); _listeners.clear(); } @@ -170,6 +195,14 @@ int _pointerButtonFromHtmlEvent(html.Event event) { return _kPrimaryMouseButton; } +int _deviceFromHtmlEvent(event) { + if (event is html.PointerEvent) { + final html.PointerEvent pointerEvent = event; + return pointerEvent.pointerId; + } + return _mouseDeviceId; +} + /// Adapter class to be used with browsers that support native pointer events. class PointerAdapter extends BaseAdapter { PointerAdapter(PointerDataCallback callback, DomRenderer domRenderer) @@ -179,12 +212,13 @@ class PointerAdapter extends BaseAdapter { void _setup() { _addEventListener('pointerdown', (html.Event event) { final int pointerButton = _pointerButtonFromHtmlEvent(event); - if (_isButtonDown(pointerButton)) { + final int device = _deviceFromHtmlEvent(event); + if (_isButtonDown(device, pointerButton)) { // TODO(flutter_web): Remove this temporary fix for right click // on web platform once context guesture is implemented. _callback(_convertEventToPointerData(ui.PointerChange.up, event)); } - _updateButtonDownState(pointerButton, true); + _updateButtonDownState(device, pointerButton, true); _callback(_convertEventToPointerData(ui.PointerChange.down, event)); }); @@ -195,8 +229,9 @@ class PointerAdapter extends BaseAdapter { // Change this when context gesture is implemented in flutter framework. final html.PointerEvent pointerEvent = event; final int pointerButton = _pointerButtonFromHtmlEvent(pointerEvent); + final int device = _deviceFromHtmlEvent(event); final List data = _convertEventToPointerData( - _isButtonDown(pointerButton) + _isButtonDown(device, pointerButton) ? ui.PointerChange.move : ui.PointerChange.hover, pointerEvent); @@ -214,10 +249,11 @@ class PointerAdapter extends BaseAdapter { // The pointer could have been released by a `pointerout` event, in which // case `pointerup` should have no effect. final int pointerButton = _pointerButtonFromHtmlEvent(event); - if (!_isButtonDown(pointerButton)) { + final int device = _deviceFromHtmlEvent(event); + if (!_isButtonDown(device, pointerButton)) { return; } - _updateButtonDownState(pointerButton, false); + _updateButtonDownState(device, pointerButton, false); _callback(_convertEventToPointerData(ui.PointerChange.up, event)); }); @@ -225,7 +261,8 @@ class PointerAdapter extends BaseAdapter { // be able to generate events (example: device is deactivated) _addEventListener('pointercancel', (html.Event event) { final int pointerButton = _pointerButtonFromHtmlEvent(event); - _updateButtonDownState(pointerButton, false); + final int device = _deviceFromHtmlEvent(event); + _updateButtonDownState(pointerButton, device, false); _callback(_convertEventToPointerData(ui.PointerChange.cancel, event)); }); @@ -308,13 +345,14 @@ class TouchAdapter extends BaseAdapter { @override void _setup() { _addEventListener('touchstart', (html.Event event) { - _updateButtonDownState(_kPrimaryMouseButton, true); + _updateButtonDownState( + _deviceFromHtmlEvent(event), _kPrimaryMouseButton, true); _callback(_convertEventToPointerData(ui.PointerChange.down, event)); }); _addEventListener('touchmove', (html.Event event) { event.preventDefault(); // Prevents standard overscroll on iOS/Webkit. - if (!_isButtonDown(_kPrimaryMouseButton)) { + if (!_isButtonDown(_deviceFromHtmlEvent(event), _kPrimaryMouseButton)) { return; } _callback(_convertEventToPointerData(ui.PointerChange.move, event)); @@ -324,7 +362,8 @@ class TouchAdapter extends BaseAdapter { // On Safari Mobile, the keyboard does not show unless this line is // added. event.preventDefault(); - _updateButtonDownState(_kPrimaryMouseButton, false); + _updateButtonDownState( + _deviceFromHtmlEvent(event), _kPrimaryMouseButton, false); _callback(_convertEventToPointerData(ui.PointerChange.up, event)); }); @@ -372,19 +411,21 @@ class MouseAdapter extends BaseAdapter { void _setup() { _addEventListener('mousedown', (html.Event event) { final int pointerButton = _pointerButtonFromHtmlEvent(event); - if (_isButtonDown(pointerButton)) { + final int device = _deviceFromHtmlEvent(event); + if (_isButtonDown(device, pointerButton)) { // TODO(flutter_web): Remove this temporary fix for right click // on web platform once context guesture is implemented. _callback(_convertEventToPointerData(ui.PointerChange.up, event)); } - _updateButtonDownState(pointerButton, true); + _updateButtonDownState(device, pointerButton, true); _callback(_convertEventToPointerData(ui.PointerChange.down, event)); }); _addEventListener('mousemove', (html.Event event) { final int pointerButton = _pointerButtonFromHtmlEvent(event); + final int device = _deviceFromHtmlEvent(event); final List data = _convertEventToPointerData( - _isButtonDown(pointerButton) + _isButtonDown(device, pointerButton) ? ui.PointerChange.move : ui.PointerChange.hover, event); @@ -392,7 +433,8 @@ class MouseAdapter extends BaseAdapter { }); _addEventListener('mouseup', (html.Event event) { - _updateButtonDownState(_pointerButtonFromHtmlEvent(event), false); + final int device = _deviceFromHtmlEvent(event); + _updateButtonDownState(device, _pointerButtonFromHtmlEvent(event), false); _callback(_convertEventToPointerData(ui.PointerChange.up, event)); }); diff --git a/lib/web_ui/test/engine/pointer_binding_test.dart b/lib/web_ui/test/engine/pointer_binding_test.dart new file mode 100644 index 00000000000..0f4dee1f61f --- /dev/null +++ b/lib/web_ui/test/engine/pointer_binding_test.dart @@ -0,0 +1,98 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:html' as html; +import 'dart:typed_data'; + +import 'package:ui/src/engine.dart'; +import 'package:ui/ui.dart' as ui; + +import 'package:test/test.dart'; + +void main() { + group('Pointer Binding', () { + html.Element glassPane = domRenderer.glassPaneElement; + + setUp(() { + // Touching domRenderer creates PointerBinding.instance. + domRenderer; + + // Set a new detector to reset the state of the listeners. + PointerBinding.instance.debugOverrideDetector(TestPointerDetector()); + + ui.window.onPointerDataPacket = null; + }); + + test('can receive pointer events on the glass pane', () { + ui.PointerDataPacket receivedPacket; + ui.window.onPointerDataPacket = (ui.PointerDataPacket packet) { + receivedPacket = packet; + }; + + glassPane.dispatchEvent(html.PointerEvent('pointerdown', { + 'pointerId': 1, + 'button': 1, + })); + + expect(receivedPacket, isNotNull); + expect(receivedPacket.data[0].device, equals(1)); + }); + + test('synthesizes a pointerup event on two pointerdowns in a row', () { + List packets = []; + ui.window.onPointerDataPacket = (ui.PointerDataPacket packet) { + packets.add(packet); + }; + + glassPane.dispatchEvent(html.PointerEvent('pointerdown', { + 'pointerId': 1, + 'button': 1, + })); + + glassPane.dispatchEvent(html.PointerEvent('pointerdown', { + 'pointerId': 1, + 'button': 1, + })); + + expect(packets, hasLength(3)); + expect(packets[0].data[0].change, equals(ui.PointerChange.down)); + expect(packets[1].data[0].change, equals(ui.PointerChange.up)); + expect(packets[2].data[0].change, equals(ui.PointerChange.down)); + }); + + test('does not synthesize pointer up if from different device', () { + List packets = []; + ui.window.onPointerDataPacket = (ui.PointerDataPacket packet) { + packets.add(packet); + }; + + glassPane.dispatchEvent(html.PointerEvent('pointerdown', { + 'pointerId': 1, + 'button': 1, + })); + + glassPane.dispatchEvent(html.PointerEvent('pointerdown', { + 'pointerId': 2, + 'button': 1, + })); + + expect(packets, hasLength(2)); + expect(packets[0].data[0].change, equals(ui.PointerChange.down)); + expect(packets[0].data[0].device, equals(1)); + expect(packets[1].data[0].change, equals(ui.PointerChange.down)); + expect(packets[1].data[0].device, equals(2)); + }); + }); +} + +class TestPointerDetector extends PointerSupportDetector { + @override + final bool hasPointerEvents = true; + + @override + final bool hasTouchEvents = false; + + @override + final bool hasMouseEvents = false; +}