Skip to content

Commit

Permalink
Ensure that Scene::toImage renders texture backed images. (flutter#6636)
Browse files Browse the repository at this point in the history
TL;DR: Offscreen surface is created on the render thread and device to host
transfer performed there before task completion on the UI thread.

While attempting to snapshot layer trees, the engine was attempting to use the
IO thread context. The reasoning was that this would be safe to do because any
textures uploaded to the GPU as a result of async texture upload would have
originated from this context and hence the handles would be valid in either
context. As it turns out, while the handles are valid, Skia does not support
this use-case because cross-context images transfer ownership of the image from
one context to another. So, when we made the hop from the UI thread to the IO
thread (for snapshotting), if either the UI or GPU threads released the last
reference to the texture backed image, the image would be invalid. This led to
such images being absent from the layer tree snapshot.

Simply referencing the images as they are being used on the IO thread is not
sufficient because accessing images on one context after their ownership has
already been transferred to another is not safe behavior (from Skia's
perspective, the handles are still valid in the sharegroup).

To work around these issues, it was decided that an offscreen render target
would be created on the render thread. The color attachment of this render
target could then be transferred as a cross context image to the IO thread for
the device to host tranfer.

Again, this is currently not quite possible because the only way to create
cross context images is from encoded data. Till Skia exposes the functionality
to create cross-context images from textures in one context, we do a device to
host transfer on the GPU thread. The side effect of this is that this is now
part of the frame workload (image compression, which dominate the wall time,
is still done of the IO thread).

A minor side effect of this patch is that the GPU latch needs to be waited on
before the UI thread tasks can be completed before shell initialization.
  • Loading branch information
chinmaygarde authored Oct 23, 2018
1 parent 25d0507 commit 20c805c
Show file tree
Hide file tree
Showing 16 changed files with 202 additions and 117 deletions.
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ FILE: ../../../flutter/lib/ui/isolate_name_server/isolate_name_server_natives.cc
FILE: ../../../flutter/lib/ui/isolate_name_server/isolate_name_server_natives.h
FILE: ../../../flutter/lib/ui/plugins/callback_cache.cc
FILE: ../../../flutter/lib/ui/plugins/callback_cache.h
FILE: ../../../flutter/lib/ui/snapshot_delegate.h
FILE: ../../../flutter/runtime/dart_service_isolate_unittests.cc
FILE: ../../../flutter/shell/common/isolate_configuration.cc
FILE: ../../../flutter/shell/common/isolate_configuration.h
Expand Down
3 changes: 2 additions & 1 deletion lib/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ source_set("ui") {
"painting/canvas.h",
"painting/codec.cc",
"painting/codec.h",
"painting/engine_layer.h",
"painting/engine_layer.cc",
"painting/engine_layer.h",
"painting/frame_info.cc",
"painting/frame_info.h",
"painting/gradient.cc",
Expand Down Expand Up @@ -65,6 +65,7 @@ source_set("ui") {
"semantics/semantics_update.h",
"semantics/semantics_update_builder.cc",
"semantics/semantics_update_builder.h",
"snapshot_delegate.h",
"text/asset_manager_font_provider.cc",
"text/asset_manager_font_provider.h",
"text/font_collection.cc",
Expand Down
139 changes: 51 additions & 88 deletions lib/ui/compositing/scene.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,46 +53,6 @@ void Scene::dispose() {
ClearDartWrapper();
}

static sk_sp<SkImage> CreateSceneSnapshot(GrContext* context,
sk_sp<SkPicture> picture,
const SkSize& size) {
TRACE_EVENT0("flutter", "CreateSceneSnapshot");
auto image_info =
SkImageInfo::MakeN32Premul(SkISize::Make(size.width(), size.height()));

sk_sp<SkSurface> surface;

if (context) {
surface = SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, image_info);
}

if (!surface) {
surface = SkSurface::MakeRaster(image_info);
}

if (!surface) {
return nullptr;
}

auto canvas = surface->getCanvas();

if (!canvas) {
return nullptr;
}

if (picture) {
canvas->drawPicture(picture.get());
}

auto snapshot = surface->makeImageSnapshot();

if (!snapshot) {
return nullptr;
}

return snapshot->makeRasterImage();
}

Dart_Handle Scene::toImage(uint32_t width,
uint32_t height,
Dart_Handle raw_image_callback) {
Expand All @@ -110,67 +70,70 @@ Dart_Handle Scene::toImage(uint32_t width,
}

auto dart_state = UIDartState::Current();

auto image_callback = std::make_unique<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();
auto snapshot_delegate = dart_state->GetSnapshotDelegate();

// We can't create an image on this task runner because we don't have a
// graphics context. Even if we did, it would be slow anyway. Also, this
// thread owns the sole reference to the layer tree. So we flatten the layer
// tree into a picture and use that as the thread transport mechanism.

auto bounds_size = SkSize::Make(width, height);
auto picture = m_layerTree->Flatten(SkRect::MakeSize(bounds_size));
auto picture_bounds = SkISize::Make(width, height);
auto picture = m_layerTree->Flatten(SkRect::MakeWH(width, height));

if (!picture) {
// Already in Dart scope.
return tonic::ToDart("Could not flatten scene into a layer tree.");
}

auto resource_context = dart_state->GetResourceContext();
auto ui_task_runner = dart_state->GetTaskRunners().GetUITaskRunner();
auto unref_queue = dart_state->GetSkiaUnrefQueue();

// The picture has been prepared on the UI thread.
dart_state->GetTaskRunners().GetIOTaskRunner()->PostTask(
fml::MakeCopyable([picture = std::move(picture), //
bounds_size, //
resource_context = std::move(resource_context), //
ui_task_runner = std::move(ui_task_runner), //
image_callback = std::move(image_callback), //
unref_queue = std::move(unref_queue) //
]() mutable {
// Snapshot the picture on the IO thread that contains an optional
// GrContext.
auto image = CreateSceneSnapshot(resource_context.get(),
std::move(picture), bounds_size);

// Send the image back to the UI thread for submission back to the
// framework.
ui_task_runner->PostTask(
fml::MakeCopyable([image = std::move(image), //
image_callback = std::move(image_callback), //
unref_queue = std::move(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 (!image) {
tonic::DartInvoke(image_callback->Get(), {Dart_Null()});
return;
}

auto dart_image = CanvasImage::Create();
dart_image->set_image({std::move(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 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.
ui_task_runner->PostTask(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]() {
gpu_task_runner->PostTask([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));
});
});

// Kick things off on the GPU.
gpu_task();

return Dart_Null();
}
Expand Down
21 changes: 21 additions & 0 deletions lib/ui/snapshot_delegate.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2018 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_LIB_UI_SNAPSHOT_DELEGATE_H_
#define FLUTTER_LIB_UI_SNAPSHOT_DELEGATE_H_

#include "third_party/skia/include/core/SkImage.h"
#include "third_party/skia/include/core/SkPicture.h"

namespace blink {

class SnapshotDelegate {
public:
virtual sk_sp<SkImage> MakeRasterSnapshot(sk_sp<SkPicture> picture,
SkISize picture_size) = 0;
};

} // namespace blink

#endif // FLUTTER_LIB_UI_SNAPSHOT_DELEGATE_H_
6 changes: 6 additions & 0 deletions lib/ui/ui_dart_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace blink {
UIDartState::UIDartState(TaskRunners task_runners,
TaskObserverAdd add_callback,
TaskObserverRemove remove_callback,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<GrContext> resource_context,
fml::RefPtr<flow::SkiaUnrefQueue> skia_unref_queue,
std::string advisory_script_uri,
Expand All @@ -25,6 +26,7 @@ UIDartState::UIDartState(TaskRunners task_runners,
: task_runners_(std::move(task_runners)),
add_callback_(std::move(add_callback)),
remove_callback_(std::move(remove_callback)),
snapshot_delegate_(std::move(snapshot_delegate)),
resource_context_(std::move(resource_context)),
advisory_script_uri_(std::move(advisory_script_uri)),
advisory_script_entrypoint_(std::move(advisory_script_entrypoint)),
Expand Down Expand Up @@ -99,6 +101,10 @@ void UIDartState::AddOrRemoveTaskObserver(bool add) {
}
}

fml::WeakPtr<SnapshotDelegate> UIDartState::GetSnapshotDelegate() const {
return snapshot_delegate_;
}

fml::WeakPtr<GrContext> UIDartState::GetResourceContext() const {
return resource_context_;
}
Expand Down
5 changes: 5 additions & 0 deletions lib/ui/ui_dart_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "flutter/fml/build_config.h"
#include "flutter/fml/memory/weak_ptr.h"
#include "flutter/lib/ui/isolate_name_server/isolate_name_server.h"
#include "flutter/lib/ui/snapshot_delegate.h"
#include "third_party/dart/runtime/include/dart_api.h"
#include "third_party/skia/include/gpu/GrContext.h"
#include "third_party/tonic/dart_microtask_queue.h"
Expand Down Expand Up @@ -47,6 +48,8 @@ class UIDartState : public tonic::DartState {

fml::RefPtr<flow::SkiaUnrefQueue> GetSkiaUnrefQueue() const;

fml::WeakPtr<SnapshotDelegate> GetSnapshotDelegate() const;

fml::WeakPtr<GrContext> GetResourceContext() const;

IsolateNameServer* GetIsolateNameServer();
Expand All @@ -68,6 +71,7 @@ class UIDartState : public tonic::DartState {
UIDartState(TaskRunners task_runners,
TaskObserverAdd add_callback,
TaskObserverRemove remove_callback,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<GrContext> resource_context,
fml::RefPtr<flow::SkiaUnrefQueue> skia_unref_queue,
std::string advisory_script_uri,
Expand All @@ -89,6 +93,7 @@ class UIDartState : public tonic::DartState {
const TaskRunners task_runners_;
const TaskObserverAdd add_callback_;
const TaskObserverRemove remove_callback_;
fml::WeakPtr<SnapshotDelegate> snapshot_delegate_;
fml::WeakPtr<GrContext> resource_context_;
const std::string advisory_script_uri_;
const std::string advisory_script_entrypoint_;
Expand Down
22 changes: 14 additions & 8 deletions runtime/dart_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
fml::RefPtr<DartSnapshot> shared_snapshot,
TaskRunners task_runners,
std::unique_ptr<Window> window,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<GrContext> resource_context,
fml::RefPtr<flow::SkiaUnrefQueue> unref_queue,
std::string advisory_script_uri,
Expand All @@ -54,14 +55,15 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
// isolate lifecycle is entirely managed by the VM).
auto root_embedder_data = std::make_unique<std::shared_ptr<DartIsolate>>(
std::make_shared<DartIsolate>(
vm, // VM
std::move(isolate_snapshot), // isolate snapshot
std::move(shared_snapshot), // shared snapshot
task_runners, // task runners
std::move(resource_context), // resource context
std::move(unref_queue), // skia unref queue
advisory_script_uri, // advisory URI
advisory_script_entrypoint, // advisory entrypoint
vm, // VM
std::move(isolate_snapshot), // isolate snapshot
std::move(shared_snapshot), // shared snapshot
task_runners, // task runners
std::move(snapshot_delegate), // snapshot delegate
std::move(resource_context), // resource context
std::move(unref_queue), // skia unref queue
advisory_script_uri, // advisory URI
advisory_script_entrypoint, // advisory entrypoint
nullptr // child isolate preparer will be set when this isolate is
// prepared to run
));
Expand Down Expand Up @@ -101,6 +103,7 @@ DartIsolate::DartIsolate(DartVM* vm,
fml::RefPtr<DartSnapshot> isolate_snapshot,
fml::RefPtr<DartSnapshot> shared_snapshot,
TaskRunners task_runners,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<GrContext> resource_context,
fml::RefPtr<flow::SkiaUnrefQueue> unref_queue,
std::string advisory_script_uri,
Expand All @@ -109,6 +112,7 @@ DartIsolate::DartIsolate(DartVM* vm,
: UIDartState(std::move(task_runners),
vm->GetSettings().task_observer_add,
vm->GetSettings().task_observer_remove,
std::move(snapshot_delegate),
std::move(resource_context),
std::move(unref_queue),
advisory_script_uri,
Expand Down Expand Up @@ -520,6 +524,7 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(
vm->GetSharedSnapshot(), // shared snapshot
null_task_runners, // task runners
nullptr, // window
{}, // snapshot delegate
{}, // resource context
{}, // unref queue
advisory_script_uri == nullptr ? ""
Expand Down Expand Up @@ -630,6 +635,7 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(
(*raw_embedder_isolate)->GetIsolateSnapshot(), // isolate_snapshot
(*raw_embedder_isolate)->GetSharedSnapshot(), // shared_snapshot
null_task_runners, // task_runners
fml::WeakPtr<SnapshotDelegate>{}, // snapshot_delegate
fml::WeakPtr<GrContext>{}, // resource_context
nullptr, // unref_queue
advisory_script_uri, // advisory_script_uri
Expand Down
3 changes: 3 additions & 0 deletions runtime/dart_isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "flutter/fml/compiler_specific.h"
#include "flutter/fml/macros.h"
#include "flutter/fml/mapping.h"
#include "flutter/lib/ui/snapshot_delegate.h"
#include "flutter/lib/ui/ui_dart_state.h"
#include "flutter/lib/ui/window/window.h"
#include "flutter/runtime/dart_snapshot.h"
Expand Down Expand Up @@ -44,6 +45,7 @@ class DartIsolate : public UIDartState {
fml::RefPtr<DartSnapshot> shared_snapshot,
TaskRunners task_runners,
std::unique_ptr<Window> window,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<GrContext> resource_context,
fml::RefPtr<flow::SkiaUnrefQueue> unref_queue,
std::string advisory_script_uri,
Expand All @@ -54,6 +56,7 @@ class DartIsolate : public UIDartState {
fml::RefPtr<DartSnapshot> isolate_snapshot,
fml::RefPtr<DartSnapshot> shared_snapshot,
TaskRunners task_runners,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<GrContext> resource_context,
fml::RefPtr<flow::SkiaUnrefQueue> unref_queue,
std::string advisory_script_uri,
Expand Down
2 changes: 2 additions & 0 deletions runtime/dart_isolate_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
vm->GetSharedSnapshot(), // shared snapshot
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // resource context
nullptr, // unref qeueue
"main.dart", // advisory uri
Expand Down Expand Up @@ -64,6 +65,7 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) {
vm->GetSharedSnapshot(), // shared snapshot
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // resource context
nullptr, // unref qeueue
"main.dart", // advisory uri
Expand Down
Loading

0 comments on commit 20c805c

Please sign in to comment.