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

Conversation

jonahwilliams
Copy link
Contributor

Reverts #39267

This change is causing the framework to fail its analysis checks because a switch over the PointerSignalKind is no longer exhaustive due to the addition of stylusAuxiliaryAction.

@flutter-dashboard flutter-dashboard bot added embedder Related to the embedder API platform-android platform-ios platform-web Code specifically for the web engine labels Feb 14, 2023
@jonahwilliams
Copy link
Contributor Author

@jonahwilliams
Copy link
Contributor Author

This looks like the failure is only in one place, so if you have a patch ready we could quickly land that.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 14, 2023
@hellohuanlin
Copy link
Contributor

Interesting. May have to look into git history and see how other fields are added.

@jonahwilliams
Copy link
Contributor Author

Generally the way you would do it is to:

  1. add a default case to the switch statement with the behavior you expect the new enum to have.
  2. Land the engine change that adds the new case
  3. Once that engine change has rolled into the framework, remove the default case and add the case for the new variant.

@hellohuanlin
Copy link
Contributor

@jonahwilliams that makes sense!

@auto-submit auto-submit bot merged commit c4f51bc into main Feb 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 14, 2023
@zanderso zanderso deleted the revert-39267-louisehsu/pencil-double-tap branch February 14, 2023 03:15
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 14, 2023
…120656)

* 8cd648d1d Roll Dart SDK from f80c5db8736a to ea59504416a8 (1 revision) (flutter/engine#39594)

* 9ac09ced1 [Impeller] Fix unsafe access for clip stencil coverage (flutter/engine#39595)

* 99a81a81f Add support for double tap action from Apple Pencil 2 (flutter/engine#39267)

* 89d41d13e Add unique device id for trackpad on web (flutter/engine#39260)

* f7dfb2b63 remove use of SkCanvas and DLCanvasRecorder from ui.Canvas native code (flutter/engine#39599)

* c2e165e36 Fix multi-function compute (flutter/engine#39603)

* c4f51bc78 Revert "Add support for double tap action from Apple Pencil 2 (#39267)" (flutter/engine#39607)
@hellohuanlin
Copy link
Contributor

@jonahwilliams im wondering how do we avoid this break in the future? maybe having some basic framework test in the engine CI?

@jmagman
Copy link
Member

jmagman commented Feb 14, 2023

@jonahwilliams im wondering how do we avoid this break in the future? maybe having some basic framework test in the engine CI?

It was caught during a engine -> framework roll, which is the earliest these kinds of failures get caught at the moment.
flutter/flutter#120646

There are some web tests that run framework tests, they clone the newest version of the framework that's older than the engine version, it's very complicated and somewhat error prone go/flutter-web-engine-break https://github.com/flutter/engine/blob/master/tools/clone_flutter.sh

@hellohuanlin
Copy link
Contributor

Gotcha. it sounds like mono-repo maybe something we want in the future where engine and framework share the same repo.

auto-submit bot pushed a commit that referenced this pull request Feb 15, 2023
0xZOne pushed a commit to 0xZOne/engine that referenced this pull request Feb 20, 2023
LouiseHsu added a commit that referenced this pull request Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App embedder Related to the embedder API platform-android platform-ios platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants