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

Initialize skwasm codecs before handing them back to the user. #43274

Merged
merged 2 commits into from
Jun 27, 2023
Merged

Initialize skwasm codecs before handing them back to the user. #43274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9056ea0
Initialize skwasm codecs before handing them back to the user.
eyebrowsoffire Jun 27, 2023
89cab73
Repetition count is not reliable.
eyebrowsoffire Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/web_ui/lib/src/engine/skwasm/skwasm_impl/renderer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,12 @@ class SkwasmRenderer implements Renderer {
if (contentType == null) {
throw Exception('Could not determine content type of image from data');
}
final ui.Codec baseDecoder = SkwasmImageDecoder(
final SkwasmImageDecoder baseDecoder = SkwasmImageDecoder(
contentType: contentType,
dataSource: list.toJS,
debugSource: 'encoded image bytes',
);
await baseDecoder.initialize();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how the error manifested on the failing tests, but it may make sense to spit out a warning on the SkwasmImageDecoder if users attempt to call method instances before they call initialize()? It doesn't look like an optional method to call, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the whole point of this change is that we should never hand the codec back to the user in an uninitialized state. Particularly because the ui.Codec API doesn't have the initialize method on it. So I don't think there is any reason to have warnings, vending an uninitialized codec is an error of the engine.

if (targetWidth == null && targetHeight == null) {
return baseDecoder;
}
Expand All @@ -395,11 +396,13 @@ class SkwasmRenderer implements Renderer {
if (contentType == null) {
throw Exception('Could not determine content type of image at url $uri');
}
return SkwasmImageDecoder(
final SkwasmImageDecoder decoder = SkwasmImageDecoder(
contentType: contentType,
dataSource: response.body as JSAny,
debugSource: uri.toString(),
);
await decoder.initialize();
return decoder;
Comment on lines +404 to +405
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If initialize didn't swallow the returned future type here:

(And returned the initialized this or something.

You could just return decoder.initialize();

}

@override
Expand Down
1 change: 1 addition & 0 deletions lib/web_ui/test/ui/filters_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Future<void> testMain() async {
final ui.Codec codec = await renderer.instantiateImageCodecFromUrl(
Uri(path: '/test_images/mandrill_128.png')
);
expect(codec.frameCount, 1);

final ui.FrameInfo info = await codec.getNextFrame();
final ui.Image image = info.image;
Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/test/ui/image_golden_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ Future<void> testMain() async {
final ui.Codec codec = await renderer.instantiateImageCodecFromUrl(
Uri(path: '/test_images/mandrill_128.png')
);
expect(codec.frameCount, 1);

final ui.FrameInfo info = await codec.getNextFrame();
return info.image;
Expand All @@ -300,6 +301,7 @@ Future<void> testMain() async {
targetWidth: 150,
targetHeight: 150,
);
expect(codec.frameCount, 1);

final ui.FrameInfo info = await codec.getNextFrame();
return info.image;
Expand Down