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

Add support for loading asset directly from ImmutableBuffer #32999

Merged
merged 31 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9623154
Add support for loading asset directly from ImmutableBuffer
jonahwilliams Apr 29, 2022
ba639ba
Update platform_configuration.h
jonahwilliams Apr 29, 2022
fe93694
make delegation the same
jonahwilliams Apr 29, 2022
7726151
update gn file
jonahwilliams Apr 29, 2022
9f45888
use fixture
jonahwilliams Apr 29, 2022
6963e65
add asset dir
jonahwilliams Apr 29, 2022
5e2f694
Merge branch 'master' of github.com:flutter/engine into immutable_buffer
jonahwilliams Apr 29, 2022
769a9d8
make async
jonahwilliams Apr 29, 2022
68f5b9b
Update painting.dart
jonahwilliams Apr 29, 2022
f216c48
Actually set length
jonahwilliams Apr 30, 2022
bae7201
Merge branch 'immutable_buffer' of github.com:jonahwilliams/engine in…
jonahwilliams Apr 30, 2022
61ec3a3
Update painting.dart
jonahwilliams Apr 30, 2022
614a997
Update painting.dart
jonahwilliams Apr 30, 2022
6bb5767
Update assets_test.dart
jonahwilliams Apr 30, 2022
8d31015
something like this
jonahwilliams Apr 30, 2022
bcea89a
Merge branch 'immutable_buffer' of github.com:jonahwilliams/engine in…
jonahwilliams Apr 30, 2022
4fdfc74
Merge branch 'master' of github.com:flutter/engine into immutable_buffer
jonahwilliams Apr 30, 2022
83b9107
Merge branch 'master' of github.com:flutter/engine into immutable_buffer
jonahwilliams Apr 30, 2022
784254f
get release proc
jonahwilliams Apr 30, 2022
4f30721
++
jonahwilliams Apr 30, 2022
bf0c614
Update apk_asset_provider.cc
jonahwilliams May 1, 2022
a13380b
Update file_in_namespace_buffer.h
jonahwilliams May 1, 2022
77b4eac
format and dispose test
jonahwilliams May 1, 2022
940d8d1
format++
jonahwilliams May 1, 2022
56029fd
Update buffer_conversions.mm
jonahwilliams May 1, 2022
512a2b6
use delete
jonahwilliams May 2, 2022
d27b4eb
special android copy
jonahwilliams May 10, 2022
75b86f9
++
jonahwilliams May 10, 2022
7082bdb
always copy
jonahwilliams May 11, 2022
3c8a0e3
Merge branch 'master' of github.com:flutter/engine into immutable_buffer
jonahwilliams May 11, 2022
66ccfc5
Update immutable_buffer.cc
jonahwilliams May 11, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 58 additions & 2 deletions lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2053,6 +2053,48 @@ Future<Codec> instantiateImageCodec(
bool allowUpscaling = true,
}) async {
final ImmutableBuffer buffer = await ImmutableBuffer.fromUint8List(list);
return instantiateImageCodecFromBuffer(
buffer,
targetWidth: targetWidth,
targetHeight: targetHeight,
allowUpscaling: allowUpscaling,
);
}

