From ba4cf054bded4975632ca0a0b2d9699a9e8ca5f7 Mon Sep 17 00:00:00 2001 From: Stanislav Baranov Date: Mon, 5 Nov 2018 12:43:00 -0800 Subject: [PATCH] Propagate positions of secondary pointers in UP events on Android (#23797) --- bin/internal/engine.version | 2 +- .../android_views/lib/motion_event_diff.dart | 23 ----------------- .../flutter/lib/src/gestures/converter.dart | 3 ++- packages/flutter/lib/src/gestures/events.dart | 7 ++++++ .../lib/src/rendering/platform_view.dart | 9 +++++-- .../test/widgets/widget_inspector_test.dart | 25 +++++++++++++++++++ 6 files changed, 42 insertions(+), 27 deletions(-) diff --git a/bin/internal/engine.version b/bin/internal/engine.version index 357a3e3c750..435c42e86d4 100644 --- a/bin/internal/engine.version +++ b/bin/internal/engine.version @@ -1 +1 @@ -88cb78cdf62ceb206e740853d3afd060bb8d781c +b5758d0d3848d8ed77308d97734b9a7933c3844f diff --git a/dev/integration_tests/android_views/lib/motion_event_diff.dart b/dev/integration_tests/android_views/lib/motion_event_diff.dart index e806cdb72f6..2c31f859ed2 100644 --- a/dev/integration_tests/android_views/lib/motion_event_diff.dart +++ b/dev/integration_tests/android_views/lib/motion_event_diff.dart @@ -95,23 +95,6 @@ void diffPointerCoordsList(StringBuffer diffBuffer, return; } - if (isSinglePointerAction(originalEvent['action'])) { - final int idx = getPointerIdx(originalEvent['action']); - final Map expected = - expectedList[idx].cast(); - final Map actual = actualList[idx].cast(); - diffPointerCoords(expected, actual, idx, diffBuffer); - // For POINTER_UP and POINTER_DOWN events the engine drops the data for all pointers - // but for the pointer that was taken up/down. - // See: https://github.com/flutter/flutter/issues/19882 - // - // Until that issue is resolved, we only compare the pointer for which the action - // applies to here. - // - // TODO(amirh): Compare all pointers once the issue mentioned above is resolved. - return; - } - for (int i = 0; i < expectedList.length; i++) { final Map expected = expectedList[i].cast(); @@ -151,12 +134,6 @@ void diffMaps( } } -bool isSinglePointerAction(int action) { - final int actionMasked = getActionMasked(action); - return actionMasked == 5 || // POINTER_DOWN - actionMasked == 6; // POINTER_UP -} - int getActionMasked(int action) => action & 0xff; int getPointerIdx(int action) => (action >> 8) & 0xff; diff --git a/packages/flutter/lib/src/gestures/converter.dart b/packages/flutter/lib/src/gestures/converter.dart index f0aa36983b3..aea2b7905c2 100644 --- a/packages/flutter/lib/src/gestures/converter.dart +++ b/packages/flutter/lib/src/gestures/converter.dart @@ -239,7 +239,8 @@ class PointerEventConverter { radiusMin: radiusMin, radiusMax: radiusMax, orientation: datum.orientation, - tilt: datum.tilt + tilt: datum.tilt, + platformData: datum.platformData, ); break; case ui.PointerChange.up: diff --git a/packages/flutter/lib/src/gestures/events.dart b/packages/flutter/lib/src/gestures/events.dart index df832589305..6fb4294e473 100644 --- a/packages/flutter/lib/src/gestures/events.dart +++ b/packages/flutter/lib/src/gestures/events.dart @@ -115,6 +115,7 @@ abstract class PointerEvent { this.radiusMax = 0.0, this.orientation = 0.0, this.tilt = 0.0, + this.platformData = 0, this.synthesized = false, }); @@ -245,6 +246,9 @@ abstract class PointerEvent { /// the stylus is flat on that surface). final double tilt; + /// Opaque platform-specific data associated with the event. + final int platformData; + /// We occasionally synthesize PointerEvents that aren't exact translations /// of [ui.PointerData] from the engine to cover small cross-OS discrepancies /// in pointer behaviors. @@ -285,6 +289,7 @@ abstract class PointerEvent { 'radiusMax: $radiusMax, ' 'orientation: $orientation, ' 'tilt: $tilt, ' + 'platformData: $platformData, ' 'synthesized: $synthesized' ')'; } @@ -495,6 +500,7 @@ class PointerMoveEvent extends PointerEvent { double radiusMax = 0.0, double orientation = 0.0, double tilt = 0.0, + int platformData = 0, bool synthesized = false, }) : super( timeStamp: timeStamp, @@ -518,6 +524,7 @@ class PointerMoveEvent extends PointerEvent { radiusMax: radiusMax, orientation: orientation, tilt: tilt, + platformData: platformData, synthesized: synthesized, ); } diff --git a/packages/flutter/lib/src/rendering/platform_view.dart b/packages/flutter/lib/src/rendering/platform_view.dart index 3e4303b2876..f4d70aba761 100644 --- a/packages/flutter/lib/src/rendering/platform_view.dart +++ b/packages/flutter/lib/src/rendering/platform_view.dart @@ -447,16 +447,21 @@ class _MotionEventsDispatcher { final int pointerIdx = pointers.indexOf(event.pointer); final int numPointers = pointers.length; + // This value must match the value in engine's FlutterView.java. + // This flag indicates whether the original Android pointer events were batched together. + const int kPointerDataFlagBatched = 1; + // Android MotionEvent objects can batch information on multiple pointers. // Flutter breaks these such batched events into multiple PointerEvent objects. // When there are multiple active pointers we accumulate the information for all pointers // as we get PointerEvents, and only send it to the embedded Android view when // we see the last pointer. This way we achieve the same batching as Android. - if(isSinglePointerAction(event) && pointerIdx < numPointers - 1) + if (event.platformData == kPointerDataFlagBatched || + (isSinglePointerAction(event) && pointerIdx < numPointers - 1)) return; int action; - switch(event.runtimeType){ + switch (event.runtimeType) { case PointerDownEvent: action = numPointers == 1 ? AndroidViewController.kActionDown : AndroidViewController.pointerAction(pointerIdx, AndroidViewController.kActionPointerDown); diff --git a/packages/flutter/test/widgets/widget_inspector_test.dart b/packages/flutter/test/widgets/widget_inspector_test.dart index 88c0f680e1c..8b77bb2646c 100644 --- a/packages/flutter/test/widgets/widget_inspector_test.dart +++ b/packages/flutter/test/widgets/widget_inspector_test.dart @@ -1806,6 +1806,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { await expectLater( layer.toImage(renderObject.semanticBounds.inflate(50.0)), matchesGoldenFile('inspector.repaint_boundary_margin.png'), + skip: !Platform.isLinux, ); // Regression test for how rendering with a pixel scale other than 1.0 @@ -1816,6 +1817,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { pixelRatio: 0.5, ), matchesGoldenFile('inspector.repaint_boundary_margin_small.png'), + skip: !Platform.isLinux, ); await expectLater( @@ -1824,6 +1826,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { pixelRatio: 2.0, ), matchesGoldenFile('inspector.repaint_boundary_margin_large.png'), + skip: !Platform.isLinux, ); final Layer layerParent = layer.parent; @@ -1839,6 +1842,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { height: 300.0, ), matchesGoldenFile('inspector.repaint_boundary.png'), + skip: !Platform.isLinux, ); // Verify that taking a screenshot didn't change the layers associated with @@ -1856,6 +1860,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { margin: 50.0, ), matchesGoldenFile('inspector.repaint_boundary_margin.png'), + skip: !Platform.isLinux, ); // Verify that taking a screenshot didn't change the layers associated with @@ -1876,6 +1881,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { debugPaint: true, ), matchesGoldenFile('inspector.repaint_boundary_debugPaint.png'), + skip: !Platform.isLinux, ); // Verify that taking a screenshot with debug paint on did not change // the number of children the layer has. @@ -1886,6 +1892,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { await expectLater( find.byType(RepaintBoundaryWithDebugPaint), matchesGoldenFile('inspector.repaint_boundary.png'), + skip: !Platform.isLinux, ); expect(renderObject.debugLayer, equals(layer)); @@ -1899,6 +1906,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { height: 100.0, ), matchesGoldenFile('inspector.container.png'), + skip: !Platform.isLinux, ); await expectLater( @@ -1909,6 +1917,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { debugPaint: true, ), matchesGoldenFile('inspector.container_debugPaint.png'), + skip: !Platform.isLinux, ); { @@ -1929,6 +1938,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { debugPaint: true, ), matchesGoldenFile('inspector.container_debugPaint.png'), + skip: !Platform.isLinux, ); expect(container.debugNeedsLayout, isFalse); } @@ -1941,6 +1951,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { height: 100.0, ), matchesGoldenFile('inspector.container_small.png'), + skip: !Platform.isLinux, ); await expectLater( @@ -1951,6 +1962,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { maxPixelRatio: 3.0, ), matchesGoldenFile('inspector.container_large.png'), + skip: !Platform.isLinux, ); // This screenshot will show the clip rect debug paint but no other @@ -1963,6 +1975,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { debugPaint: true, ), matchesGoldenFile('inspector.clipRect_debugPaint.png'), + skip: !Platform.isLinux, ); final Element clipRect = find.byType(ClipRRect).evaluate().single; @@ -2007,6 +2020,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { await expectLater( screenshotImage, matchesReferenceImage(await clipRectScreenshot), + skip: !Platform.isLinux, ); // Test with a very visible debug paint @@ -2018,6 +2032,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { debugPaint: true, ), matchesGoldenFile('inspector.padding_debugPaint.png'), + skip: !Platform.isLinux, ); // The bounds for this box crop its rendered content. @@ -2029,6 +2044,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { debugPaint: true, ), matchesGoldenFile('inspector.sizedBox_debugPaint.png'), + skip: !Platform.isLinux, ); // Verify that setting a margin includes the previously cropped content. @@ -2041,6 +2057,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { debugPaint: true, ), matchesGoldenFile('inspector.sizedBox_debugPaint_margin.png'), + skip: !Platform.isLinux, ); }); @@ -2112,6 +2129,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { await expectLater( find.byKey(mainStackKey), matchesGoldenFile('inspector.composited_transform.only_offsets.png'), + skip: !Platform.isLinux, ); await expectLater( @@ -2121,11 +2139,13 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { height: 500.0, ), matchesGoldenFile('inspector.composited_transform.only_offsets_follower.png'), + skip: !Platform.isLinux, ); await expectLater( WidgetInspectorService.instance.screenshot(find.byType(Stack).evaluate().first, width: 300.0, height: 300.0), matchesGoldenFile('inspector.composited_transform.only_offsets_small.png'), + skip: !Platform.isLinux, ); await expectLater( @@ -2135,6 +2155,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { height: 500.0, ), matchesGoldenFile('inspector.composited_transform.only_offsets_target.png'), + skip: !Platform.isLinux, ); }); @@ -2207,6 +2228,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { await expectLater( find.byKey(mainStackKey), matchesGoldenFile('inspector.composited_transform.with_rotations.png'), + skip: !Platform.isLinux, ); await expectLater( @@ -2216,6 +2238,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { height: 500.0, ), matchesGoldenFile('inspector.composited_transform.with_rotations_small.png'), + skip: !Platform.isLinux, ); await expectLater( @@ -2225,6 +2248,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { height: 500.0, ), matchesGoldenFile('inspector.composited_transform.with_rotations_target.png'), + skip: !Platform.isLinux, ); await expectLater( @@ -2234,6 +2258,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService { height: 500.0, ), matchesGoldenFile('inspector.composited_transform.with_rotations_follower.png'), + skip: !Platform.isLinux, ); // Make sure taking screenshots hasn't modified the positions of the