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

Commit

Permalink
Fix async image loading issues in skwasm. (#47117)
Browse files Browse the repository at this point in the history
This fixes flutter/flutter#134045

There were a few different issues here:
* We need to do our own message passing for rendering pictures. The async methods provided by emscripten have their own queue that can drain synchronously, so basically it's not guaranteed to be FIFO with other messages sent to the web worker or main thread.
* When we drop frames, we should only drop intermediate frames, so that when the rendering flurry stops that the frame that is displayed is the last one that was actually requested.
* We need to reset the GL context after lazy image creation, otherwise skia's renderer gets into a bad state for that frame.
  • Loading branch information
eyebrowsoffire authored and harryterkelsen committed Oct 23, 2023
1 parent f4fa7db commit cdac5ca
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 26 deletions.
51 changes: 42 additions & 9 deletions lib/web_ui/lib/src/engine/scene_view.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@ abstract class PictureRenderer {
FutureOr<DomImageBitmap> renderPicture(ScenePicture picture);
}

class _SceneRender {
_SceneRender(this.scene, this._completer) {
scene.beginRender();
}

final EngineScene scene;
final Completer<void> _completer;

void done() {
scene.endRender();
_completer.complete();
}
}

// This class builds a DOM tree that composites an `EngineScene`.
class EngineSceneView {
factory EngineSceneView(PictureRenderer pictureRenderer) {
Expand All @@ -30,16 +44,38 @@ class EngineSceneView {

List<SliceContainer> containers = <SliceContainer>[];

int queuedRenders = 0;
static const int kMaxQueuedRenders = 3;
_SceneRender? _currentRender;
_SceneRender? _nextRender;

Future<void> renderScene(EngineScene scene) {
if (_currentRender != null) {
// If a scene is already queued up, drop it and queue this one up instead
// so that the scene view always displays the most recently requested scene.
_nextRender?.done();
final Completer<void> completer = Completer<void>();
_nextRender = _SceneRender(scene, completer);
return completer.future;
}
final Completer<void> completer = Completer<void>();
_currentRender = _SceneRender(scene, completer);
_kickRenderLoop();
return completer.future;
}

Future<void> renderScene(EngineScene scene) async {
if (queuedRenders >= kMaxQueuedRenders) {
Future<void> _kickRenderLoop() async {
final _SceneRender current = _currentRender!;
await _renderScene(current.scene);
current.done();
_currentRender = _nextRender;
_nextRender = null;
if (_currentRender == null) {
return;
} else {
return _kickRenderLoop();
}
queuedRenders += 1;
}

scene.beginRender();
Future<void> _renderScene(EngineScene scene) async {
final List<LayerSlice> slices = scene.rootLayer.slices;
final Iterable<Future<DomImageBitmap?>> renderFutures = slices.map(
(LayerSlice slice) async => switch (slice) {
Expand Down Expand Up @@ -113,9 +149,6 @@ class EngineSceneView {
sceneElement.removeChild(currentElement);
currentElement = sibling;
}
scene.endRender();

queuedRenders -= 1;
}
}

Expand Down
4 changes: 4 additions & 0 deletions lib/web_ui/skwasm/image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ class TextureSourceImageGenerator : public GrExternalTextureGenerator {

auto backendTexture = GrBackendTextures::MakeGL(
fInfo.width(), fInfo.height(), mipmapped, glInfo);

// In order to bind the image source to the texture, makeTexture has changed
// which texture is "in focus" for the WebGL context.
GrAsDirectContext(context)->resetContext(kTextureBinding_GrGLBackendState);
return std::make_unique<ExternalWebGLTexture>(
backendTexture, glInfo.fID, emscripten_webgl_get_current_context());
}
Expand Down
13 changes: 13 additions & 0 deletions lib/web_ui/skwasm/library_skwasm_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ mergeInto(LibraryManager.library, {
return;
}
switch (skwasmMessage) {
case 'renderPicture':
_surface_renderPictureOnWorker(data.surface, data.picture, data.callbackId);
return;
case 'onRenderComplete':
_surface_onRenderComplete(data.surface, data.callbackId, data.imageBitmap);
return;
Expand All @@ -51,6 +54,14 @@ mergeInto(LibraryManager.library, {
PThread.pthreads[threadId].addEventListener("message", eventListener);
}
};
_skwasm_dispatchRenderPicture = function(threadId, surfaceHandle, pictureHandle, callbackId) {
PThread.pthreads[threadId].postMessage({
skwasmMessage: 'renderPicture',
surface: surfaceHandle,
picture: pictureHandle,
callbackId,
});
};
_skwasm_createOffscreenCanvas = function(width, height) {
const canvas = new OffscreenCanvas(width, height);
var contextAttributes = {
Expand Down Expand Up @@ -114,6 +125,8 @@ mergeInto(LibraryManager.library, {
skwasm_disposeAssociatedObjectOnThread__deps: ['$skwasm_support_setup'],
skwasm_registerMessageListener: function() {},
skwasm_registerMessageListener__deps: ['$skwasm_support_setup'],
skwasm_dispatchRenderPicture: function() {},
skwasm_dispatchRenderPicture__deps: ['$skwasm_support_setup'],
skwasm_createOffscreenCanvas: function () {},
skwasm_createOffscreenCanvas__deps: ['$skwasm_support_setup'],
skwasm_resizeCanvas: function () {},
Expand Down
5 changes: 5 additions & 0 deletions lib/web_ui/skwasm/skwasm_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <emscripten/threading.h>
#include <cinttypes>
#include "third_party/skia/include/core/SkPicture.h"

namespace Skwasm {
class Surface;
Expand All @@ -19,6 +20,10 @@ extern SkwasmObject skwasm_getAssociatedObject(void* pointer);
extern void skwasm_disposeAssociatedObjectOnThread(unsigned long threadId,
void* pointer);
extern void skwasm_registerMessageListener(pthread_t threadId);
extern void skwasm_dispatchRenderPicture(unsigned long threadId,
Skwasm::Surface* surface,
SkPicture* picture,
uint32_t callbackId);
extern uint32_t skwasm_createOffscreenCanvas(int width, int height);
extern void skwasm_resizeCanvas(uint32_t contextHandle, int width, int height);
extern void skwasm_captureImageBitmap(Skwasm::Surface* surfaceHandle,
Expand Down
20 changes: 9 additions & 11 deletions lib/web_ui/skwasm/surface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ uint32_t Surface::renderPicture(SkPicture* picture) {
assert(emscripten_is_main_browser_thread());
uint32_t callbackId = ++_currentCallbackId;
picture->ref();
emscripten_dispatch_to_thread(_thread, EM_FUNC_SIG_VIII,
reinterpret_cast<void*>(fRenderPicture),
nullptr, this, picture, callbackId);
skwasm_dispatchRenderPicture(_thread, this, picture, callbackId);
return callbackId;
}

Expand Down Expand Up @@ -138,7 +136,7 @@ void Surface::_recreateSurface() {
}

// Worker thread only
void Surface::_renderPicture(const SkPicture* picture, uint32_t callbackId) {
void Surface::renderPictureOnWorker(SkPicture* picture, uint32_t callbackId) {
SkRect pictureRect = picture->cullRect();
SkIRect roundedOutRect;
pictureRect.roundOut(&roundedOutRect);
Expand Down Expand Up @@ -195,13 +193,6 @@ void Surface::fDispose(Surface* surface) {
surface->_dispose();
}

void Surface::fRenderPicture(Surface* surface,
SkPicture* picture,
uint32_t callbackId) {
surface->_renderPicture(picture, callbackId);
picture->unref();
}

void Surface::fOnRasterizeComplete(Surface* surface,
SkData* imageData,
uint32_t callbackId) {
Expand Down Expand Up @@ -239,6 +230,13 @@ SKWASM_EXPORT uint32_t surface_renderPicture(Surface* surface,
return surface->renderPicture(picture);
}

SKWASM_EXPORT void surface_renderPictureOnWorker(Surface* surface,
SkPicture* picture,
uint32_t callbackId) {
surface->renderPictureOnWorker(picture, callbackId);
picture->unref();
}

SKWASM_EXPORT uint32_t surface_rasterizeImage(Surface* surface,
SkImage* image,
ImageByteFormat format) {
Expand Down
7 changes: 3 additions & 4 deletions lib/web_ui/skwasm/surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,15 @@ class Surface {
std::unique_ptr<TextureSourceWrapper> createTextureSourceWrapper(
SkwasmObject textureSource);

// Worker thread
void renderPictureOnWorker(SkPicture* picture, uint32_t callbackId);

private:
void _runWorker();
void _init();
void _dispose();
void _resizeCanvasToFit(int width, int height);
void _recreateSurface();
void _renderPicture(const SkPicture* picture, uint32_t callbackId);
void _rasterizeImage(SkImage* image,
ImageByteFormat format,
uint32_t callbackId);
Expand All @@ -99,9 +101,6 @@ class Surface {
pthread_t _thread;

static void fDispose(Surface* surface);
static void fRenderPicture(Surface* surface,
SkPicture* picture,
uint32_t callbackId);
static void fOnRenderComplete(Surface* surface,
uint32_t callbackId,
SkwasmObject imageBitmap);
Expand Down
33 changes: 31 additions & 2 deletions lib/web_ui/test/engine/scene_view_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class StubPictureRenderer implements PictureRenderer {

@override
Future<DomImageBitmap> renderPicture(ScenePicture picture) async {
renderedPictures.add(picture);
final ui.Rect cullRect = picture.cullRect;
final DomImageBitmap bitmap = (await createSizedImageBitmap(
scratchCanvasElement,
Expand All @@ -32,12 +33,16 @@ class StubPictureRenderer implements PictureRenderer {
cullRect.height.toInt()))!;
return bitmap;
}

List<ScenePicture> renderedPictures = <ScenePicture>[];
}

void testMain() {
late EngineSceneView sceneView;
late StubPictureRenderer stubPictureRenderer;
setUp(() {
sceneView = EngineSceneView(StubPictureRenderer());
stubPictureRenderer = StubPictureRenderer();
sceneView = EngineSceneView(stubPictureRenderer);
});

test('SceneView places canvas according to device-pixel ratio', () async {
Expand Down Expand Up @@ -74,7 +79,7 @@ void testMain() {
debugOverrideDevicePixelRatio(null);
});

test('SceneView places canvas according to device-pixel ratio', () async {
test('SceneView places platform view according to device-pixel ratio', () async {
debugOverrideDevicePixelRatio(2.0);

final PlatformView platformView = PlatformView(
Expand Down Expand Up @@ -103,4 +108,28 @@ void testMain() {

debugOverrideDevicePixelRatio(null);
});

test('SceneView always renders most recent picture and skips intermediate pictures', () async {
final List<StubPicture> pictures = <StubPicture>[];
final List<Future<void>> renderFutures = <Future<void>>[];
for (int i = 1; i < 20; i++) {
final StubPicture picture = StubPicture(const ui.Rect.fromLTWH(
50,
80,
100,
120,
));
pictures.add(picture);
final EngineRootLayer rootLayer = EngineRootLayer();
rootLayer.slices.add(PictureSlice(picture));
final EngineScene scene = EngineScene(rootLayer);
renderFutures.add(sceneView.renderScene(scene));
}
await Future.wait(renderFutures);

// Should just render the first and last pictures and skip the one inbetween.
expect(stubPictureRenderer.renderedPictures.length, 2);
expect(stubPictureRenderer.renderedPictures.first, pictures.first);
expect(stubPictureRenderer.renderedPictures.last, pictures.last);
});
}

0 comments on commit cdac5ca

Please sign in to comment.