Implement frame timing callbacks in Skwasm. #50737

eyebrowsoffire · 2024-02-16T23:36:46Z

Some notes here:

Refactored the frame timing systems so that we can deal with asynchronous rendering.
Consolidated rendering of multiple pictures in skwasm into a single call, so that the rasterization can be properly measured.
Pulled the frame timings tests into the ui test suite so that they run on all renderers (including skwasm).

mdebbar · 2024-02-20T16:04:05Z

lib/web_ui/lib/frame_timings.dart

I strongly recommend that we keep these files as similar as possible to their lib/ui/ counterparts to keep maintenance of the dart:ui API surface easy across platforms.

I agree. We deviated before, which was difficult to maintain.

Kind of makes me sad but okay.

yjbanov

yjbanov · 2024-02-20T18:36:32Z

lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart

-        );
+        (await (renderer as SkwasmRenderer).surface.renderPictures(
+          <SkwasmPicture>[recorder.endRecording() as SkwasmPicture],
+        )).imageBitmaps.first;


Maybe I'm being paranoid, but using .single instead of .first may be able to catch bugs in the future.

yjbanov · 2024-02-20T18:44:38Z

lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart

+        )).imageBitmaps.first;
      final DomOffscreenCanvas offscreenCanvas =
        createDomOffscreenCanvas(bitmap.width.toDartInt, bitmap.height.toDartInt);
      final DomCanvasRenderingContextBitmapRenderer context =