/// Instantiates an image [Codec].
///
/// This method is a convenience wrapper around the [ImageDescriptor] API, and
/// using [ImageDescriptor] directly is preferred since it allows the caller to
/// make better determinations about how and whether to use the `targetWidth`
/// and `targetHeight` parameters.
///
/// The [buffer] parameter is the binary image data (e.g a PNG or GIF binary data).
/// The data can be for either static or animated images. The following image
/// formats are supported: {@macro dart.ui.imageFormats}
///
/// The [targetWidth] and [targetHeight] arguments specify the size of the
/// output image, in image pixels. If they are not equal to the intrinsic
/// dimensions of the image, then the image will be scaled after being decoded.
/// If the `allowUpscaling` parameter is not set to true, both dimensions will
/// be capped at the intrinsic dimensions of the image, even if only one of
/// them would have exceeded those intrinsic dimensions. If exactly one of these
/// two arguments is specified, then the aspect ratio will be maintained while
/// forcing the image to match the other given dimension. If neither is
/// specified, then the image maintains its intrinsic size.
///
/// Scaling the image to larger than its intrinsic size should usually be
/// avoided, since it causes the image to use more memory than necessary.
/// Instead, prefer scaling the [Canvas] transform. If the image must be scaled
/// up, the `allowUpscaling` parameter must be set to true.
///
/// The returned future can complete with an error if the image decoding has
/// failed.
Future<Codec> instantiateImageCodecFromBuffer(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't think of what to do besides copy the API

ImmutableBuffer buffer, {
int? targetWidth,
int? targetHeight,
bool allowUpscaling = true,
}) async {
final ImageDescriptor descriptor = await ImageDescriptor.encoded(buffer);
if (!allowUpscaling) {
if (targetWidth != null && targetWidth > descriptor.width) {
Expand Down Expand Up @@ -5511,7 +5553,7 @@ class Shadow {
/// The creator of this object is responsible for calling [dispose] when it is
/// no longer needed.
class ImmutableBuffer extends NativeFieldWrapperClass1 {
ImmutableBuffer._(this.length);
ImmutableBuffer._(this._length);

/// Creates a copy of the data from a [Uint8List] suitable for internal use
/// in the engine.
Expand All @@ -5521,10 +5563,24 @@ class ImmutableBuffer extends NativeFieldWrapperClass1 {
instance._init(list, callback);
}).then((_) => instance);
}

/// Create a buffer from the asset with key [assetKey].
///
/// Throws an [Exception] if the asset does not exist.
static Future<ImmutableBuffer> fromAsset(String assetKey) {
final ImmutableBuffer instance = ImmutableBuffer._(0);
return _futurize((_Callback<int> callback) {
return instance._initFromAsset(assetKey, callback);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_futurize handles throwing an exception if this method returns a non-null string.

}).then((int length) => instance.._length = length);
}

void _init(Uint8List list, _Callback<void> callback) native 'ImmutableBuffer_init';

String? _initFromAsset(String assetKey, _Callback<int> callback) native 'ImmutableBuffer_initFromAsset';

/// The length, in bytes, of the underlying data.
final int length;
int get length => _length;
int _length;

bool _debugDisposed = false;

Expand Down
43 changes: 43 additions & 0 deletions lib/ui/painting/immutable_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <cstring>

#include "flutter/lib/ui/ui_dart_state.h"
#include "flutter/lib/ui/window/platform_configuration.h"
#include "third_party/tonic/converter/dart_converter.h"
#include "third_party/tonic/dart_args.h"
#include "third_party/tonic/dart_binding_macros.h"
Expand All @@ -30,6 +31,9 @@ ImmutableBuffer::~ImmutableBuffer() {}
void ImmutableBuffer::RegisterNatives(tonic::DartLibraryNatives* natives) {
natives->Register({{"ImmutableBuffer_init", ImmutableBuffer::init, 3, true},
FOR_EACH_BINDING(DART_REGISTER_NATIVE)});
natives->Register({{"ImmutableBuffer_initFromAsset",
ImmutableBuffer::initFromAsset, 3, true},
FOR_EACH_BINDING(DART_REGISTER_NATIVE)});
}

void ImmutableBuffer::init(Dart_NativeArguments args) {
Expand All @@ -49,6 +53,45 @@ void ImmutableBuffer::init(Dart_NativeArguments args) {
tonic::DartInvoke(callback_handle, {Dart_TypeVoid()});
}

void ImmutableBuffer::initFromAsset(Dart_NativeArguments args) {
UIDartState::ThrowIfUIOperationsProhibited();
Dart_Handle callback_handle = Dart_GetNativeArgument(args, 2);
if (!Dart_IsClosure(callback_handle)) {
Dart_SetReturnValue(args, tonic::ToDart("Callback must be a function"));
return;
}
Dart_Handle asset_name_handle = Dart_GetNativeArgument(args, 1);
uint8_t* chars = nullptr;
intptr_t asset_length = 0;
Dart_Handle result =
Dart_StringToUTF8(asset_name_handle, &chars, &asset_length);
if (Dart_IsError(result)) {
Dart_SetReturnValue(args, tonic::ToDart("Asset must be valid UTF8"));
return;
}
Dart_Handle immutable_buffer = Dart_GetNativeArgument(args, 0);

std::string asset_name = std::string{reinterpret_cast<const char*>(chars),
static_cast<size_t>(asset_length)};

std::shared_ptr<AssetManager> asset_manager = UIDartState::Current()
->platform_configuration()
->client()
->GetAssetManager();
std::unique_ptr<fml::Mapping> data = asset_manager->GetAsMapping(asset_name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these mappings thread safe? These usually come from flutter::AssetResolvers and looking at the inheritance diagram from these, flutter::APKAssetProvider seems to be a subclass. Its assets are backed by AAsset handles whose thread safety notes say "AAsset objects are NOT thread-safe, and should not be shared across threads.". Since there used to be a copy earlier, it sort of didn't matter. But now that these are referenced across thread (for texture decompression), there may be threading violations that we have perhaps not contended with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. Sounds like we can't avoid the copy in that case.

If we didn't expose the lenght, we could get away with this being something that's done entirely on the decoder thread though right? We could just give that a way to acquire the data and then let it do the whole thing there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need the length in the framework for this case. We could always say "length might be -1 for certain instances not created from a uint8list"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could also copy once into an skdata - that would still reduce the number of copies by 1. Though having a single createCodecFromAsset method would mean we could do the entire thing on the right thread, i'm not sure how well that works with the framework wanting to abstract byte acquisition from image decoding.

I don't think adding a special case for asset images would be a bad thing fwiw, but it would be a special case.

if (data == nullptr) {
Dart_SetReturnValue(args, tonic::ToDart("Asset not found"));
return;
}

auto size = data->GetSize();
const void* bytes = static_cast<const void*>(data->GetMapping());
auto sk_data = MakeSkDataWithCopy(bytes, size);
auto buffer = fml::MakeRefCounted<ImmutableBuffer>(sk_data);
buffer->AssociateWithDartWrapper(immutable_buffer);
tonic::DartInvoke(callback_handle, {tonic::ToDart(size)});
}

size_t ImmutableBuffer::GetAllocationSize() const {
return sizeof(ImmutableBuffer) + data_->size();
}
Expand Down
13 changes: 13 additions & 0 deletions lib/ui/painting/immutable_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ class ImmutableBuffer : public RefCountedDartWrappable<ImmutableBuffer> {
/// when the copy has completed.
static void init(Dart_NativeArguments args);

/// Initializes a new ImmutableData from an asset matching a provided
/// asset string.
///
/// The zero indexed argument is the caller that will be registered as the
/// Dart peer of the native ImmutableBuffer object.
///
/// The first indexed argumented is a String corresponding to the asset
/// to load.
///
/// The second indexed argument is expected to be a void callback to signal
/// when the copy has completed.
static void initFromAsset(Dart_NativeArguments args);

/// The length of the data in bytes.
size_t length() const {
FML_DCHECK(data_);
Expand Down
6 changes: 6 additions & 0 deletions lib/ui/window/platform_configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <unordered_map>
#include <vector>

#include "flutter/assets/asset_manager.h"
#include "flutter/fml/time/time_point.h"
#include "flutter/lib/ui/semantics/semantics_update.h"
#include "flutter/lib/ui/window/pointer_data_packet.h"
Expand Down Expand Up @@ -98,6 +99,11 @@ class PlatformConfigurationClient {
/// creation.
virtual FontCollection& GetFontCollection() = 0;

//--------------------------------------------------------------------------
/// @brief Returns the current collection of assets available on the
/// platform.
virtual std::shared_ptr<AssetManager> GetAssetManager() = 0;

//--------------------------------------------------------------------------
/// @brief Notifies this client of the name of the root isolate and its
/// port when that isolate is launched, restarted (in the
Expand Down
24 changes: 22 additions & 2 deletions lib/web_ui/lib/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,20 @@ Future<Codec> instantiateImageCodec(
}
}

Future<Codec> instantiateImageCodecFromBuffer(
ImmutableBuffer buffer, {
int? targetWidth,
int? targetHeight,
bool allowUpscaling = true,
}) async {
if (engine.useCanvasKit) {
return engine.skiaInstantiateImageCodec(buffer._list!, targetWidth, targetHeight);
} else {
final html.Blob blob = html.Blob(<dynamic>[buffer._list!.buffer]);
return engine.HtmlBlobCodec(blob);
}
}

Future<Codec> webOnlyInstantiateImageCodecFromUrl(Uri uri,
{engine.WebOnlyImageCodecChunkCallback? chunkCallback}) {
if (engine.useCanvasKit) {
Expand Down Expand Up @@ -735,15 +749,21 @@ class ImageShader extends Shader {
}

class ImmutableBuffer {
ImmutableBuffer._(this.length);
ImmutableBuffer._(this._length);
static Future<ImmutableBuffer> fromUint8List(Uint8List list) async {
final ImmutableBuffer instance = ImmutableBuffer._(list.length);
instance._list = list;
return instance;
}

static Future<ImmutableBuffer> fromAsset(String assetKey) async {
throw UnsupportedError('ImmutableBuffer.fromAsset is not supported on the web.');
}

Uint8List? _list;
final int length;

int get length => _length;
int _length;

bool get debugDisposed {
late bool disposed;
Expand Down
5 changes: 5 additions & 0 deletions runtime/runtime_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,11 @@ FontCollection& RuntimeController::GetFontCollection() {
return client_.GetFontCollection();
}

// |PlatfromConfigurationClient|
std::shared_ptr<AssetManager> RuntimeController::GetAssetManager() {
return client_.GetAssetManager();
}

// |PlatformConfigurationClient|
void RuntimeController::UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) {
Expand Down
4 changes: 4 additions & 0 deletions runtime/runtime_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <vector>

#include "flutter/assets/asset_manager.h"
#include "flutter/common/task_runners.h"
#include "flutter/flow/layers/layer_tree.h"
#include "flutter/fml/macros.h"
Expand Down Expand Up @@ -614,6 +615,9 @@ class RuntimeController : public PlatformConfigurationClient {
// |PlatformConfigurationClient|
FontCollection& GetFontCollection() override;

// |PlatformConfigurationClient|
std::shared_ptr<AssetManager> GetAssetManager() override;

// |PlatformConfigurationClient|
void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) override;
Expand Down
3 changes: 3 additions & 0 deletions runtime/runtime_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <vector>

#include "flutter/assets/asset_manager.h"
#include "flutter/flow/layers/layer_tree.h"
#include "flutter/lib/ui/semantics/custom_accessibility_action.h"
#include "flutter/lib/ui/semantics/semantics_node.h"
Expand All @@ -33,6 +34,8 @@ class RuntimeDelegate {

virtual FontCollection& GetFontCollection() = 0;

virtual std::shared_ptr<AssetManager> GetAssetManager() = 0;

virtual void OnRootIsolateCreated() = 0;

virtual void UpdateIsolateDescription(const std::string isolate_name,
Expand Down
4 changes: 2 additions & 2 deletions shell/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -755,8 +755,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
// |RuntimeDelegate|
FontCollection& GetFontCollection() override;

// Return the asset manager associated with the current engine, or nullptr.
std::shared_ptr<AssetManager> GetAssetManager();
// |RuntimeDelegate|
std::shared_ptr<AssetManager> GetAssetManager() override;

// Return the weak_ptr of ImageDecoder.
fml::WeakPtr<ImageDecoder> GetImageDecoderWeakPtr();
Expand Down
1 change: 1 addition & 0 deletions shell/common/engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class MockRuntimeDelegate : public RuntimeDelegate {
void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates));
MOCK_METHOD1(HandlePlatformMessage, void(std::unique_ptr<PlatformMessage>));
MOCK_METHOD0(GetFontCollection, FontCollection&());
MOCK_METHOD0(GetAssetManager, std::shared_ptr<AssetManager>());
MOCK_METHOD0(OnRootIsolateCreated, void());
MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
MOCK_METHOD1(SetNeedsReportTimings, void(bool));
Expand Down
1 change: 1 addition & 0 deletions testing/dart/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import("//flutter/testing/dart/compile_test.gni")

tests = [
"assets_test.dart",
"canvas_test.dart",
"channel_buffers_test.dart",
"codec_test.dart",
Expand Down
32 changes: 32 additions & 0 deletions testing/dart/assets_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2013 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.

import 'dart:ui' as ui;

import 'package:litetest/litetest.dart';

void main() {
test('Loading an asset that does not exist returns null', () async {
Object? error;
try {
await ui.ImmutableBuffer.fromAsset('ThisDoesNotExist');
} catch (err) {
error = err;
}
expect(error, isNotNull);
expect(error is Exception, true);
});

test('returns the bytes of a bundled asset', () async {
final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromAsset('assets/DashInNooglerHat.jpg');

expect(buffer.length == 354679, true);
});

test('can dispose immutable buffer', () async {
final ui.ImmutableBuffer buffer = await ui.ImmutableBuffer.fromAsset('assets/DashInNooglerHat.jpg');

buffer.dispose();
});
}
1 change: 1 addition & 0 deletions testing/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ def RunDartTest(build_dir, test_packages, dart_file, verbose_dart_snapshot, mult
command_args += [
'--use-test-fonts',
'--icu-data-file-path=%s' % os.path.join(build_dir, 'icudtl.dat'),
'--flutter-assets-dir=%s' % os.path.join(build_dir, 'gen', 'flutter', 'lib', 'ui'),
kernel_file_output,
]

Expand Down