Skip to content

Commit

Permalink
Ensure that Picture::RasterizeToImage destroys Dart persistent values…
Browse files Browse the repository at this point in the history
… on the UI thread (flutter#8182)

The DartPersistentValue used to hold the image callback is tied to a
Dart isolate.  Destructing the DartPersistentValue requires entering
the isolate and must be done on the UI thread.

Fixes flutter/flutter#29379
  • Loading branch information
jason-simmons authored Mar 28, 2019
1 parent 930033d commit 6c948ba
Showing 1 changed file with 36 additions and 45 deletions.
81 changes: 36 additions & 45 deletions lib/ui/painting/picture.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ Dart_Handle Picture::RasterizeToImage(sk_sp<SkPicture> picture,
}

auto* dart_state = UIDartState::Current();
auto image_callback = std::make_unique<tonic::DartPersistentValue>(
dart_state, raw_image_callback);
tonic::DartPersistentValue* image_callback =
new tonic::DartPersistentValue(dart_state, raw_image_callback);
auto unref_queue = dart_state->GetSkiaUnrefQueue();
auto ui_task_runner = dart_state->GetTaskRunners().GetUITaskRunner();
auto gpu_task_runner = dart_state->GetTaskRunners().GetGPUTaskRunner();
Expand All @@ -84,52 +84,43 @@ Dart_Handle Picture::RasterizeToImage(sk_sp<SkPicture> picture,

auto picture_bounds = SkISize::Make(width, height);

auto ui_task = fml::MakeCopyable(
[ui_task_runner, image_callback = std::move(image_callback),
unref_queue](sk_sp<SkImage> raster_image) mutable {
// Send the raster image back to the UI thread for submission to the
// framework.
fml::TaskRunner::RunNowOrPostTask(
ui_task_runner,
fml::MakeCopyable([raster_image,
image_callback = std::move(image_callback),
unref_queue]() mutable {
auto dart_state = image_callback->dart_state().lock();
if (!dart_state) {
// The root isolate could have died in the meantime.
return;
}
tonic::DartState::Scope scope(dart_state);

if (!raster_image) {
tonic::DartInvoke(image_callback->Get(), {Dart_Null()});
return;
}

auto dart_image = CanvasImage::Create();
dart_image->set_image(
{std::move(raster_image), std::move(unref_queue)});
auto* raw_dart_image = tonic::ToDart(std::move(dart_image));

// All done!
tonic::DartInvoke(image_callback->Get(), {raw_dart_image});
}));
});

auto gpu_task = fml::MakeCopyable([gpu_task_runner, picture, picture_bounds,
snapshot_delegate, ui_task]() {
fml::TaskRunner::RunNowOrPostTask(gpu_task_runner, [snapshot_delegate,
picture, picture_bounds,
ui_task]() {
// Snapshot the picture on the GPU thread. This thread has access to the
// GPU contexts that may contain the sole references to texture backed
// images in the picture.
ui_task(snapshot_delegate->MakeRasterSnapshot(picture, picture_bounds));
});
auto ui_task = fml::MakeCopyable([image_callback, unref_queue](
sk_sp<SkImage> raster_image) mutable {
auto dart_state = image_callback->dart_state().lock();
if (!dart_state) {
// The root isolate could have died in the meantime.
return;
}
tonic::DartState::Scope scope(dart_state);

if (!raster_image) {
tonic::DartInvoke(image_callback->Get(), {Dart_Null()});
return;
}

auto dart_image = CanvasImage::Create();
dart_image->set_image({std::move(raster_image), std::move(unref_queue)});
auto* raw_dart_image = tonic::ToDart(std::move(dart_image));

// All done!
tonic::DartInvoke(image_callback->Get(), {raw_dart_image});

// image_callback is associated with the Dart isolate and must be deleted
// on the UI thread
delete image_callback;
});

// Kick things off on the GPU.
gpu_task();
fml::TaskRunner::RunNowOrPostTask(
gpu_task_runner,
[ui_task_runner, snapshot_delegate, picture, picture_bounds, ui_task] {
sk_sp<SkImage> raster_image =
snapshot_delegate->MakeRasterSnapshot(picture, picture_bounds);

fml::TaskRunner::RunNowOrPostTask(
ui_task_runner,
[ui_task, raster_image]() { ui_task(raster_image); });
});

return Dart_Null();
}
Expand Down

0 comments on commit 6c948ba

Please sign in to comment.