[web] use a render target instead of a new surface for Picture.toImage #38573

jonahwilliams · 2022-12-30T02:17:12Z

I don't have full context into the original problem this change was solving, but in the general case it is incredibly slow. Using a different render target instead of a new glContext is significantly faster.

This does hit an issue where we are perhaps setting up some of the rendering state too late, but that only seems to happen once.

jonahwilliams · 2022-12-30T02:50:43Z

This currently doesn't handle the destruction of the surface when resizing the window

jonahwilliams · 2022-12-30T03:56:02Z

We shouldn't be losing the context on resize anyway: flutter/flutter#117803

lib/web_ui/lib/src/engine/canvaskit/picture.dart

…_fast

jonahwilliams · 2023-01-12T19:56:46Z

update: holding off on this change util runtime effect support lands in the engine so I can easily test it

…_fast

jonahwilliams · 2023-01-13T21:15:37Z

Verified that samplers still work with this change, marking as ready for review

…_fast

jonahwilliams · 2023-01-17T18:24:44Z

Update: working on the case where there is no GL context, we may need differend code to fall back to a surface.

…_fast

harryterkelsen

LGTM with small change

harryterkelsen · 2023-01-24T22:08:35Z

lib/web_ui/lib/src/engine/canvaskit/surface.dart

+  /// This is only valid after the first frame or if [ensureSurface] has been
+  /// called
+  bool get usingSoftwareBackend => _glContext == null ||
+      _grContext == null || webGLVersion == -1 || !configuration.canvasKitForceCpuOnly;


don't invert configuration.canvasKitForceCpuOnly

Oops, good catch. Fixed

harryterkelsen · 2023-01-24T22:12:55Z

lib/web_ui/lib/src/engine/canvaskit/surface.dart

+    // TODO(jonahwilliams): this is somewhat wasteful. We should probably
+    // eagerly setup this surface instead of delaying until the first frame?
+    // Or at least cache the estimated window size.
+    createOrUpdateSurface(const ui.Size(1, 1));


maybe ensureSurface can take an optional Size?

