Manually Revert "fix(web): Fix LateInitializationError in CkSurface and SkwasmSurface (#185116)"#185539
Conversation
…nd SkwasmSurface (flutter#185116)" This reverts commit b38bda1. This is being reverted because there was a GitHub incident which caused this to be a bad merge (it contains a bunch of unrelated changes which themselves are unintentional reverts of previous commits). We will reland the actual fix cleanly after this merges.
There was a problem hiding this comment.
Code Review
This pull request implements iterative downscaling for the Flutter web engine to address aliasing artifacts when scaling images below 50% of their original size. It introduces a new image_downscaler.dart utility, a DownscaledImageCache, and integrates these into the CanvasKit and Skwasm backends. Feedback highlights that using the late keyword on nullable fields in CkSurface and SkwasmSurface is problematic and that documentation should be maintained for the newly private _surfaceCreateFn field.
| final Completer<void> _initialized = Completer<void>(); | ||
|
|
||
| Completer<void>? _handledContextLostEvent; | ||
| late Completer<void>? _handledContextLostEvent; |
There was a problem hiding this comment.
Using the late keyword on a nullable field like Completer<void>? is generally discouraged in Dart and is the likely cause of the LateInitializationError mentioned in the PR title. Since null is a valid initial state for this field, it is safer to initialize it to null (which is the default for nullable fields) rather than using late. This ensures that accessing the field before it is explicitly assigned a Completer returns null instead of throwing an exception.
| late Completer<void>? _handledContextLostEvent; | |
| Completer<void>? _handledContextLostEvent; |
| BitmapSize _currentSize = const BitmapSize(1, 1); | ||
| Completer<void> _initializedCompleter; | ||
| Completer<void>? _handledContextLostEvent; | ||
| late Completer<void>? _handledContextLostEvent; |
There was a problem hiding this comment.
Using the late keyword on a nullable field like Completer<void>? is generally discouraged in Dart and is the likely cause of the LateInitializationError mentioned in the PR title. Since null is a valid initial state for this field, it is safer to initialize it to null (which is the default for nullable fields) rather than using late. This ensures that accessing the field before it is explicitly assigned a Completer returns null instead of throwing an exception.
| late Completer<void>? _handledContextLostEvent; | |
| Completer<void>? _handledContextLostEvent; |
| /// A function that creates a new surface of type [C] using a [CanvasProvider] of type [D]. | ||
| @visibleForTesting | ||
| final C Function(D) surfaceCreateFn; | ||
| final C Function(D) _surfaceCreateFn; |
There was a problem hiding this comment.
Although this field has been made private, it is still helpful to maintain its documentation to explain its purpose, especially since it was previously documented. According to the Flutter style guide (Rule 67), /// should be used for public-quality documentation even on private members.
| final C Function(D) _surfaceCreateFn; | |
| /// A function that creates a new surface of type [C] using a [CanvasProvider] of type [D]. | |
| final C Function(D) _surfaceCreateFn; |
References
- Use /// for public-quality documentation, even on private members. (link)
This reverts commit b38bda1.
This is being reverted because there was a GitHub incident which caused this to be a bad merge (it contains a bunch of unrelated changes which themselves are unintentional reverts of previous commits). We will reland the actual fix cleanly after this merges.