â¦h iPad pencil getting stuck (#55724)"
This reverts commit 21eb44914c22761f3c8bdb39b1d36b2fc15bde3b.
Revert since a customer reported the platform view touch is broken after the change.
*List which issues are fixed by this PR. You must list at least one issue.*
Reopens https://github.com/flutter/engine/pull/55724
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Addresses https://github.com/flutter/flutter/issues/155541
This does **not** remove Skia itself from the build. I'll stage that in a followup. Want to keep the scope of this patch small.
This is the new behavior:
* Impeller is the default on iOS devices **and** simulators.
* On iOS devices **only**, all flags and plist options are ignored.
* On iOS simulators **only**, Flutter will used Impeller's Metal backend by default and fallback to the Null backend if Metal device access is not available.
* On iOS simulators **only**, Flutter can pick Skia using the command line flags or plist flags. This is to allow users one more month to migrate as communicated by @zanderso.
The settings objects `enable_impeller` field is now `static constexpr` to avoid accidentally disabling the flag while it still exists.
This is because GTK3 can't share contexts between GtkGLAreas.
We may be able to avoid the copy using the GL_OES_EGL_image or
GL_OES_EGL_image_external extensions.
Note that keyboard input doesn't work on the secondary view, this
requires moving the keyboard handling from FlView to FlEngine. Proposing
this now as a step forwards, but not yet useful for real applications.
The MockCanvas API was originally used to test the output of rendering methods to match them against a reference set of "MockData" structures. This API has been superseded by just using a DisplayListBuilder to record reference operations and then to compare the resulting DisplayList objects.
This PR deletes the last remaining uses of MockCanvas as well as the mechanism itself.
Reverts: flutter/engine#55445
Initiated by: zijiehe-google-com
Reason for reverting: This change would break the build_fuchsia_artifacts.py without https://github.com/flutter/engine/pull/55832/files. I'd merge two into one.
Original PR Author: zijiehe-google-com
Reviewed By: {jrwang}
This change reverts the following previous change:
This change removes the in-house built pm-based build rules in favor of the high level fuchsia_component / fuchsia_package in the gn-sdk.
The build_fuchsia_artifacts.py is still using pm, and it will be handled in a following change.
Bug: http://b/353729557, http://b/368608542
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
## Summary
I came across our the "gesture recognizer delegate" implementation and it is quite odd (see below). After fixing it, the problem is resolved. However, it's hard to reason about how it's related to iPad pencil, since it's internal logic that we don't know (see my research below).
## Gesture recognizer delegate
### Existing odd implementation
- shouldBeRequiredToFailByGestureRecognizer:
`otherGestureRecognizer != self` is always YES because the delegate set to self, hence `gestureRecognizer` must be self, hence `otherGestureRecognizer` must not be self.
- shouldRequireFailureOfGestureRecognizer:
`otherGestureRecognizer == self` is always NO, for the same reason described above.
### new implementation:
After digging into various PRs, the idea seems to be that we want to have a precedence of "Forwarding recognizer > Delaying recognizer > Other recognizers in platform view".
- shouldBeRequiredToFailByGestureRecognizer:
`return otherGestureRecognizer != _forwardingRecognizer` means Delaying recognizer needs to be higher precedence than all non-Forwarding recognizer. (aka "Delaying recognizer > Other recognizers in platform view")
- shouldRequireFailureOfGestureRecognizer:
`return otherGestureRecognizer == _forwardingRecognizer` means Delaying recognizer needs to have lower precedence than forwarding recognizer. (aka "Forwarding recognizer > Delaying recognizer").
## Some research
This is a tricky one since pencil and finger triggers exactly the same callbacks. It turns out that when pencil is involved after finger interaction, the platform view's "forwarding" gesture recognizer is stuck at failed state. This seems to be an iOS bug, because according to [the API doc](https://developer.apple.com/documentation/uikit/uigesturerecognizerstate/uigesturerecognizerstatefailed?language=objc), it should be reset back to "possible" state:
> No action message is sent and the gesture recognizer is reset to [UIGestureRecognizerStatePossible](https://developer.apple.com/documentation/uikit/uigesturerecognizerstate/uigesturerecognizerstatepossible?language=objc).
However, when iPad pencil is involved, the state is not reset. I tried to KVO the state property, and wasn't able to capture the change. This means the state change very likely happened internally within the recognizer via the backing ivar of the state property.
*List which issues are fixed by this PR. You must list at least one issue.*
Fixes https://github.com/flutter/flutter/issues/136244
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This is the beginning of the bulk of de-skia-fication work in the engine. All of the standard types in the DlCanvas API now have overloads that specify the corresponding Dl type, mainly for Dl*Rect and DlPoint types. This enables further work to switch from SkFoo types to DlFoo types in the various engine modules culminating in the elimination of the old methods that use the Sk types.
All of the former methods that used the basic Sk types are now implemented as inlinable translation overloads and the underlying implementations of DlCanvas now implement only the newer style interfaces so that they don't need to be further modified as we eliminate the old Skia types from the interface.
There are still a couple of Skia types remaining in the DlCanvas API without any DL type variants which will be handled in a future phase:
- SkRRect
- SkRSXform
- SkTextBlob (will be hidden behind a common interface along with TextFrame)
- SkImageInfo (only used in a few calling sites)
The Vulkan command buffer wrapper delegates almost all of its functionality to the "CommandEncoderVK". I did not find that this separation was useful, as they have a 1-1 relationship and an identical lifecycle. Lets combine them to reduce the number of things to worry about.
Aiks color source is more or less a one to one copy of the impeller::DisplayListColorSource. We can rip it out, instead of flutter::DisplayList::ColorSource -> impeller::Aiks::ColorSource -> impelller:Entity::ColorSourceContents, just go directly flutter::DisplayList::ColorSource -> impelller:Entity::ColorSourceContents.
Part of https://github.com/flutter/flutter/issues/142054
Reverts: flutter/engine#55393
Initiated by: matanlurey
Reason for reverting: Postsubmit turned red (clang tidy and build).
Original PR Author: jonahwilliams
Reviewed By: {chinmaygarde}
This change reverts the following previous change:
Hoist the content context up to the ios_surface so that overlay layers don't recreate a bunch of expensive machinery.
Fixes https://github.com/flutter/flutter/issues/154836
Aiks has been gutted, and most of the reming types need to be merged with dl/entity types and deleted. this is difficult to do with impeller/display_list and aiks as separate gn targets as it requires circular deps.
Lets merge them and remove the aiks directory.
Keeps fml::scoped_* wrappers used in C++ class declarations in headers, since those are injected into both ARC and non-ARC translation units and these classes are ARC-safe. Once we've migrated all users of those headers to ARC, we can use the underlying classes directly.
No semantic changes, therefore not changes to tests.
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts: flutter/engine#55092
Initiated by: jonahwilliams
Reason for reverting: framework golden failures.
Original PR Author: jonahwilliams
Reviewed By: {chinmaygarde, jtmcdole}
This change reverts the following previous change:
Follow up to https://github.com/flutter/engine/pull/55060
Currently we have multiple stages of hashing while font rendering, which is relatively expensive for the actualy required workload. First, we hash the contents of all text frames to compute the unique set of glyphs per frame. Then we diff this hash set against the hashmap of glyphs within the atlas. Finally we hash and lookup the final rendered bounds for each glyph.
We can simplify this to 2. hash lookups for glyphs not yet in the atlas and 1. hash lookup for glyphs that are in the atlas. This is done by combing the step where we uniquely compute glyphs per frame with the diff against the current atlas. When this lookup is performed, we also store the glyph position (if found) in the text_frame itself - which allows text contents to skip the last hash, as long as the glyph has already been rendered.
### Before
### After
Using this handy dandy test app:
```dart
import 'package:flutter/material.dart';
void main() {
runApp(MyApp());
}
class MyApp extends StatelessWidget {
Widget build(context) {
return MaterialApp(
home: Scaffold(
appBar: AppBar(
title: Text('Platform View'),
),
body: SafeArea(child: Stack(children: [
SizedBox(
width: 380,
height: 380,
child: LinearProgressIndicator(),
),
Stack(
children: List<Widget>.generate(1000, (index) {
// The problem already happens with a small amount of widgets.
// Using an excessive amount of widgets is just to make the problem more evident.
return Text("Lots of Texts represent a Widget with complex components.");
}),
),
Align(
alignment: Alignment.bottomCenter,
child:
TextButton(
child: Text("Button"),
onPressed: () {
print("Tap ${DateTime.now()}");
},
),
),
],
),
),
),
);
}
}
```
Follow up to https://github.com/flutter/engine/pull/55060
Currently we have multiple stages of hashing while font rendering, which is relatively expensive for the actualy required workload. First, we hash the contents of all text frames to compute the unique set of glyphs per frame. Then we diff this hash set against the hashmap of glyphs within the atlas. Finally we hash and lookup the final rendered bounds for each glyph.
We can simplify this to 2. hash lookups for glyphs not yet in the atlas and 1. hash lookup for glyphs that are in the atlas. This is done by combing the step where we uniquely compute glyphs per frame with the diff against the current atlas. When this lookup is performed, we also store the glyph position (if found) in the text_frame itself - which allows text contents to skip the last hash, as long as the glyph has already been rendered.
### Before
### After
Using this handy dandy test app:
```dart
import 'package:flutter/material.dart';
void main() {
runApp(MyApp());
}
class MyApp extends StatelessWidget {
Widget build(context) {
return MaterialApp(
home: Scaffold(
appBar: AppBar(
title: Text('Platform View'),
),
body: SafeArea(child: Stack(children: [
SizedBox(
width: 380,
height: 380,
child: LinearProgressIndicator(),
),
Stack(
children: List<Widget>.generate(1000, (index) {
// The problem already happens with a small amount of widgets.
// Using an excessive amount of widgets is just to make the problem more evident.
return Text("Lots of Texts represent a Widget with complex components.");
}),
),
Align(
alignment: Alignment.bottomCenter,
child:
TextButton(
child: Text("Button"),
onPressed: () {
print("Tap ${DateTime.now()}");
},
),
),
],
),
),
),
);
}
}
```
Reverts: flutter/engine#55450
Initiated by: matanlurey
Reason for reverting: Fixed forward in https://github.com/flutter/packages/pull/7712.
Original PR Author: auto-submit[bot]
Reviewed By: {fluttergithubbot}
This change reverts the following previous change:
Reverts: flutter/engine#55418
Initiated by: bdero
Reason for reverting: [Engine->Framework roll breakage](https://github.com/flutter/flutter/issues/155727#issuecomment-2375489803)
Original PR Author: matanlurey
Reviewed By: {jonahwilliams}
This change reverts the following previous change:
Closes https://github.com/flutter/flutter/issues/155131.
Not only did I rename the method, but I also changed the contract slightly - now `onSurfaceAvailable` is _only_ invoked _after_ `onSurfaceDestroyed` has been called. The cost is a single `boolean`, and it honestly makes the API make a lot more sense than someone having to track this themselves.
/cc @johnmccutchan (OOO), and @flutter/android-reviewers.
Fixes https://github.com/flutter/flutter/issues/125640
This doesn't actually do anything, instead it forces opaque to false. If developers want opaque to be false, they can just set it directly.
Reverts: flutter/engine#55418
Initiated by: bdero
Reason for reverting: [Engine->Framework roll breakage](https://github.com/flutter/flutter/issues/155727#issuecomment-2375489803)
Original PR Author: matanlurey
Reviewed By: {jonahwilliams}
This change reverts the following previous change:
Closes https://github.com/flutter/flutter/issues/155131.
Not only did I rename the method, but I also changed the contract slightly - now `onSurfaceAvailable` is _only_ invoked _after_ `onSurfaceDestroyed` has been called. The cost is a single `boolean`, and it honestly makes the API make a lot more sense than someone having to track this themselves.
/cc @johnmccutchan (OOO), and @flutter/android-reviewers.