…ure.toImage (flutter/engine#38573)

flutter#38573) * [web] use a render target instead of a new surface for Picture.toImage * setup grcontext if missing * ++ * dispose * Add unit test for MakeRenderTarget * ++ * ++ * ++ * ++ * ++ * ++ * ++

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

* a0f7c8c 6f806491e [web] use a render target instead of a new surface for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143) * e85547b Roll Plugins from 11361d0 to 8bab180 (28 revisions) (flutter/flutter#119115) * 81052a7 Add usage event to track when a iOS network device is used (flutter/flutter#118915) * cd34fa6 24aa324b8 Roll Skia from da5034f9d117 to c4b171fe5668 (1 revision) (flutter/engine#39127) (flutter/flutter#119159) * 6cd4fa4 Add --serve-observatory flag to run, attach, and test (flutter/flutter#118402) * 48cd95d 1e5efd1 [various] Enable use_build_context_synchronously (flutter/plugins#6585) (flutter/flutter#119162) * b907acd Add the cupertino system colors mint, cyan, and brown (flutter/flutter#118971) * c6fa5d9 c54580138 Only build analyze_snapshot on Linux host (flutter/engine#39129) (flutter/flutter#119164) * f34ce86 7b72038ef Roll Fuchsia Linux SDK from E9m-Gk382PkB7_Nbp... to pGX7tanT1okL8XCg-... (flutter/engine#39130) (flutter/flutter#119169) * 0dd63d3 Export View (flutter/flutter#117475) * 1fd71de Remove superfluous words from comments (flutter/flutter#119055) * cef9cc7 2e7d6fa7b Remove unnecessary null checks (flutter/engine#39113) (flutter/flutter#119174) * 3be330a 30c02e4c8 [Impeller] Make text glyph offsets respect the current transform (flutter/engine#39119) (flutter/flutter#119179) * a45727d Add MediaQuery to View (flutter/flutter#118004) * 02a9c15 Fix lexer issue where select/plural/other/underscores cannot be in identifier names. (flutter/flutter#119190) * 766e4d2 Remove single-view assumption from material library (flutter/flutter#117486) * dcd3679 Roll Flutter Engine from 30c02e4c8b01 to 44362c90fcec (2 revisions) (flutter/flutter#119185) * 9037e3f roll packages (flutter/flutter#119192) * e0e88da Roll Flutter Engine from 44362c90fcec to 308ce918f67f (2 revisions) (flutter/flutter#119201) * 202e902 Roll Flutter Engine from 308ce918f67f to 8f1e5dc1b124 (4 revisions) (flutter/flutter#119208) * b319938 Add more flexible image API (flutter/flutter#118966) * fc02701 Marks Mac run_debug_test_macos to be unflaky (flutter/flutter#117470) * c9affdb Move windows-x64-flutter.zip to windows-x64-debug location. (flutter/flutter#119177)

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

* a0f7c8c 6f806491e [web] use a render target instead of a new surface for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143) * e85547b Roll Plugins from 11361d0 to 8bab180 (28 revisions) (flutter/flutter#119115) * 81052a7 Add usage event to track when a iOS network device is used (flutter/flutter#118915) * cd34fa6 24aa324b8 Roll Skia from da5034f9d117 to c4b171fe5668 (1 revision) (flutter/engine#39127) (flutter/flutter#119159) * 6cd4fa4 Add --serve-observatory flag to run, attach, and test (flutter/flutter#118402) * 48cd95d 1e5efd1 [various] Enable use_build_context_synchronously (#6585) (flutter/flutter#119162) * b907acd Add the cupertino system colors mint, cyan, and brown (flutter/flutter#118971) * c6fa5d9 c54580138 Only build analyze_snapshot on Linux host (flutter/engine#39129) (flutter/flutter#119164) * f34ce86 7b72038ef Roll Fuchsia Linux SDK from E9m-Gk382PkB7_Nbp... to pGX7tanT1okL8XCg-... (flutter/engine#39130) (flutter/flutter#119169) * 0dd63d3 Export View (flutter/flutter#117475) * 1fd71de Remove superfluous words from comments (flutter/flutter#119055) * cef9cc7 2e7d6fa7b Remove unnecessary null checks (flutter/engine#39113) (flutter/flutter#119174) * 3be330a 30c02e4c8 [Impeller] Make text glyph offsets respect the current transform (flutter/engine#39119) (flutter/flutter#119179) * a45727d Add MediaQuery to View (flutter/flutter#118004) * 02a9c15 Fix lexer issue where select/plural/other/underscores cannot be in identifier names. (flutter/flutter#119190) * 766e4d2 Remove single-view assumption from material library (flutter/flutter#117486) * dcd3679 Roll Flutter Engine from 30c02e4c8b01 to 44362c90fcec (2 revisions) (flutter/flutter#119185) * 9037e3f roll packages (flutter/flutter#119192) * e0e88da Roll Flutter Engine from 44362c90fcec to 308ce918f67f (2 revisions) (flutter/flutter#119201) * 202e902 Roll Flutter Engine from 308ce918f67f to 8f1e5dc1b124 (4 revisions) (flutter/flutter#119208) * b319938 Add more flexible image API (flutter/flutter#118966) * fc02701 Marks Mac run_debug_test_macos to be unflaky (flutter/flutter#117470) * c9affdb Move windows-x64-flutter.zip to windows-x64-debug location. (flutter/flutter#119177) * 7d3b762 Fix: Added `margin` parameter for `MaterialBanner` class (flutter/flutter#119005) * 40bd82e Roll Plugins from 1e5efd1 to e9406bc (4 revisions) (flutter/flutter#119249) * 07522b7 Roll Flutter Engine from 8f1e5dc1b124 to 04f22beebb42 (5 revisions) (flutter/flutter#119218) * 459c1b7 Marks Mac complex_layout_scroll_perf_macos__timeline_summary to be unflaky (flutter/flutter#119157) * 2b8f2d0 Add API for discovering assets (flutter/flutter#118410) * a04ab71 Revert "Add API for discovering assets (#118410)" (flutter/flutter#119273) * 1da487d Roll Flutter Engine from 04f22beebb42 to 93901260098e (12 revisions) (flutter/flutter#119279) * 1b779b6 Roll Flutter Engine from 93901260098e to be0125bd5716 (2 revisions) (flutter/flutter#119283) * 42bd5f2 Download platform-agnostic Flutter Web SDK in the flutter_tool (flutter/flutter#118654) * d52b6b9 Roll Flutter Engine from be0125bd5716 to d17004dd96d7 (2 revisions) (flutter/flutter#119287) * 4aed487 Roll Flutter Engine from d17004dd96d7 to a63d98feb608 (3 revisions) (flutter/flutter#119299) * 05fc29f Rename DeviceGestureSettings.fromWindow to DeviceGestureSettings.fromView (flutter/flutter#119291) * 86ab01d Revert "Add --serve-observatory flag to run, attach, and test (#118402)" (flutter/flutter#119302) * 8d03af3 Roll Flutter Engine from a63d98feb608 to 79c958fc7e9b (3 revisions) (flutter/flutter#119306) * 27f8ebd ade610ec8 [fuchsia] Migrate to new RealmBuilder API (flutter/engine#39175) (flutter/flutter#119310) * c31856b Roll Plugins from e9406bc to ff84c44 (2 revisions) (flutter/flutter#119335) * d939863 Roll Flutter Engine from ade610ec88b5 to 621e13cc9be3 (3 revisions) (flutter/flutter#119344) * 0b57596 Run "flutter update-packages --force-upgrade" (flutter/flutter#119340) * 0417f66 Fix nullability of TableRow.children (flutter/flutter#119285) * fc3e824 Roll Flutter Engine from 621e13cc9be3 to 189a69d9918d (3 revisions) (flutter/flutter#119347) * ad1a44d Add `requestFocusOnTap` to `DropdownMenu` (flutter/flutter#117504) * 4dbb573 [flutter_tools] remove usage of remap samplers arg (flutter/flutter#119346) * b2f2bf3 Marks Linux run_release_test_linux to be unflaky (flutter/flutter#119156) * 3f95bef Roll Flutter Engine from 189a69d9918d to b32fc7fef208 (3 revisions) (flutter/flutter#119358) * 2e8bebd Remove single window assumption from macrobenchmark (flutter/flutter#119368) * ab2232a Roll Flutter Engine from b32fc7fef208 to 8567d96993ed (5 revisions) (flutter/flutter#119369) * e9ca9cc Remove references to dart:ui's window singelton (flutter/flutter#119296) * da3d4bd Roll Flutter Engine from 8567d96993ed to 225ae87334a5 (2 revisions) (flutter/flutter#119376) * 018c1f8 e2e089ebb Use arm64 engine variant on simulators in iOS unit tests (flutter/engine#39213) (flutter/flutter#119387) * e349bdc 19651cb1d Roll Dart SDK from 2cd9b7ac95e8 to 135f4c51c9ff (3 revisions) (flutter/engine#39214) (flutter/flutter#119389) * 95345b5 77bee011d Roll Dart SDK from 2cd9b7ac95e8 to 135f4c51c9ff (3 revisions) (flutter/engine#39217) (flutter/flutter#119394) * de43ec9 Roll Flutter Engine from 77bee011dabf to 3394b84cc5d7 (3 revisions) (flutter/flutter#119405) * f8d4de4 3dd0fc13f Roll Fuchsia Linux SDK from 6c2H32X3EXOGlWIgb... to TiK_fVODtUaKOgxRf... (flutter/engine#39224) (flutter/flutter#119408) * 7856411 7c5c6c9c9 Roll Skia from 0b75650caf2a to 7df7a83f733d (13 revisions) (flutter/engine#39225) (flutter/flutter#119413) * 75680ae 649362168 Roll Dart SDK from f9583e13e214 to 52dc94238144 (1 revision) (flutter/engine#39227) (flutter/flutter#119416)

* a0f7c8c 6f806491e [web] use a render target instead of a new surface for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143) * e85547b Roll Plugins from 11361d0 to 8bab180 (28 revisions) (flutter/flutter#119115) * 81052a7 Add usage event to track when a iOS network device is used (flutter/flutter#118915) * cd34fa6 24aa324b8 Roll Skia from da5034f9d117 to c4b171fe5668 (1 revision) (flutter/engine#39127) (flutter/flutter#119159) * 6cd4fa4 Add --serve-observatory flag to run, attach, and test (flutter/flutter#118402) * 48cd95d 1e5efd1 [various] Enable use_build_context_synchronously (flutter/plugins#6585) (flutter/flutter#119162) * b907acd Add the cupertino system colors mint, cyan, and brown (flutter/flutter#118971) * c6fa5d9 c54580138 Only build analyze_snapshot on Linux host (flutter/engine#39129) (flutter/flutter#119164) * f34ce86 7b72038ef Roll Fuchsia Linux SDK from E9m-Gk382PkB7_Nbp... to pGX7tanT1okL8XCg-... (flutter/engine#39130) (flutter/flutter#119169) * 0dd63d3 Export View (flutter/flutter#117475) * 1fd71de Remove superfluous words from comments (flutter/flutter#119055) * cef9cc7 2e7d6fa7b Remove unnecessary null checks (flutter/engine#39113) (flutter/flutter#119174) * 3be330a 30c02e4c8 [Impeller] Make text glyph offsets respect the current transform (flutter/engine#39119) (flutter/flutter#119179) * a45727d Add MediaQuery to View (flutter/flutter#118004) * 02a9c15 Fix lexer issue where select/plural/other/underscores cannot be in identifier names. (flutter/flutter#119190) * 766e4d2 Remove single-view assumption from material library (flutter/flutter#117486) * dcd3679 Roll Flutter Engine from 30c02e4c8b01 to 44362c90fcec (2 revisions) (flutter/flutter#119185) * 9037e3f roll packages (flutter/flutter#119192) * e0e88da Roll Flutter Engine from 44362c90fcec to 308ce918f67f (2 revisions) (flutter/flutter#119201) * 202e902 Roll Flutter Engine from 308ce918f67f to 8f1e5dc1b124 (4 revisions) (flutter/flutter#119208) * b319938 Add more flexible image API (flutter/flutter#118966) * fc02701 Marks Mac run_debug_test_macos to be unflaky (flutter/flutter#117470) * c9affdb Move windows-x64-flutter.zip to windows-x64-debug location. (flutter/flutter#119177)

[web] use a render target instead of a new surface for Picture.toImage

8c06a2a

flutter-dashboard bot added the platform-web Code specifically for the web engine label Dec 30, 2022

jonahwilliams added 2 commits January 4, 2023 09:10

setup grcontext if missing

6b8cec3

++

749cf71

harryterkelsen reviewed Jan 10, 2023

View reviewed changes

lib/web_ui/lib/src/engine/canvaskit/picture.dart Show resolved Hide resolved

Merge branch 'master' of github.com:flutter/engine into make_to_image…

2c1a45e

…_fast

jonahwilliams added 2 commits January 13, 2023 12:41

Merge branch 'master' of github.com:flutter/engine into make_to_image…

8743dcf

…_fast

dispose

8679c53

jonahwilliams marked this pull request as ready for review January 13, 2023 21:15

jonahwilliams added 3 commits January 13, 2023 13:18

Add unit test for MakeRenderTarget

df4bcd2

Merge branch 'master' of github.com:flutter/engine into make_to_image…

474041c

…_fast

++

1d72eb8

jonahwilliams added 6 commits January 23, 2023 17:42

Merge branch 'master' of github.com:flutter/engine into make_to_image…

aff14fd

…_fast

++

22f94b1

++

b86e621

++

e84c819

++

96b9605

++

3411d93

harryterkelsen approved these changes Jan 24, 2023

View reviewed changes

++

af8d6b8

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 25, 2023

auto-submit bot merged commit 6f80649 into flutter:main Jan 25, 2023

jonahwilliams deleted the make_to_image_fast branch January 25, 2023 15:10

engine-flutter-autoroll mentioned this pull request Jan 25, 2023

Roll Flutter Engine from 8efc7183bab6 to 6f806491e103 (1 revision) flutter/flutter#119143

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 25, 2023

6f806491e [web] use a render target instead of a new surface for Pict…

f185ef5

…ure.toImage (flutter/engine#38573)

This was referenced Jan 26, 2023

Roll Flutter from a815ee634202 to c9affdba9dcd (23 revisions) flutter/packages#3093

Merged

Roll Flutter from a815ee634202 to c9affdba9dcd (23 revisions) flutter/plugins#7032

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

afdcf34

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 26, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

40d0b7f

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll mentioned this pull request Jan 26, 2023

Roll Flutter from a815ee634202 to c9affdba9dcd (23 revisions) flutter/plugins#7033

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 26, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

6f71701

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll mentioned this pull request Jan 26, 2023

Roll Flutter from a815ee634202 to 7d3b762dfc4e (24 revisions) flutter/plugins#7035

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 26, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

7bb961d

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll mentioned this pull request Jan 27, 2023

Roll Flutter from a815ee634202 to 27f8ebdaed70 (38 revisions) flutter/plugins#7037

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 27, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

2a10af8

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll mentioned this pull request Jan 27, 2023

Roll Flutter from a815ee634202 to c31856bc4859 (39 revisions) flutter/plugins#7039

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 27, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

4f74e3c

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll mentioned this pull request Jan 27, 2023

Roll Flutter from a815ee634202 to d939863a2638 (40 revisions) flutter/plugins#7040

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 27, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

48c628f

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll mentioned this pull request Jan 27, 2023

Roll Flutter from a815ee634202 to 0417f6621eaa (42 revisions) flutter/plugins#7041

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 27, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

0e7a36c

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll mentioned this pull request Jan 27, 2023

Roll Flutter from a815ee634202 to fc3e8243cab5 (43 revisions) flutter/plugins#7042

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 27, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

ef7ca33

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll mentioned this pull request Jan 27, 2023

Roll Flutter from a815ee634202 to ad1a44d0a702 (44 revisions) flutter/plugins#7044

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 27, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

149248d

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 28, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

f2c43d1

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 28, 2023

a0f7c8c 6f806491e [web] use a render target instead of a new surface …

0f64ad1

…for Picture.toImage (flutter/engine#38573) (flutter/flutter#119143)

yjbanov mentioned this pull request Mar 6, 2023

[canvaskit] read pixels back in Picture.toImage #40004

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[web] use a render target instead of a new surface for Picture.toImage #38573

[web] use a render target instead of a new surface for Picture.toImage #38573

Uh oh!

jonahwilliams commented Dec 30, 2022

Uh oh!

jonahwilliams commented Dec 30, 2022

Uh oh!

jonahwilliams commented Dec 30, 2022

Uh oh!

Uh oh!

jonahwilliams commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 13, 2023

Uh oh!

jonahwilliams commented Jan 17, 2023

Uh oh!

harryterkelsen left a comment

Uh oh!

harryterkelsen Jan 24, 2023

Uh oh!

jonahwilliams Jan 25, 2023

Uh oh!

harryterkelsen Jan 24, 2023

Uh oh!

jonahwilliams Jan 25, 2023

Uh oh!

Uh oh!

[web] use a render target instead of a new surface for Picture.toImage #38573

[web] use a render target instead of a new surface for Picture.toImage #38573

Uh oh!

Conversation

jonahwilliams commented Dec 30, 2022

Uh oh!

jonahwilliams commented Dec 30, 2022

Uh oh!

jonahwilliams commented Dec 30, 2022

Uh oh!

Uh oh!

jonahwilliams commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 13, 2023

Uh oh!

jonahwilliams commented Jan 17, 2023

Uh oh!

harryterkelsen left a comment

Choose a reason for hiding this comment

Uh oh!

harryterkelsen Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Jan 25, 2023

Choose a reason for hiding this comment

Uh oh!

harryterkelsen Jan 24, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Jan 25, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!