(github won't allow me to leave a comment on the line below, so putting it here)

The lines below that are zeroing out the contents of the canvas to reclaim resources should be identical to context.transferFromImageBitmap(null). Since context owns the ImageBitmap that's transferred into it, transferring null would free any previous ImageBitmap that it owned.

PS: It's also weird that we need a canvas to turn an ImageBitmap to a PNG, but I guess it's the web for ya.

yjbanov · 2024-02-20T18:49:16Z

lib/web_ui/lib/src/engine/skwasm/skwasm_impl/surface.dart

+
+extension RasterResultExtension on RasterResult {
+  external JSNumber get rasterStart;
+  external JSNumber get rasterEnd;


Should we also use the Micros suffix here, for consistency with the Dart API?

It's not actually micros, it's milliseconds as a double (what performance.now returns). I guess I should rename them to match that though, or at least put a comment in.

yjbanov · 2024-02-20T18:55:05Z

lib/web_ui/skwasm/surface.cpp

+  }
+
+  // Releasing picturePointers here and will recreate the unique_ptr on the
+  // other thread See skwasm_renderPicturesOnWorker


nit: should skwasm_renderPicturesOnWorker be surface_renderPicturesOnWorker?

mdebbar

Overall LGTM, but I'm also leaning on Yegor's LGTM for the rendering bits that I'm not too familiar with :)

…#50737)

…143972) flutter/engine@06448ee...cb6115d 2024-02-22 [email protected] Avoid generated plugin registrant warnings for scenario_app (flutter/engine#50874) 2024-02-22 [email protected] Delete and create iOS simulator before running Scenario app test (flutter/engine#50835) 2024-02-22 [email protected] Roll Skia from b9c16065b76d to dd8cd405d145 (2 revisions) (flutter/engine#50872) 2024-02-22 [email protected] Remove 'bringup: true' from 'Mac mac_unopt' (flutter/engine#50865) 2024-02-22 [email protected] Pass the missing strut half leading flag over to skia paragraph builder (flutter/engine#50385) 2024-02-22 [email protected] Implement frame timing callbacks in Skwasm. (flutter/engine#50737) 2024-02-22 [email protected] [Impeller] Add stroke benchmarks that create UVs with no transform (flutter/engine#50847) 2024-02-22 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Remove WindowManager reflection in SingleViewPresentation.java (#49996)" (flutter/engine#50873) 2024-02-22 [email protected] Use RBE on more Windows builders (flutter/engine#50866) 2024-02-22 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] cache onscreen render targets. (#50751)" (flutter/engine#50871) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

goderbauer · 2024-02-22T23:37:51Z

Reason for revert: Fails in device lab, see https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8755350727803344657/+/u/run_web_benchmarks_skwasm/stdout

This reverts commit 3ee6f25.

Reverts #50737 Initiated by: goderbauer Reason for reverting: Fails in device lab, see https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8755350727803344657/+/u/run_web_benchmarks_skwasm/stdout Original PR Author: eyebrowsoffire Reviewed By: {mdebbar, yjbanov} This change reverts the following previous change: Original Description: Fixes flutter/flutter#140429 Some notes here: * Refactored the frame timing systems so that we can deal with asynchronous rendering. * Consolidated rendering of multiple pictures in skwasm into a single call, so that the rasterization can be properly measured. * Pulled the frame timings tests into the `ui` test suite so that they run on all renderers (including skwasm).

flutter/engine#50737 broke the tree

…#50737)

…sions) (#143989) Manual roll requested by [email protected] flutter/engine@06448ee...0f3ad23 2024-02-22 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Implement frame timing callbacks in Skwasm. (#50737)" (flutter/engine#50895) 2024-02-22 [email protected] Roll Dart SDK from 0f0f7400c38a to e2f2d9b464e9 (4 revisions) (flutter/engine#50881) 2024-02-22 [email protected] Roll Skia from a285cd79614c to 58772db6bc46 (3 revisions) (flutter/engine#50884) 2024-02-22 [email protected] [Impeller] moved the kernel size assert to a more appropriate location. (flutter/engine#50880) 2024-02-22 [email protected] Build macOS engine as an xcframework (flutter/engine#50300) 2024-02-22 [email protected] Roll Skia from dd8cd405d145 to a285cd79614c (3 revisions) (flutter/engine#50878) 2024-02-22 [email protected] Avoid generated plugin registrant warnings for scenario_app (flutter/engine#50874) 2024-02-22 [email protected] Delete and create iOS simulator before running Scenario app test (flutter/engine#50835) 2024-02-22 [email protected] Roll Skia from b9c16065b76d to dd8cd405d145 (2 revisions) (flutter/engine#50872) 2024-02-22 [email protected] Remove 'bringup: true' from 'Mac mac_unopt' (flutter/engine#50865) 2024-02-22 [email protected] Pass the missing strut half leading flag over to skia paragraph builder (flutter/engine#50385) 2024-02-22 [email protected] Implement frame timing callbacks in Skwasm. (flutter/engine#50737) 2024-02-22 [email protected] [Impeller] Add stroke benchmarks that create UVs with no transform (flutter/engine#50847) 2024-02-22 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Remove WindowManager reflection in SingleViewPresentation.java (#49996)" (flutter/engine#50873) 2024-02-22 [email protected] Use RBE on more Windows builders (flutter/engine#50866) 2024-02-22 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] cache onscreen render targets. (#50751)" (flutter/engine#50871) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Fixes flutter/flutter#140429 Some notes here: * Refactored the frame timing systems so that we can deal with asynchronous rendering. * Consolidated rendering of multiple pictures in skwasm into a single call, so that the rasterization can be properly measured. * Pulled the frame timings tests into the `ui` test suite so that they run on all renderers (including skwasm).

Implement frame timing callbacks in Skwasm.

98ae006

flutter/flutter#140429

github-actions bot added the platform-web Code specifically for the web engine label Feb 16, 2024

eyebrowsoffire added 2 commits February 16, 2024 16:04

Fix issue from merge conflict.

1303811

Merge branch 'main' into skwasm_frame_timings

2f4beeb

mdebbar reviewed Feb 20, 2024

View reviewed changes

yjbanov approved these changes Feb 20, 2024

View reviewed changes

eyebrowsoffire added 5 commits February 21, 2024 13:06

Don't fail if more than one scene is rendered in a frame.

890cff3

Merge branch 'main' into skwasm_frame_timings

7ac88cc

Address comments.

b9e1b1f

Don't use late variables to be more lenient in test scenarios.

769775e

Update license golden.

698c43d

mdebbar approved these changes Feb 22, 2024

View reviewed changes

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 22, 2024

auto-submit bot merged commit 3ee6f25 into flutter:main Feb 22, 2024

engine-flutter-autoroll mentioned this pull request Feb 22, 2024

Roll Flutter Engine from 06448ee8165f to bd7c9d78f81f (6 revisions) flutter/flutter#143968

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 22, 2024

3ee6f251a Implement frame timing callbacks in Skwasm. (flutter/engine…

fa2473d

…#50737)

engine-flutter-autoroll mentioned this pull request Feb 22, 2024

Roll Flutter Engine from 06448ee8165f to 547c83466f5e (9 revisions) flutter/flutter#143970

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 22, 2024

3ee6f251a Implement frame timing callbacks in Skwasm. (flutter/engine…

53b9941

…#50737)

engine-flutter-autoroll mentioned this pull request Feb 22, 2024

Roll Flutter Engine from 06448ee8165f to cb6115d3a6ab (10 revisions) flutter/flutter#143972

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 22, 2024

3ee6f251a Implement frame timing callbacks in Skwasm. (flutter/engine…

4d72e0c

…#50737)

goderbauer added the revert Label used to revert changes in a closed and merged pull request. label Feb 22, 2024

eyebrowsoffire added the revert Label used to revert changes in a closed and merged pull request. label Feb 22, 2024

auto-submit bot pushed a commit that referenced this pull request Feb 22, 2024

Revert "Implement frame timing callbacks in Skwasm. (#50737)"

b46f085

This reverts commit 3ee6f25.

auto-submit bot mentioned this pull request Feb 22, 2024

Reverts "Implement frame timing callbacks in Skwasm. (#50737)" #50895

Merged

auto-submit bot removed the revert Label used to revert changes in a closed and merged pull request. label Feb 22, 2024

goderbauer added a commit to flutter/flutter that referenced this pull request Feb 22, 2024

Revert engine back to 06448ee8165f4ff6579ef9510ac69322eda896ac

653233d

flutter/engine#50737 broke the tree

goderbauer mentioned this pull request Feb 22, 2024

Revert engine back to 06448ee8165f4ff6579ef9510ac69322eda896ac flutter/flutter#143986

Merged

goderbauer added a commit to flutter/flutter that referenced this pull request Feb 22, 2024

Revert engine back to 06448ee8165f4ff6579ef9510ac69322eda896ac (#143986)

94855a9

flutter/engine#50737 broke the tree

engine-flutter-autoroll mentioned this pull request Feb 23, 2024

Manual roll Flutter Engine from 06448ee8165f to 0f3ad23b84ed (16 revisions) flutter/flutter#143989

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 23, 2024

3ee6f251a Implement frame timing callbacks in Skwasm. (flutter/engine…

5b9efa6

…#50737)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement frame timing callbacks in Skwasm. #50737

Implement frame timing callbacks in Skwasm. #50737

Uh oh!

eyebrowsoffire commented Feb 16, 2024

Uh oh!

mdebbar Feb 20, 2024

Uh oh!

yjbanov Feb 20, 2024

Uh oh!

eyebrowsoffire Feb 21, 2024

Uh oh!

yjbanov left a comment

Uh oh!

yjbanov Feb 20, 2024

Uh oh!

yjbanov Feb 20, 2024

Uh oh!

yjbanov Feb 20, 2024

Uh oh!

eyebrowsoffire Feb 20, 2024

Uh oh!

yjbanov Feb 20, 2024

Uh oh!

mdebbar left a comment

Uh oh!

goderbauer commented Feb 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Implement frame timing callbacks in Skwasm. #50737

Implement frame timing callbacks in Skwasm. #50737

Uh oh!

Conversation

eyebrowsoffire commented Feb 16, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yjbanov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

goderbauer commented Feb 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants