From 2c58573690d4025cdbd8dbfcf201c43c97046dea Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Fri, 29 Sep 2023 13:52:36 -0700 Subject: [PATCH] Use `dart:_wasm` constructs to avoid dependence on `WebAssembly.Function` (flutter/engine#46388) Use the newly exposed functionality in `dart:_wasm` to fix up two different hacks we have: 1) When creating an image from an image source, use `wasm:import` instead of `@Native` and pass the image source directly as an externref. (Direct wasm binding instead of a JS interop shim, yay). 2) When binding the surface callback, previously we were wrapping the callback in a JS function, and then using `WebAssembly.Function` to create a wasm function wrapper around that. Now, we can create a `WasmFuncRef` that is a direct reference to a dart function and pass that over. Now there are no intermediary JavaScript layers when skwasm calls back to us, and we no longer are dependent on the type reflection flag in Chrome. This fixes https://github.com/flutter/flutter/issues/134556 --- .../flutter/lib/web_ui/dev/test_dart2wasm.js | 3 +- .../skwasm/skwasm_impl/raw/raw_image.dart | 49 ++++------- .../skwasm/skwasm_impl/raw/skwasm_module.dart | 31 +------ .../engine/skwasm/skwasm_impl/surface.dart | 84 +++++++++++-------- engine/src/flutter/web_sdk/sdk_rewriter.dart | 3 + .../web_sdk/test/sdk_rewriter_test.dart | 1 + 6 files changed, 74 insertions(+), 97 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/dev/test_dart2wasm.js b/engine/src/flutter/lib/web_ui/dev/test_dart2wasm.js index b48e2403a35..e1d1c32c164 100644 --- a/engine/src/flutter/lib/web_ui/dev/test_dart2wasm.js +++ b/engine/src/flutter/lib/web_ui/dev/test_dart2wasm.js @@ -64,7 +64,8 @@ window.onload = async function () { const skwasmInstance = await skwasm(); window._flutter_skwasmInstance = skwasmInstance; resolve({ - "skwasm": skwasmInstance.asm ?? skwasmInstance.wasmExports, + "skwasm": skwasmInstance.wasmExports, + "skwasmWrapper": skwasmInstance, "ffi": { "memory": skwasmInstance.wasmMemory, } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/raw/raw_image.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/raw/raw_image.dart index 8b6c968e937..c0cd2e25db5 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/raw/raw_image.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/raw/raw_image.dart @@ -5,6 +5,7 @@ @DefaultAsset('skwasm') library skwasm_impl; +import 'dart:_wasm'; import 'dart:ffi'; import 'dart:js_interop'; @@ -39,45 +40,27 @@ external ImageHandle imageCreateFromPixels( int rowByteCount, ); -// We actually want this function to look something like this: -// -// @Native(symbol: 'image_createFromTextureSource', isLeaf: true) -// external ImageHandle imageCreateFromTextureSource( -// JSAny textureSource, -// int width, -// int height, -// SurfaceHandle handle, -// ); -// -// However, this doesn't work because we cannot use extern refs as part of @Native -// annotations currently. For now, we can use JS interop to expose this function -// instead. -extension SkwasmImageExtension on SkwasmInstance { - @JS('wasmExports.image_createFromTextureSource') - external JSNumber imageCreateFromTextureSource( - JSAny textureSource, - JSNumber width, - JSNumber height, - JSNumber surfaceHandle, - ); -} +// We use a wasm import directly here instead of @Native since this uses an externref +// in the function signature. ImageHandle imageCreateFromTextureSource( JSAny frame, int width, int height, SurfaceHandle handle ) => ImageHandle.fromAddress( - skwasmInstance.imageCreateFromTextureSource( - frame, - width.toJS, - height.toJS, - handle.address.toJS, - ).toDartInt + imageCreateFromTextureSourceImpl( + externRefForJSAny(frame), + width.toWasmI32(), + height.toWasmI32(), + handle.address.toWasmI32(), + ).toIntUnsigned() +); +@pragma('wasm:import', 'skwasm.image_createFromTextureSource') +external WasmI32 imageCreateFromTextureSourceImpl( + WasmExternRef? frame, + WasmI32 width, + WasmI32 height, + WasmI32 surfaceHandle, ); @Native(symbol:'image_ref', isLeaf: true) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/raw/skwasm_module.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/raw/skwasm_module.dart index 3063a557aae..d162f190dbe 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/raw/skwasm_module.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/raw/skwasm_module.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:_wasm'; import 'dart:js_interop'; @JS() @@ -17,37 +18,11 @@ extension WebAssemblyMemoryExtension on WebAssemblyMemory { class SkwasmInstance {} extension SkwasmInstanceExtension on SkwasmInstance { - external JSNumber getEmptyTableSlot(); - - // The function here *must* be a directly exported wasm function, not a - // JavaScript function. If you actually need to add a JavaScript function, - // use `addFunction` instead. - external void setWasmTableEntry(JSNumber index, JSAny function); - - external JSNumber addFunction(WebAssemblyFunction function); - external void removeFunction(JSNumber functionPointer); - external WebAssemblyMemory get wasmMemory; } @JS('window._flutter_skwasmInstance') external SkwasmInstance get skwasmInstance; -@JS() -@staticInterop -@anonymous -class WebAssemblyFunctionType { - external factory WebAssemblyFunctionType({ - required JSArray parameters, - required JSArray results, - }); -} - -@JS('WebAssembly.Function') -@staticInterop -class WebAssemblyFunction { - external factory WebAssemblyFunction( - WebAssemblyFunctionType functionType, - JSFunction function - ); -} +@pragma('wasm:import', 'skwasmWrapper.addFunction') +external WasmI32 addFunction(WasmFuncRef function); diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/surface.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/surface.dart index 4827c78b00f..34af06d0d1f 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/surface.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/surface.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:_wasm'; import 'dart:async'; import 'dart:ffi'; import 'dart:js_interop'; @@ -11,7 +12,52 @@ import 'package:ui/src/engine.dart'; import 'package:ui/src/engine/skwasm/skwasm_impl.dart'; import 'package:ui/ui.dart' as ui; +@pragma('wasm:export') +WasmVoid callbackHandler(WasmI32 callbackId, WasmI32 context, WasmExternRef? jsContext) { + // Actually hide this call behind whether skwasm is enabled. Otherwise, the SkwasmCallbackHandler + // won't actually be tree-shaken, and we end up with skwasm imports in non-skwasm builds. + if (FlutterConfiguration.flutterWebUseSkwasm) { + SkwasmCallbackHandler.instance.handleCallback(callbackId, context, jsContext); + } + return WasmVoid(); +} +// This class handles callbacks coming from Skwasm by keeping a map of callback IDs to Completers +class SkwasmCallbackHandler { + SkwasmCallbackHandler._withCallbackPointer(this.callbackPointer); + + factory SkwasmCallbackHandler._() { + final WasmFuncRef wasmFunction = WasmFunction.fromFunction(callbackHandler); + final int functionIndex = addFunction(wasmFunction).toIntUnsigned(); + return SkwasmCallbackHandler._withCallbackPointer( + OnRenderCallbackHandle.fromAddress(functionIndex) + ); + } + static SkwasmCallbackHandler instance = SkwasmCallbackHandler._(); + + final OnRenderCallbackHandle callbackPointer; + final Map> _pendingCallbacks = >{}; + + // Returns a future that will resolve when Skwasm calls back with the given callbackID + Future registerCallback(int callbackId) { + final Completer completer = Completer(); + _pendingCallbacks[callbackId] = completer; + return completer.future; + } + + void handleCallback(WasmI32 callbackId, WasmI32 context, WasmExternRef? jsContext) { + // Skwasm can either callback with a JS object (an externref) or it can call back + // with a simple integer, which usually refers to a pointer on its heap. In order + // to coerce these into a single type, we just make the completers take a JSAny + // that either contains the JS object or a JSNumber that contains the integer value. + final Completer completer = _pendingCallbacks.remove(callbackId.toIntUnsigned())!; + if (!jsContext.isNull) { + completer.complete(jsContext!.toJS); + } else { + completer.complete(context.toIntUnsigned().toJS); + } + } +} class SkwasmSurface { factory SkwasmSurface() { @@ -25,32 +71,16 @@ class SkwasmSurface { SkwasmSurface._fromHandle(this.handle) : threadId = surfaceGetThreadId(handle); final SurfaceHandle handle; - OnRenderCallbackHandle _callbackHandle = nullptr; - final Map> _pendingCallbacks = >{}; final int threadId; void _initialize() { - final WebAssemblyFunction wasmFunction = WebAssemblyFunction( - WebAssemblyFunctionType( - parameters: [ - 'i32'.toJS, - 'i32'.toJS, - 'externref'.toJS - ].toJS, - results: [].toJS - ), - _callbackHandler.toJS, - ); - _callbackHandle = OnRenderCallbackHandle.fromAddress( - skwasmInstance.addFunction(wasmFunction).toDartInt, - ); - surfaceSetCallbackHandler(handle, _callbackHandle); + surfaceSetCallbackHandler(handle, SkwasmCallbackHandler.instance.callbackPointer); } Future renderPicture(SkwasmPicture picture) async { final int callbackId = surfaceRenderPicture(handle, picture.handle); - final DomImageBitmap bitmap = (await _registerCallback(callbackId)) as DomImageBitmap; + final DomImageBitmap bitmap = (await SkwasmCallbackHandler.instance.registerCallback(callbackId)) as DomImageBitmap; return bitmap; } @@ -60,7 +90,7 @@ class SkwasmSurface { image.handle, format.index, ); - final int context = (await _registerCallback(callbackId) as JSNumber).toDartInt; + final int context = (await SkwasmCallbackHandler.instance.registerCallback(callbackId) as JSNumber).toDartInt; final SkDataHandle dataHandle = SkDataHandle.fromAddress(context); final int byteCount = skDataGetSize(dataHandle); final Pointer dataPointer = skDataGetConstPointer(dataHandle).cast(); @@ -72,23 +102,7 @@ class SkwasmSurface { return ByteData.sublistView(output); } - Future _registerCallback(int callbackId) { - final Completer completer = Completer(); - _pendingCallbacks[callbackId] = completer; - return completer.future; - } - - void _callbackHandler(JSNumber callbackId, JSNumber context, JSAny? jsContext) { - final Completer completer = _pendingCallbacks.remove(callbackId.toDartInt)!; - if (jsContext.isUndefinedOrNull) { - completer.complete(context); - } else { - completer.complete(jsContext); - } - } - void dispose() { surfaceDestroy(handle); - skwasmInstance.removeFunction(_callbackHandle.address.toJS); } } diff --git a/engine/src/flutter/web_sdk/sdk_rewriter.dart b/engine/src/flutter/web_sdk/sdk_rewriter.dart index 96b37598a6f..da1aa102a14 100644 --- a/engine/src/flutter/web_sdk/sdk_rewriter.dart +++ b/engine/src/flutter/web_sdk/sdk_rewriter.dart @@ -178,6 +178,9 @@ List getExtraImportsForLibrary(String libraryName) { extraImports.add(entry.value); } } + if (libraryName == 'skwasm_impl') { + extraImports.add("import 'dart:_wasm';"); + } return extraImports; } diff --git a/engine/src/flutter/web_sdk/test/sdk_rewriter_test.dart b/engine/src/flutter/web_sdk/test/sdk_rewriter_test.dart index 2eb2c0795dc..95cfe8256f9 100644 --- a/engine/src/flutter/web_sdk/test/sdk_rewriter_test.dart +++ b/engine/src/flutter/web_sdk/test/sdk_rewriter_test.dart @@ -184,6 +184,7 @@ void printSomething() { "import 'dart:_web_unicode';", "import 'dart:_web_test_fonts';", "import 'dart:_web_locale_keymap' as locale_keymap;", + "import 'dart:_wasm';", ]); // Other libraries (should not have extra imports).