Skip to content

Commit 634e499

Browse files
dnfielddnfield
authored
Use hint freed specifically for image disposal (flutter#20754)
* Use hint freed specifically for image disposal
1 parent f6920da commit 634e499

22 files changed

+351
-20
lines changed

‎ci/licenses_golden/licenses_flutter‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ FILE: ../../../flutter/lib/ui/fixtures/hello_loop_2.webp
308308
FILE: ../../../flutter/lib/ui/fixtures/ui_test.dart
309309
FILE: ../../../flutter/lib/ui/geometry.dart
310310
FILE: ../../../flutter/lib/ui/hash_codes.dart
311+
FILE: ../../../flutter/lib/ui/hint_freed_delegate.h
311312
FILE: ../../../flutter/lib/ui/hooks.dart
312313
FILE: ../../../flutter/lib/ui/io_manager.h
313314
FILE: ../../../flutter/lib/ui/isolate_name_server.dart
@@ -337,6 +338,7 @@ FILE: ../../../flutter/lib/ui/painting/image_decoder.h
337338
FILE: ../../../flutter/lib/ui/painting/image_decoder_unittests.cc
338339
FILE: ../../../flutter/lib/ui/painting/image_descriptor.cc
339340
FILE: ../../../flutter/lib/ui/painting/image_descriptor.h
341+
FILE: ../../../flutter/lib/ui/painting/image_dispose_unittests.cc
340342
FILE: ../../../flutter/lib/ui/painting/image_encoding.cc
341343
FILE: ../../../flutter/lib/ui/painting/image_encoding.h
342344
FILE: ../../../flutter/lib/ui/painting/image_encoding_unittests.cc

‎lib/ui/BUILD.gn‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ if (enable_unittests) {
181181
public_configs = [ "//flutter:export_dynamic_symbols" ]
182182

183183
sources = [
184+
"painting/image_dispose_unittests.cc",
184185
"painting/image_encoding_unittests.cc",
185186
"painting/vertices_unittests.cc",
186187
"window/platform_configuration_unittests.cc",

‎lib/ui/fixtures/ui_test.dart‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// Use of this source code is governed by a BSD-style license that can be
44
// found in the LICENSE file.
55

6+
import 'dart:async';
67
import 'dart:typed_data';
78
import 'dart:ui';
89

@@ -73,3 +74,61 @@ Future<void> encodeImageProducesExternalUint8List() async {
7374
void _encodeImage(Image i, int format, void Function(Uint8List result))
7475
native 'EncodeImage';
7576
void _validateExternal(Uint8List result) native 'ValidateExternal';
77+
78+
@pragma('vm:entry-point')
79+
Future<void> pumpImage() async {
80+
const int width = 6000;
81+
const int height = 6000;
82+
final Completer<Image> completer = Completer<Image>();
83+
decodeImageFromPixels(
84+
Uint8List.fromList(List<int>.filled(width * height * 4, 0xFF)),
85+
width,
86+
height,
87+
PixelFormat.rgba8888,
88+
(Image image) => completer.complete(image),
89+
);
90+
final Image image = await completer.future;
91+
92+
final FrameCallback renderBlank = (Duration duration) {
93+
image.dispose();
94+
95+
final PictureRecorder recorder = PictureRecorder();
96+
final Canvas canvas = Canvas(recorder);
97+
canvas.drawRect(Rect.largest, Paint());
98+
final Picture picture = recorder.endRecording();
99+
100+
final SceneBuilder builder = SceneBuilder();
101+
builder.addPicture(Offset.zero, picture);
102+
103+
final Scene scene = builder.build();
104+
window.render(scene);
105+
scene.dispose();
106+
window.onBeginFrame = (Duration duration) {
107+
window.onDrawFrame = _onBeginFrameDone;
108+
};
109+
window.scheduleFrame();
110+
};
111+
112+
final FrameCallback renderImage = (Duration duration) {
113+
final PictureRecorder recorder = PictureRecorder();
114+
final Canvas canvas = Canvas(recorder);
115+
canvas.drawImage(image, Offset.zero, Paint());
116+
final Picture picture = recorder.endRecording();
117+
118+
final SceneBuilder builder = SceneBuilder();
119+
builder.addPicture(Offset.zero, picture);
120+
121+
_captureImageAndPicture(image, picture);
122+
123+
final Scene scene = builder.build();
124+
window.render(scene);
125+
scene.dispose();
126+
window.onBeginFrame = renderBlank;
127+
window.scheduleFrame();
128+
};
129+
130+
window.onBeginFrame = renderImage;
131+
window.scheduleFrame();
132+
}
133+
void _captureImageAndPicture(Image image, Picture picture) native 'CaptureImageAndPicture';
134+
Future<void> _onBeginFrameDone() native 'OnBeginFrameDone';

‎lib/ui/hint_freed_delegate.h‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef FLUTTER_LIB_UI_HINT_FREED_DELEGATE_H_
6+
#define FLUTTER_LIB_UI_HINT_FREED_DELEGATE_H_
7+
8+
namespace flutter {
9+
10+
class HintFreedDelegate {
11+
public:
12+
//----------------------------------------------------------------------------
13+
/// @brief Notifies the engine that native bytes might be freed if a
14+
/// garbage collection ran at the next NotifyIdle period.
15+
///
16+
/// @param[in] size The number of bytes freed. This size adds to any
17+
/// previously supplied value, rather than replacing.
18+
///
19+
virtual void HintFreed(size_t size) = 0;
20+
};
21+
22+
} // namespace flutter
23+
24+
#endif // FLUTTER_LIB_UI_HINT_FREED_DELEGATE_H_

‎lib/ui/painting/image.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ Dart_Handle CanvasImage::toByteData(int format, Dart_Handle callback) {
3737
}
3838

3939
void CanvasImage::dispose() {
40+
auto hint_freed_delegate = UIDartState::Current()->GetHintFreedDelegate();
41+
if (hint_freed_delegate) {
42+
hint_freed_delegate->HintFreed(GetAllocationSize());
43+
}
4044
image_.reset();
4145
ClearDartWrapper();
4246
}

‎lib/ui/painting/image_dispose_unittests.cc‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#define FML_USED_ON_EMBEDDER
6+
7+
#include "flutter/common/task_runners.h"
8+
#include "flutter/fml/synchronization/waitable_event.h"
9+
#include "flutter/lib/ui/painting/image.h"
10+
#include "flutter/lib/ui/painting/picture.h"
11+
#include "flutter/runtime/dart_vm.h"
12+
#include "flutter/shell/common/shell_test.h"
13+
#include "flutter/shell/common/thread_host.h"
14+
#include "flutter/testing/testing.h"
15+
16+
namespace flutter {
17+
namespace testing {
18+
19+
class ImageDisposeTest : public ShellTest {
20+
public:
21+
template <class T>
22+
T* GetNativePeer(Dart_NativeArguments args, int index) {
23+
auto handle = Dart_GetNativeArgument(args, index);
24+
intptr_t peer = 0;
25+
EXPECT_FALSE(Dart_IsError(Dart_GetNativeInstanceField(
26+
handle, tonic::DartWrappable::kPeerIndex, &peer)));
27+
return reinterpret_cast<T*>(peer);
28+
}
29+
30+
// Used to wait on Dart callbacks or Shell task runner flushing
31+
fml::AutoResetWaitableEvent message_latch_;
32+
33+
fml::AutoResetWaitableEvent picture_finalizer_latch_;
34+
static void picture_finalizer(void* isolate_callback_data, void* peer) {
35+
auto latch = reinterpret_cast<fml::AutoResetWaitableEvent*>(peer);
36+
latch->Signal();
37+
}
38+
39+
sk_sp<SkPicture> current_picture_;
40+
sk_sp<SkImage> current_image_;
41+
};
42+
43+
TEST_F(ImageDisposeTest, ImageReleasedAfterFrame) {
44+
auto native_capture_image_and_picture = [&](Dart_NativeArguments args) {
45+
CanvasImage* image = GetNativePeer<CanvasImage>(args, 0);
46+
Picture* picture = GetNativePeer<Picture>(args, 1);
47+
ASSERT_FALSE(image->image()->unique());
48+
ASSERT_FALSE(picture->picture()->unique());
49+
current_image_ = image->image();
50+
current_picture_ = picture->picture();
51+
52+
Dart_NewFinalizableHandle(Dart_GetNativeArgument(args, 1),
53+
&picture_finalizer_latch_, 0, &picture_finalizer);
54+
};
55+
56+
auto native_on_begin_frame_done = [&](Dart_NativeArguments args) {
57+
message_latch_.Signal();
58+
};
59+
60+
Settings settings = CreateSettingsForFixture();
61+
auto task_runner = CreateNewThread();
62+
TaskRunners task_runners("test", // label
63+
GetCurrentTaskRunner(), // platform
64+
task_runner, // raster
65+
task_runner, // ui
66+
task_runner // io
67+
);
68+
69+
AddNativeCallback("CaptureImageAndPicture",
70+
CREATE_NATIVE_ENTRY(native_capture_image_and_picture));
71+
AddNativeCallback("OnBeginFrameDone",
72+
CREATE_NATIVE_ENTRY(native_on_begin_frame_done));
73+
74+
std::unique_ptr<Shell> shell = CreateShell(std::move(settings), task_runners);
75+
76+
ASSERT_TRUE(shell->IsSetup());
77+
78+
SetViewportMetrics(shell.get(), 800, 600);
79+
80+
shell->GetPlatformView()->NotifyCreated();
81+
82+
auto configuration = RunConfiguration::InferFromSettings(settings);
83+
configuration.SetEntrypoint("pumpImage");
84+
85+
shell->RunEngine(std::move(configuration), [&](auto result) {
86+
ASSERT_EQ(result, Engine::RunStatus::Success);
87+
});
88+
89+
message_latch_.Wait();
90+
91+
ASSERT_TRUE(current_picture_);
92+
ASSERT_TRUE(current_image_);
93+
94+
// Simulate a large notify idle, as the animator would do
95+
// when it has no frames left.
96+
// On slower machines, this is especially important - we capture that
97+
// this happens normally in devicelab bnechmarks like large_image_changer.
98+
NotifyIdle(shell.get(), Dart_TimelineGetMicros() + 100000);
99+
100+
picture_finalizer_latch_.Wait();
101+
102+
// Force a drain the SkiaUnrefQueue.
103+
message_latch_.Reset();
104+
task_runner->PostTask([&, io_manager = shell->GetIOManager()]() {
105+
io_manager->GetSkiaUnrefQueue()->Drain();
106+
message_latch_.Signal();
107+
});
108+
message_latch_.Wait();
109+
110+
EXPECT_TRUE(current_picture_->unique());
111+
current_picture_.reset();
112+
113+
EXPECT_TRUE(current_image_->unique());
114+
current_image_.reset();
115+
116+
shell->GetPlatformView()->NotifyDestroyed();
117+
DestroyShell(std::move(shell), std::move(task_runners));
118+
}
119+
120+
} // namespace testing
121+
} // namespace flutter

‎lib/ui/ui_dart_state.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ UIDartState::UIDartState(
1818
TaskObserverAdd add_callback,
1919
TaskObserverRemove remove_callback,
2020
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
21+
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
2122
fml::WeakPtr<IOManager> io_manager,
2223
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
2324
fml::WeakPtr<ImageDecoder> image_decoder,
@@ -31,6 +32,7 @@ UIDartState::UIDartState(
3132
add_callback_(std::move(add_callback)),
3233
remove_callback_(std::move(remove_callback)),
3334
snapshot_delegate_(std::move(snapshot_delegate)),
35+
hint_freed_delegate_(std::move(hint_freed_delegate)),
3436
io_manager_(std::move(io_manager)),
3537
skia_unref_queue_(std::move(skia_unref_queue)),
3638
image_decoder_(std::move(image_decoder)),
@@ -136,6 +138,10 @@ fml::WeakPtr<SnapshotDelegate> UIDartState::GetSnapshotDelegate() const {
136138
return snapshot_delegate_;
137139
}
138140

141+
fml::WeakPtr<HintFreedDelegate> UIDartState::GetHintFreedDelegate() const {
142+
return hint_freed_delegate_;
143+
}
144+
139145
fml::WeakPtr<GrDirectContext> UIDartState::GetResourceContext() const {
140146
if (!io_manager_) {
141147
return {};

‎lib/ui/ui_dart_state.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "flutter/fml/build_config.h"
1616
#include "flutter/fml/memory/weak_ptr.h"
1717
#include "flutter/fml/synchronization/waitable_event.h"
18+
#include "flutter/lib/ui/hint_freed_delegate.h"
1819
#include "flutter/lib/ui/io_manager.h"
1920
#include "flutter/lib/ui/isolate_name_server/isolate_name_server.h"
2021
#include "flutter/lib/ui/painting/image_decoder.h"
@@ -60,6 +61,8 @@ class UIDartState : public tonic::DartState {
6061

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

64+
fml::WeakPtr<HintFreedDelegate> GetHintFreedDelegate() const;
65+
6366
fml::WeakPtr<GrDirectContext> GetResourceContext() const;
6467

6568
fml::WeakPtr<ImageDecoder> GetImageDecoder() const;
@@ -87,6 +90,7 @@ class UIDartState : public tonic::DartState {
8790
TaskObserverAdd add_callback,
8891
TaskObserverRemove remove_callback,
8992
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
93+
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
9094
fml::WeakPtr<IOManager> io_manager,
9195
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
9296
fml::WeakPtr<ImageDecoder> image_decoder,
@@ -113,6 +117,7 @@ class UIDartState : public tonic::DartState {
113117
const TaskObserverAdd add_callback_;
114118
const TaskObserverRemove remove_callback_;
115119
fml::WeakPtr<SnapshotDelegate> snapshot_delegate_;
120+
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate_;
116121
fml::WeakPtr<IOManager> io_manager_;
117122
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue_;
118123
fml::WeakPtr<ImageDecoder> image_decoder_;

‎runtime/dart_isolate.cc‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
5858
TaskRunners task_runners,
5959
std::unique_ptr<PlatformConfiguration> platform_configuration,
6060
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
61+
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
6162
fml::WeakPtr<IOManager> io_manager,
6263
fml::RefPtr<SkiaUnrefQueue> unref_queue,
6364
fml::WeakPtr<ImageDecoder> image_decoder,
@@ -84,15 +85,16 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
8485

8586
auto isolate_data = std::make_unique<std::shared_ptr<DartIsolate>>(
8687
std::shared_ptr<DartIsolate>(new DartIsolate(
87-
settings, // settings
88-
task_runners, // task runners
89-
std::move(snapshot_delegate), // snapshot delegate
90-
std::move(io_manager), // IO manager
91-
std::move(unref_queue), // Skia unref queue
92-
std::move(image_decoder), // Image Decoder
93-
advisory_script_uri, // advisory URI
94-
advisory_script_entrypoint, // advisory entrypoint
95-
true // is_root_isolate
88+
settings, // settings
89+
task_runners, // task runners
90+
std::move(snapshot_delegate), // snapshot delegate
91+
std::move(hint_freed_delegate), // hint freed delegate
92+
std::move(io_manager), // IO manager
93+
std::move(unref_queue), // Skia unref queue
94+
std::move(image_decoder), // Image Decoder
95+
advisory_script_uri, // advisory URI
96+
advisory_script_entrypoint, // advisory entrypoint
97+
true // is_root_isolate
9698
)));
9799

98100
DartErrorString error;
@@ -120,6 +122,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
120122
DartIsolate::DartIsolate(const Settings& settings,
121123
TaskRunners task_runners,
122124
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
125+
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
123126
fml::WeakPtr<IOManager> io_manager,
124127
fml::RefPtr<SkiaUnrefQueue> unref_queue,
125128
fml::WeakPtr<ImageDecoder> image_decoder,
@@ -130,6 +133,7 @@ DartIsolate::DartIsolate(const Settings& settings,
130133
settings.task_observer_add,
131134
settings.task_observer_remove,
132135
std::move(snapshot_delegate),
136+
std::move(hint_freed_delegate),
133137
std::move(io_manager),
134138
std::move(unref_queue),
135139
std::move(image_decoder),
@@ -603,6 +607,7 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(
603607
null_task_runners, // task runners
604608
nullptr, // platform_configuration
605609
{}, // snapshot delegate
610+
{}, // Hint freed delegate
606611
{}, // IO Manager
607612
{}, // Skia unref queue
608613
{}, // Image Decoder
@@ -706,6 +711,7 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
706711
(*isolate_group_data)->GetSettings(), // settings
707712
null_task_runners, // task_runners
708713
fml::WeakPtr<SnapshotDelegate>{}, // snapshot_delegate
714+
fml::WeakPtr<HintFreedDelegate>{}, // hint_freed_delegate
709715
fml::WeakPtr<IOManager>{}, // io_manager
710716
fml::RefPtr<SkiaUnrefQueue>{}, // unref_queue
711717
fml::WeakPtr<ImageDecoder>{}, // image_decoder
@@ -749,6 +755,7 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data,
749755
(*isolate_group_data)->GetSettings(), // settings
750756
null_task_runners, // task_runners
751757
fml::WeakPtr<SnapshotDelegate>{}, // snapshot_delegate
758+
fml::WeakPtr<HintFreedDelegate>{}, // hint_freed_delegate
752759
fml::WeakPtr<IOManager>{}, // io_manager
753760
fml::RefPtr<SkiaUnrefQueue>{}, // unref_queue
754761
fml::WeakPtr<ImageDecoder>{}, // image_decoder

0 commit comments

Comments
 (0)