From cf5f280513574ec6effdf159c2a6c1753cb5d6ee Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 27 Apr 2021 12:50:41 -0700 Subject: [PATCH 01/10] Add image generator registry and lookup with priority --- ci/licenses_golden/licenses_flutter | 5 + lib/ui/BUILD.gn | 5 + lib/ui/painting/image_generator.cc | 69 ++++++++++++ lib/ui/painting/image_generator.h | 82 ++++++++++++++ lib/ui/painting/image_generator_registry.cc | 68 ++++++++++++ lib/ui/painting/image_generator_registry.h | 55 +++++++++ .../image_generator_registry_unittests.cc | 104 ++++++++++++++++++ lib/ui/ui_dart_state.cc | 7 ++ lib/ui/ui_dart_state.h | 5 + runtime/dart_isolate.cc | 75 +++++++------ runtime/dart_isolate.h | 9 ++ runtime/dart_isolate_unittests.cc | 5 + runtime/dart_lifecycle_unittests.cc | 1 + runtime/runtime_controller.cc | 12 +- runtime/runtime_controller.h | 11 +- shell/common/engine.cc | 35 +++--- shell/common/engine.h | 2 + testing/dart_isolate_runner.cc | 1 + .../tonic/tests/dart_state_unittest.cc | 1 + 19 files changed, 497 insertions(+), 55 deletions(-) create mode 100644 lib/ui/painting/image_generator.cc create mode 100644 lib/ui/painting/image_generator.h create mode 100644 lib/ui/painting/image_generator_registry.cc create mode 100644 lib/ui/painting/image_generator_registry.h create mode 100644 lib/ui/painting/image_generator_registry_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index ef9a420aae284..b9a68fbdae449 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -359,6 +359,11 @@ FILE: ../../../flutter/lib/ui/painting/image_encoding.h FILE: ../../../flutter/lib/ui/painting/image_encoding_unittests.cc FILE: ../../../flutter/lib/ui/painting/image_filter.cc FILE: ../../../flutter/lib/ui/painting/image_filter.h +FILE: ../../../flutter/lib/ui/painting/image_generator.cc +FILE: ../../../flutter/lib/ui/painting/image_generator.h +FILE: ../../../flutter/lib/ui/painting/image_generator_registry.cc +FILE: ../../../flutter/lib/ui/painting/image_generator_registry.h +FILE: ../../../flutter/lib/ui/painting/image_generator_registry_unittests.cc FILE: ../../../flutter/lib/ui/painting/image_shader.cc FILE: ../../../flutter/lib/ui/painting/image_shader.h FILE: ../../../flutter/lib/ui/painting/immutable_buffer.cc diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 1d2ab90df406f..2dda9d54b9e37 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -42,6 +42,10 @@ source_set("ui") { "painting/image_encoding.h", "painting/image_filter.cc", "painting/image_filter.h", + "painting/image_generator.cc", + "painting/image_generator.h", + "painting/image_generator_registry.cc", + "painting/image_generator_registry.h", "painting/image_shader.cc", "painting/image_shader.h", "painting/immutable_buffer.cc", @@ -197,6 +201,7 @@ if (enable_unittests) { sources = [ "painting/image_dispose_unittests.cc", "painting/image_encoding_unittests.cc", + "painting/image_generator_registry_unittests.cc", "painting/path_unittests.cc", "painting/vertices_unittests.cc", "window/platform_configuration_unittests.cc", diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc new file mode 100644 index 0000000000000..95833dbe50d1b --- /dev/null +++ b/lib/ui/painting/image_generator.cc @@ -0,0 +1,69 @@ +// 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. + +#include "flutter/lib/ui/painting/image_generator.h" + +namespace flutter { + +BuiltinSkiaImageGenerator::BuiltinSkiaImageGenerator( + std::unique_ptr generator) + : generator_(std::move(generator)) {} + +const SkImageInfo& BuiltinSkiaImageGenerator::getInfo() const { + return generator_->getInfo(); +} + +bool BuiltinSkiaImageGenerator::getPixels(const SkImageInfo& info, + void* pixels, + size_t rowBytes) const { + return generator_->getPixels(info, pixels, rowBytes); +} + +SkISize BuiltinSkiaImageGenerator::getScaledDimensions(float desiredScale) { + return generator_->getInfo().dimensions(); +} + +std::unique_ptr BuiltinSkiaImageGenerator::makeFromGenerator( + std::unique_ptr generator) { + if (!generator) { + return nullptr; + } + return std::make_unique(std::move(generator)); +} + +BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( + std::unique_ptr codec) + : codec_generator_(static_cast( + SkCodecImageGenerator::MakeFromCodec(std::move(codec)).release())) {} + +BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( + sk_sp buffer) + : codec_generator_(static_cast( + SkCodecImageGenerator::MakeFromEncodedCodec(buffer).release())) {} + +const SkImageInfo& BuiltinSkiaCodecImageGenerator::getInfo() const { + return codec_generator_->getInfo(); +} + +bool BuiltinSkiaCodecImageGenerator::getPixels(const SkImageInfo& info, + void* pixels, + size_t rowBytes) const { + return codec_generator_->getPixels(info, pixels, rowBytes); +} + +SkISize BuiltinSkiaCodecImageGenerator::getScaledDimensions( + float desiredScale) { + return codec_generator_->getScaledDimensions(desiredScale); +} + +std::unique_ptr BuiltinSkiaCodecImageGenerator::makeFromData( + sk_sp data) { + auto codec = SkCodec::MakeFromData(data); + if (!codec) { + return nullptr; + } + return std::make_unique(std::move(codec)); +} + +} // namespace flutter diff --git a/lib/ui/painting/image_generator.h b/lib/ui/painting/image_generator.h new file mode 100644 index 0000000000000..5e1f1138e5682 --- /dev/null +++ b/lib/ui/painting/image_generator.h @@ -0,0 +1,82 @@ +// 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. + +#ifndef FLUTTER_LIB_UI_PAINTING_IMAGE_GENERATOR_H_ +#define FLUTTER_LIB_UI_PAINTING_IMAGE_GENERATOR_H_ + +#include "flutter/lib/ui/painting/codec.h" +#include "third_party/skia/include/core/SkImageInfo.h" + +namespace flutter { + +/// The minimal interface necessary for creating decoders that can be used for +/// single-frame image decoding with optional support for decoding into a +/// subscaled buffer. +/// +/// See `getScaledDimensions` for details about how subpixel image decoding can +/// be supported. +class ImageGenerator { + public: + virtual ~ImageGenerator(){}; + + virtual const SkImageInfo& getInfo() const = 0; + + virtual bool getPixels(const SkImageInfo& info, + void* pixels, + size_t rowBytes) const = 0; + + /// Given a scale value, returns the closest image size that can be used for + /// efficiently decoding the image. If subpixel image decoding is not + /// supported by the codec, this method just returns the original image size. + virtual SkISize getScaledDimensions(float scale) = 0; +}; + +class BuiltinSkiaImageGenerator : public ImageGenerator { + public: + BuiltinSkiaImageGenerator(std::unique_ptr generator); + + ~BuiltinSkiaImageGenerator() = default; + + const SkImageInfo& getInfo() const override; + + bool getPixels(const SkImageInfo& info, + void* pixels, + size_t rowBytes) const override; + + SkISize getScaledDimensions(float desiredScale) override; + + static std::unique_ptr makeFromGenerator( + std::unique_ptr generator); + + private: + BuiltinSkiaImageGenerator() = delete; + std::unique_ptr generator_; +}; + +class BuiltinSkiaCodecImageGenerator : public ImageGenerator { + public: + BuiltinSkiaCodecImageGenerator(std::unique_ptr codec); + + BuiltinSkiaCodecImageGenerator(sk_sp buffer); + + ~BuiltinSkiaCodecImageGenerator() = default; + + const SkImageInfo& getInfo() const override; + + bool getPixels(const SkImageInfo& info, + void* pixels, + size_t rowBytes) const override; + + SkISize getScaledDimensions(float desiredScale) override; + + static std::unique_ptr makeFromData(sk_sp data); + + private: + BuiltinSkiaCodecImageGenerator() = delete; + std::unique_ptr codec_generator_; +}; + +} // namespace flutter + +#endif // FLUTTER_LIB_UI_PAINTING_IMAGE_GENERATOR_H_ diff --git a/lib/ui/painting/image_generator_registry.cc b/lib/ui/painting/image_generator_registry.cc new file mode 100644 index 0000000000000..741470d983b07 --- /dev/null +++ b/lib/ui/painting/image_generator_registry.cc @@ -0,0 +1,68 @@ +// 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. + +#include + +#include "flutter/lib/ui/painting/image_generator_registry.h" +#include "third_party/skia/include/codec/SkCodec.h" +#include "third_party/skia/include/core/SkImageGenerator.h" +#include "third_party/skia/src/codec/SkCodecImageGenerator.h" +#ifdef OS_MACOSX +#include "third_party/skia/include/ports/SkImageGeneratorCG.h" +#elif OS_WIN +#include "third_party/skia/include/ports/SkImageGeneratorWIC.h" +#endif + +namespace flutter { + +ImageGeneratorRegistry::ImageGeneratorRegistry() : weak_factory_(this) { + add( + [](sk_sp buffer) { + return BuiltinSkiaCodecImageGenerator::makeFromData(buffer); + }, + 0); + +#ifdef OS_MACOSX + add( + [](sk_sp buffer) { + auto generator = SkImageGeneratorCG::MakeFromEncodedCG(buffer); + return BuiltinSkiaImageGenerator::makeFromGenerator( + std::move(generator)); + }, + 0); +#elif OS_WIN + add( + [](sk_sp buffer) { + auto generator = SkImageGeneratorWIC::MakeFromEncodedWIC(buffer); + return BuiltinSkiaImageGenerator::makeFromGenerator( + std::move(generator)); + }, + 0); +#endif +} + +void ImageGeneratorRegistry::add(ImageGeneratorFactory factory, + int32_t priority) { + image_generator_factories_.push_back({factory, priority}); + std::stable_sort(image_generator_factories_.begin(), + image_generator_factories_.end()); +} + +std::unique_ptr ImageGeneratorRegistry::createCompatible( + sk_sp buffer) { + for (auto& factory : image_generator_factories_) { + std::unique_ptr result = factory.callback(buffer); + if (result) { + return result; + } + } + return nullptr; +} + +fml::WeakPtr ImageGeneratorRegistry::GetWeakPtr() + const { + return weak_factory_.GetWeakPtr(); +} + +} // namespace flutter diff --git a/lib/ui/painting/image_generator_registry.h b/lib/ui/painting/image_generator_registry.h new file mode 100644 index 0000000000000..9b9e0d7382258 --- /dev/null +++ b/lib/ui/painting/image_generator_registry.h @@ -0,0 +1,55 @@ +// 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. + +#ifndef FLUTTER_LIB_UI_PAINTING_IMAGE_GENERATOR_REGISTRY_H_ +#define FLUTTER_LIB_UI_PAINTING_IMAGE_GENERATOR_REGISTRY_H_ + +#include +#include + +#include "flutter/fml/mapping.h" +#include "flutter/fml/memory/weak_ptr.h" +#include "flutter/lib/ui/painting/image_generator.h" + +namespace flutter { + +/// Keeps a priority-ordered registry of image generator builders to be used +/// when decoding images. This object must be created, accessed, and collected +/// on the UI thread (typically the engine or its runtime controller). +class ImageGeneratorRegistry { + public: + using ImageGeneratorFactory = + std::function(sk_sp buffer)>; + + ImageGeneratorRegistry(); + + ~ImageGeneratorRegistry() = default; + + void add(ImageGeneratorFactory factory, int32_t priority); + + /// Walks the list of image generator builders in descending priority order + /// until a compatible SkImageGenerator is able to be built. If no compatible + /// image generator could be produced, `std::unique_ptr(nullptr)` is returned. + std::unique_ptr createCompatible(sk_sp buffer); + + fml::WeakPtr GetWeakPtr() const; + + private: + struct PrioritizedFactory_ { + ImageGeneratorFactory callback; + + int32_t priority; + + // Order by descending priority. + inline bool operator<(const PrioritizedFactory_& other) const { + return priority > other.priority; + } + }; + std::vector image_generator_factories_; + fml::WeakPtrFactory weak_factory_; +}; + +} // namespace flutter + +#endif // FLUTTER_LIB_UI_PAINTING_IMAGE_DECODER_H_ diff --git a/lib/ui/painting/image_generator_registry_unittests.cc b/lib/ui/painting/image_generator_registry_unittests.cc new file mode 100644 index 0000000000000..fee78559eae6e --- /dev/null +++ b/lib/ui/painting/image_generator_registry_unittests.cc @@ -0,0 +1,104 @@ +// 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. + +#include "flutter/lib/ui/painting/image_generator_registry.h" + +#include "flutter/fml/mapping.h" +#include "flutter/shell/common/shell_test.h" +#include "flutter/testing/testing.h" + +namespace flutter { +namespace testing { + +static sk_sp LoadValidImageFixture() { + // Load the fixture with fml mappings. + auto fixtures_directory = + fml::OpenDirectory(GetFixturesPath(), false, fml::FilePermission::kRead); + auto fixture_mapping = fml::FileMapping::CreateReadOnly( + fixtures_directory, "DashInNooglerHat.jpg"); + + // Remap to sk_sp. + SkData::ReleaseProc on_release = [](const void* ptr, void* context) -> void { + delete reinterpret_cast(context); + }; + auto data = SkData::MakeWithProc(fixture_mapping->GetMapping(), + fixture_mapping->GetSize(), on_release, + fixture_mapping.get()); + fixture_mapping.release(); + + return data; +} + +TEST_F(ShellTest, CreateCompatibleReturnsBuiltinImageGeneratorForValidImage) { + auto data = LoadValidImageFixture(); + + // Fetch the generator and query for basic info + ImageGeneratorRegistry registry; + auto result = registry.createCompatible(data); + auto info = result->getInfo(); + ASSERT_EQ(info.width(), 3024); + ASSERT_EQ(info.height(), 4032); +} + +TEST_F(ShellTest, CreateCompatibleReturnsNullptrForInvalidImage) { + ImageGeneratorRegistry registry; + auto result = registry.createCompatible(SkData::MakeEmpty()); + ASSERT_EQ(result, nullptr); +} + +class FakeImageGenerator : public ImageGenerator { + public: + FakeImageGenerator(int identifiableFakeWidth) + : info_(SkImageInfo::Make(identifiableFakeWidth, + identifiableFakeWidth, + SkColorType::kRGBA_8888_SkColorType, + SkAlphaType::kOpaque_SkAlphaType)){}; + ~FakeImageGenerator() = default; + const SkImageInfo& getInfo() const { return info_; } + + bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const { + return false; + }; + + SkISize getScaledDimensions(float scale) { + return SkISize::Make(info_.width(), info_.height()); + } + + private: + SkImageInfo info_; +}; + +TEST_F(ShellTest, PositivePriorityTakesPrecedentOverDefaultGenerators) { + ImageGeneratorRegistry registry; + + const int fakeWidth = 1337; + registry.add( + [fakeWidth](sk_sp buffer) { + return std::make_unique(fakeWidth); + }, + 1); + + // Fetch the generator and query for basic info. + auto result = registry.createCompatible(LoadValidImageFixture()); + ASSERT_EQ(result->getInfo().width(), fakeWidth); +} + +TEST_F(ShellTest, DefaultGeneratorsTakePrecedentOverNegativePriority) { + ImageGeneratorRegistry registry; + + registry.add( + [](sk_sp buffer) { + return std::make_unique(1337); + }, + -1); + + // Fetch the generator and query for basic info. + auto result = registry.createCompatible(LoadValidImageFixture()); + // If the real width of the image pops out, then the default generator was + // returned rather than the fake one. + ASSERT_EQ(result->getInfo().width(), 3024); +} + +} // namespace testing +} // namespace flutter diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index 2612cbe1fd88e..9382215a2d8a2 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -33,6 +33,7 @@ UIDartState::UIDartState( fml::WeakPtr io_manager, fml::RefPtr skia_unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, std::string logger_prefix, @@ -50,6 +51,7 @@ UIDartState::UIDartState( io_manager_(std::move(io_manager)), skia_unref_queue_(std::move(skia_unref_queue)), image_decoder_(std::move(image_decoder)), + image_generator_registry_(std::move(image_generator_registry)), volatile_path_tracker_(std::move(volatile_path_tracker)), advisory_script_uri_(std::move(advisory_script_uri)), advisory_script_entrypoint_(std::move(advisory_script_entrypoint)), @@ -175,6 +177,11 @@ fml::WeakPtr UIDartState::GetImageDecoder() const { return image_decoder_; } +fml::WeakPtr UIDartState::GetImageGeneratorRegistry() + const { + return image_generator_registry_; +} + std::shared_ptr UIDartState::GetIsolateNameServer() const { return isolate_name_server_; } diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index 636f32a12a973..b08e9893071d3 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -29,6 +29,7 @@ namespace flutter { class FontSelector; +class ImageGeneratorRegistry; class PlatformConfiguration; class UIDartState : public tonic::DartState { @@ -70,6 +71,8 @@ class UIDartState : public tonic::DartState { fml::WeakPtr GetImageDecoder() const; + fml::WeakPtr GetImageGeneratorRegistry() const; + std::shared_ptr GetIsolateNameServer() const; tonic::DartErrorHandleType GetLastError(); @@ -107,6 +110,7 @@ class UIDartState : public tonic::DartState { fml::WeakPtr io_manager, fml::RefPtr skia_unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, std::string logger_prefix, @@ -137,6 +141,7 @@ class UIDartState : public tonic::DartState { fml::WeakPtr io_manager_; fml::RefPtr skia_unref_queue_; fml::WeakPtr image_decoder_; + fml::WeakPtr image_generator_registry_; std::shared_ptr volatile_path_tracker_; const std::string advisory_script_uri_; const std::string advisory_script_entrypoint_; diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index ddec2276191e7..ec2fa3b89e57e 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -95,10 +95,10 @@ std::weak_ptr DartIsolate::SpawnIsolate( settings, GetIsolateGroupData().GetIsolateSnapshot(), GetTaskRunners(), std::move(platform_configuration), snapshot_delegate, hint_freed_delegate, GetIOManager(), GetSkiaUnrefQueue(), GetImageDecoder(), - advisory_script_uri, advisory_script_entrypoint, flags, - isolate_create_callback, isolate_shutdown_callback, dart_entrypoint, - dart_entrypoint_library, std::move(isolate_configration), - GetVolatilePathTracker(), this); + GetImageGeneratorRegistry(), advisory_script_uri, + advisory_script_entrypoint, flags, isolate_create_callback, + isolate_shutdown_callback, dart_entrypoint, dart_entrypoint_library, + std::move(isolate_configration), GetVolatilePathTracker(), this); } std::weak_ptr DartIsolate::CreateRunningRootIsolate( @@ -111,6 +111,7 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( fml::WeakPtr io_manager, fml::RefPtr skia_unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags isolate_flags, @@ -143,6 +144,7 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( io_manager, // skia_unref_queue, // image_decoder, // + image_generator_registry, // advisory_script_uri, // advisory_script_entrypoint, // isolate_flags, // @@ -222,6 +224,7 @@ std::weak_ptr DartIsolate::CreateRootIsolate( fml::WeakPtr io_manager, fml::RefPtr unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, @@ -247,17 +250,18 @@ std::weak_ptr DartIsolate::CreateRootIsolate( auto isolate_data = std::make_unique>( std::shared_ptr(new DartIsolate( - settings, // settings - task_runners, // task runners - std::move(snapshot_delegate), // snapshot delegate - std::move(hint_freed_delegate), // hint freed delegate - std::move(io_manager), // IO manager - std::move(unref_queue), // Skia unref queue - std::move(image_decoder), // Image Decoder - advisory_script_uri, // advisory URI - advisory_script_entrypoint, // advisory entrypoint - true, // is_root_isolate - std::move(volatile_path_tracker) // volatile path tracker + settings, // settings + task_runners, // task runners + std::move(snapshot_delegate), // snapshot delegate + std::move(hint_freed_delegate), // hint freed delegate + std::move(io_manager), // IO manager + std::move(unref_queue), // Skia unref queue + std::move(image_decoder), // Image Decoder + std::move(image_generator_registry), // Image generator registry + advisory_script_uri, // advisory URI + advisory_script_entrypoint, // advisory entrypoint + true, // is_root_isolate + std::move(volatile_path_tracker) // volatile path tracker ))); DartErrorString error; @@ -326,6 +330,7 @@ DartIsolate::DartIsolate( fml::WeakPtr io_manager, fml::RefPtr unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, bool is_root_isolate, @@ -338,6 +343,7 @@ DartIsolate::DartIsolate( std::move(io_manager), std::move(unref_queue), std::move(image_decoder), + std::move(image_generator_registry), advisory_script_uri, advisory_script_entrypoint, settings.log_tag, @@ -870,6 +876,7 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( {}, // IO Manager {}, // Skia unref queue {}, // Image Decoder + {}, // Image generator registry DART_VM_SERVICE_ISOLATE_NAME, // script uri DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint DartIsolate::Flags{flags}, // flags @@ -985,17 +992,18 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( auto isolate_data = std::make_unique>( std::shared_ptr(new DartIsolate( - (*isolate_group_data)->GetSettings(), // settings - null_task_runners, // task_runners - fml::WeakPtr{}, // snapshot_delegate - fml::WeakPtr{}, // hint_freed_delegate - fml::WeakPtr{}, // io_manager - fml::RefPtr{}, // unref_queue - fml::WeakPtr{}, // image_decoder - advisory_script_uri, // advisory_script_uri - advisory_script_entrypoint, // advisory_script_entrypoint - false, // is_root_isolate - nullptr))); // volatile path tracker + (*isolate_group_data)->GetSettings(), // settings + null_task_runners, // task_runners + fml::WeakPtr{}, // snapshot_delegate + fml::WeakPtr{}, // hint_freed_delegate + fml::WeakPtr{}, // io_manager + fml::RefPtr{}, // unref_queue + fml::WeakPtr{}, // image_decoder + fml::WeakPtr{}, // image_generator_registry + advisory_script_uri, // advisory_script_uri + advisory_script_entrypoint, // advisory_script_entrypoint + false, // is_root_isolate + nullptr))); // volatile path tracker Dart_Isolate vm_isolate = CreateDartIsolateGroup( std::move(isolate_group_data), std::move(isolate_data), flags, error, @@ -1042,13 +1050,14 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data, auto embedder_isolate = std::make_unique>( std::shared_ptr(new DartIsolate( - (*isolate_group_data)->GetSettings(), // settings - null_task_runners, // task_runners - fml::WeakPtr{}, // snapshot_delegate - fml::WeakPtr{}, // hint_freed_delegate - fml::WeakPtr{}, // io_manager - fml::RefPtr{}, // unref_queue - fml::WeakPtr{}, // image_decoder + (*isolate_group_data)->GetSettings(), // settings + null_task_runners, // task_runners + fml::WeakPtr{}, // snapshot_delegate + fml::WeakPtr{}, // hint_freed_delegate + fml::WeakPtr{}, // io_manager + fml::RefPtr{}, // unref_queue + fml::WeakPtr{}, // image_decoder + fml::WeakPtr{}, // image_generator_registry (*isolate_group_data)->GetAdvisoryScriptURI(), // advisory_script_uri (*isolate_group_data) ->GetAdvisoryScriptEntrypoint(), // advisory_script_entrypoint diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index d7d00bd707d42..1c83f58123621 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -176,6 +176,12 @@ class DartIsolate : public UIDartState { /// @param[in] io_manager The i/o manager. /// @param[in] unref_queue The Skia unref queue. /// @param[in] image_decoder The image decoder. + /// @param[in] image_generator_registry Cascading registry of image + /// generator builders. Given + /// compressed image bytes as input, + /// this is used to find and create + /// image generators, which can then + /// be used for image decoding. /// @param[in] advisory_script_uri The advisory script uri. This is /// only used in instrumentation. /// @param[in] advisory_script_entrypoint The advisory script entrypoint. @@ -225,6 +231,7 @@ class DartIsolate : public UIDartState { fml::WeakPtr io_manager, fml::RefPtr skia_unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, @@ -460,6 +467,7 @@ class DartIsolate : public UIDartState { fml::WeakPtr io_manager, fml::RefPtr skia_unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, @@ -475,6 +483,7 @@ class DartIsolate : public UIDartState { fml::WeakPtr io_manager, fml::RefPtr unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, bool is_root_isolate, diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 11e70c2b4f153..ed57014853d85 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -61,6 +61,7 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) { {}, // io manager {}, // unref queue {}, // image decoder + {}, // image generator registry "main.dart", // advisory uri "main", // advisory entrypoint, DartIsolate::Flags{}, // flags @@ -104,6 +105,7 @@ TEST_F(DartIsolateTest, SpawnIsolate) { {}, // io manager {}, // unref queue {}, // image decoder + {}, // image generator registry "main.dart", // advisory uri "main", // advisory entrypoint, DartIsolate::Flags{}, // flags @@ -182,6 +184,7 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) { {}, // io manager {}, // unref queue {}, // image decoder + {}, // image generator registry "main.dart", // advisory uri "main", // advisory entrypoint DartIsolate::Flags{}, // flags @@ -441,6 +444,7 @@ TEST_F(DartIsolateTest, CanCreateServiceIsolate) { {}, // io manager {}, // unref queue {}, // image decoder + {}, // image generator registry "main.dart", // advisory uri "main", // advisory entrypoint, DartIsolate::Flags{}, // flags @@ -542,6 +546,7 @@ TEST_F(DartIsolateTest, InvalidLoadingUnitFails) { {}, // io manager {}, // unref queue {}, // image decoder + {}, // image generator registry "main.dart", // advisory uri "main", // advisory entrypoint DartIsolate::Flags{}, // flags diff --git a/runtime/dart_lifecycle_unittests.cc b/runtime/dart_lifecycle_unittests.cc index 09de19f6e1687..02a18798d356f 100644 --- a/runtime/dart_lifecycle_unittests.cc +++ b/runtime/dart_lifecycle_unittests.cc @@ -66,6 +66,7 @@ static std::shared_ptr CreateAndRunRootIsolate( {}, // io_manager {}, // unref_queue {}, // image_decoder + {}, // image_generator_registry "main.dart", // advisory_script_uri entrypoint.c_str(), // advisory_script_entrypoint DartIsolate::Flags{}, // flags diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 7da201b7a056f..7da1ecaca47a9 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -31,6 +31,7 @@ RuntimeController::RuntimeController( fml::WeakPtr p_io_manager, fml::RefPtr p_unref_queue, fml::WeakPtr p_image_decoder, + fml::WeakPtr p_image_generator_registry, std::string p_advisory_script_uri, std::string p_advisory_script_entrypoint, const std::function& idle_notification_callback, @@ -48,6 +49,7 @@ RuntimeController::RuntimeController( io_manager_(p_io_manager), unref_queue_(p_unref_queue), image_decoder_(p_image_decoder), + image_generator_registry_(p_image_generator_registry), advisory_script_uri_(p_advisory_script_uri), advisory_script_entrypoint_(p_advisory_script_entrypoint), idle_notification_callback_(idle_notification_callback), @@ -68,10 +70,10 @@ std::unique_ptr RuntimeController::Spawn( auto result = std::make_unique( client, vm_, isolate_snapshot_, task_runners_, snapshot_delegate_, hint_freed_delegate_, io_manager_, unref_queue_, image_decoder_, - advisory_script_uri, advisory_script_entrypoint, - idle_notification_callback, platform_data_, isolate_create_callback, - isolate_shutdown_callback, persistent_isolate_data, - volatile_path_tracker_); + image_generator_registry_, advisory_script_uri, + advisory_script_entrypoint, idle_notification_callback, platform_data_, + isolate_create_callback, isolate_shutdown_callback, + persistent_isolate_data, volatile_path_tracker_); result->spawning_isolate_ = root_isolate_; return result; } @@ -108,6 +110,7 @@ std::unique_ptr RuntimeController::Clone() const { io_manager_, // unref_queue_, // image_decoder_, // + image_generator_registry_, // advisory_script_uri_, // advisory_script_entrypoint_, // idle_notification_callback_, // @@ -398,6 +401,7 @@ bool RuntimeController::LaunchRootIsolate( io_manager_, // unref_queue_, // image_decoder_, // + image_generator_registry_, // advisory_script_uri_, // advisory_script_entrypoint_, // DartIsolate::Flags{}, // diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 38976557496e9..9276d522dfe37 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -13,6 +13,7 @@ #include "flutter/fml/macros.h" #include "flutter/fml/mapping.h" #include "flutter/lib/ui/io_manager.h" +#include "flutter/lib/ui/painting/image_generator_registry.h" #include "flutter/lib/ui/text/font_collection.h" #include "flutter/lib/ui/ui_dart_state.h" #include "flutter/lib/ui/volatile_path_tracker.h" @@ -80,7 +81,13 @@ class RuntimeController : public PlatformConfigurationClient { /// isolate to collect resources that /// may reference resources on the /// GPU. - /// @param[in] image_decoder The image decoder + /// @param[in] image_decoder The image decoder. + /// @param[in] image_generator_registry Cascading registry of image + /// generator builders. Given + /// compressed image bytes as input, + /// this is used to find and create + /// image generators, which can then + /// be used for image decoding. /// @param[in] advisory_script_uri The advisory script URI (only used /// for debugging). This does not /// affect the code being run in the @@ -124,6 +131,7 @@ class RuntimeController : public PlatformConfigurationClient { fml::WeakPtr io_manager, fml::RefPtr unref_queue, fml::WeakPtr image_decoder, + fml::WeakPtr image_generator_registry, std::string advisory_script_uri, std::string advisory_script_entrypoint, const std::function& idle_notification_callback, @@ -627,6 +635,7 @@ class RuntimeController : public PlatformConfigurationClient { fml::WeakPtr io_manager_; fml::RefPtr unref_queue_; fml::WeakPtr image_decoder_; + fml::WeakPtr image_generator_registry_; std::string advisory_script_uri_; std::string advisory_script_entrypoint_; std::function idle_notification_callback_; diff --git a/shell/common/engine.cc b/shell/common/engine.cc index db72230590b2a..678ff65eb0938 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -87,23 +87,24 @@ Engine::Engine(Delegate& delegate, std::make_shared(), nullptr) { runtime_controller_ = std::make_unique( - *this, // runtime delegate - &vm, // VM - std::move(isolate_snapshot), // isolate snapshot - task_runners_, // task runners - std::move(snapshot_delegate), // snapshot delegate - GetWeakPtr(), // hint freed delegate - std::move(io_manager), // io manager - std::move(unref_queue), // Skia unref queue - image_decoder_.GetWeakPtr(), // image decoder - settings_.advisory_script_uri, // advisory script uri - settings_.advisory_script_entrypoint, // advisory script entrypoint - settings_.idle_notification_callback, // idle notification callback - platform_data, // platform data - settings_.isolate_create_callback, // isolate create callback - settings_.isolate_shutdown_callback, // isolate shutdown callback - settings_.persistent_isolate_data, // persistent isolate data - std::move(volatile_path_tracker) // volatile path tracker + *this, // runtime delegate + &vm, // VM + std::move(isolate_snapshot), // isolate snapshot + task_runners_, // task runners + std::move(snapshot_delegate), // snapshot delegate + GetWeakPtr(), // hint freed delegate + std::move(io_manager), // io manager + std::move(unref_queue), // Skia unref queue + image_decoder_.GetWeakPtr(), // image decoder + image_generator_registry_.GetWeakPtr(), // image generator registry + settings_.advisory_script_uri, // advisory script uri + settings_.advisory_script_entrypoint, // advisory script entrypoint + settings_.idle_notification_callback, // idle notification callback + platform_data, // platform data + settings_.isolate_create_callback, // isolate create callback + settings_.isolate_shutdown_callback, // isolate shutdown callback + settings_.persistent_isolate_data, // persistent isolate data + std::move(volatile_path_tracker) // volatile path tracker ); } diff --git a/shell/common/engine.h b/shell/common/engine.h index 1b42fe244d180..674ce8d519c5d 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -15,6 +15,7 @@ #include "flutter/fml/memory/weak_ptr.h" #include "flutter/lib/ui/hint_freed_delegate.h" #include "flutter/lib/ui/painting/image_decoder.h" +#include "flutter/lib/ui/painting/image_generator_registry.h" #include "flutter/lib/ui/semantics/custom_accessibility_action.h" #include "flutter/lib/ui/semantics/semantics_node.h" #include "flutter/lib/ui/snapshot_delegate.h" @@ -921,6 +922,7 @@ class Engine final : public RuntimeDelegate, bool have_surface_; std::shared_ptr font_collection_; ImageDecoder image_decoder_; + ImageGeneratorRegistry image_generator_registry_; TaskRunners task_runners_; size_t hint_freed_bytes_since_last_call_ = 0; fml::TimePoint last_hint_freed_call_time_; diff --git a/testing/dart_isolate_runner.cc b/testing/dart_isolate_runner.cc index a4ec2704430b1..e8f3d0875afdd 100644 --- a/testing/dart_isolate_runner.cc +++ b/testing/dart_isolate_runner.cc @@ -130,6 +130,7 @@ std::unique_ptr RunDartCodeInIsolateOnUITaskRunner( io_manager, // io manager {}, // unref queue {}, // image decoder + {}, // image generator registry "main.dart", // advisory uri entrypoint.c_str(), // advisory entrypoint DartIsolate::Flags{}, // flags diff --git a/third_party/tonic/tests/dart_state_unittest.cc b/third_party/tonic/tests/dart_state_unittest.cc index 8a91cc20b41ae..f5725cbfc8d57 100644 --- a/third_party/tonic/tests/dart_state_unittest.cc +++ b/third_party/tonic/tests/dart_state_unittest.cc @@ -37,6 +37,7 @@ TEST_F(DartState, IsShuttingDown) { {}, // io manager {}, // unref queue {}, // image decoder + {}, // image generator registry "main.dart", // advisory uri "main", // advisory entrypoint DartIsolate::Flags{}, // flags From f1574efbadf17e6c9ac8984902183ae625869b9b Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 11 May 2021 15:23:26 -0700 Subject: [PATCH 02/10] Minor review fixes --- lib/ui/painting/image_generator.cc | 22 ++++++---- lib/ui/painting/image_generator.h | 43 +++++++++++-------- lib/ui/painting/image_generator_registry.cc | 18 ++++---- lib/ui/painting/image_generator_registry.h | 15 ++++--- .../image_generator_registry_unittests.cc | 35 ++++++++------- 5 files changed, 74 insertions(+), 59 deletions(-) diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index 95833dbe50d1b..79c0a0dc374ab 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -6,25 +6,31 @@ namespace flutter { +ImageGenerator::~ImageGenerator() = default; + +BuiltinSkiaImageGenerator::~BuiltinSkiaImageGenerator() = default; + BuiltinSkiaImageGenerator::BuiltinSkiaImageGenerator( std::unique_ptr generator) : generator_(std::move(generator)) {} -const SkImageInfo& BuiltinSkiaImageGenerator::getInfo() const { +const SkImageInfo& BuiltinSkiaImageGenerator::GetInfo() const { return generator_->getInfo(); } -bool BuiltinSkiaImageGenerator::getPixels(const SkImageInfo& info, +bool BuiltinSkiaImageGenerator::GetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const { return generator_->getPixels(info, pixels, rowBytes); } -SkISize BuiltinSkiaImageGenerator::getScaledDimensions(float desiredScale) { +SkISize BuiltinSkiaImageGenerator::GetScaledDimensions(float desiredScale) { return generator_->getInfo().dimensions(); } -std::unique_ptr BuiltinSkiaImageGenerator::makeFromGenerator( +BuiltinSkiaCodecImageGenerator::~BuiltinSkiaCodecImageGenerator() = default; + +std::unique_ptr BuiltinSkiaImageGenerator::MakeFromGenerator( std::unique_ptr generator) { if (!generator) { return nullptr; @@ -42,22 +48,22 @@ BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( : codec_generator_(static_cast( SkCodecImageGenerator::MakeFromEncodedCodec(buffer).release())) {} -const SkImageInfo& BuiltinSkiaCodecImageGenerator::getInfo() const { +const SkImageInfo& BuiltinSkiaCodecImageGenerator::GetInfo() const { return codec_generator_->getInfo(); } -bool BuiltinSkiaCodecImageGenerator::getPixels(const SkImageInfo& info, +bool BuiltinSkiaCodecImageGenerator::GetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const { return codec_generator_->getPixels(info, pixels, rowBytes); } -SkISize BuiltinSkiaCodecImageGenerator::getScaledDimensions( +SkISize BuiltinSkiaCodecImageGenerator::GetScaledDimensions( float desiredScale) { return codec_generator_->getScaledDimensions(desiredScale); } -std::unique_ptr BuiltinSkiaCodecImageGenerator::makeFromData( +std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( sk_sp data) { auto codec = SkCodec::MakeFromData(data); if (!codec) { diff --git a/lib/ui/painting/image_generator.h b/lib/ui/painting/image_generator.h index 5e1f1138e5682..af4f69454e68f 100644 --- a/lib/ui/painting/image_generator.h +++ b/lib/ui/painting/image_generator.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_LIB_UI_PAINTING_IMAGE_GENERATOR_H_ #define FLUTTER_LIB_UI_PAINTING_IMAGE_GENERATOR_H_ +#include "flutter/fml/macros.h" #include "flutter/lib/ui/painting/codec.h" #include "third_party/skia/include/core/SkImageInfo.h" @@ -18,62 +19,68 @@ namespace flutter { /// be supported. class ImageGenerator { public: - virtual ~ImageGenerator(){}; + virtual ~ImageGenerator(); - virtual const SkImageInfo& getInfo() const = 0; + virtual const SkImageInfo& GetInfo() const = 0; - virtual bool getPixels(const SkImageInfo& info, + virtual bool GetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const = 0; /// Given a scale value, returns the closest image size that can be used for /// efficiently decoding the image. If subpixel image decoding is not /// supported by the codec, this method just returns the original image size. - virtual SkISize getScaledDimensions(float scale) = 0; + virtual SkISize GetScaledDimensions(float scale) = 0; }; class BuiltinSkiaImageGenerator : public ImageGenerator { public: - BuiltinSkiaImageGenerator(std::unique_ptr generator); + ~BuiltinSkiaImageGenerator(); - ~BuiltinSkiaImageGenerator() = default; + BuiltinSkiaImageGenerator(std::unique_ptr generator); - const SkImageInfo& getInfo() const override; + // |ImageGenerator| + const SkImageInfo& GetInfo() const override; - bool getPixels(const SkImageInfo& info, + // |ImageGenerator| + bool GetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const override; - SkISize getScaledDimensions(float desiredScale) override; + // |ImageGenerator| + SkISize GetScaledDimensions(float desiredScale) override; - static std::unique_ptr makeFromGenerator( + static std::unique_ptr MakeFromGenerator( std::unique_ptr generator); private: - BuiltinSkiaImageGenerator() = delete; + FML_DISALLOW_COPY_ASSIGN_AND_MOVE(BuiltinSkiaImageGenerator); std::unique_ptr generator_; }; class BuiltinSkiaCodecImageGenerator : public ImageGenerator { public: + ~BuiltinSkiaCodecImageGenerator(); + BuiltinSkiaCodecImageGenerator(std::unique_ptr codec); BuiltinSkiaCodecImageGenerator(sk_sp buffer); - ~BuiltinSkiaCodecImageGenerator() = default; - - const SkImageInfo& getInfo() const override; + // |ImageGenerator| + const SkImageInfo& GetInfo() const override; - bool getPixels(const SkImageInfo& info, + // |ImageGenerator| + bool GetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const override; - SkISize getScaledDimensions(float desiredScale) override; + // |ImageGenerator| + SkISize GetScaledDimensions(float desiredScale) override; - static std::unique_ptr makeFromData(sk_sp data); + static std::unique_ptr MakeFromData(sk_sp data); private: - BuiltinSkiaCodecImageGenerator() = delete; + FML_DISALLOW_COPY_ASSIGN_AND_MOVE(BuiltinSkiaCodecImageGenerator); std::unique_ptr codec_generator_; }; diff --git a/lib/ui/painting/image_generator_registry.cc b/lib/ui/painting/image_generator_registry.cc index 741470d983b07..cc89ccb6d1387 100644 --- a/lib/ui/painting/image_generator_registry.cc +++ b/lib/ui/painting/image_generator_registry.cc @@ -17,17 +17,17 @@ namespace flutter { ImageGeneratorRegistry::ImageGeneratorRegistry() : weak_factory_(this) { - add( + AddFactory( [](sk_sp buffer) { - return BuiltinSkiaCodecImageGenerator::makeFromData(buffer); + return BuiltinSkiaCodecImageGenerator::MakeFromData(buffer); }, 0); #ifdef OS_MACOSX - add( + AddFactory( [](sk_sp buffer) { auto generator = SkImageGeneratorCG::MakeFromEncodedCG(buffer); - return BuiltinSkiaImageGenerator::makeFromGenerator( + return BuiltinSkiaImageGenerator::MakeFromGenerator( std::move(generator)); }, 0); @@ -42,15 +42,17 @@ ImageGeneratorRegistry::ImageGeneratorRegistry() : weak_factory_(this) { #endif } -void ImageGeneratorRegistry::add(ImageGeneratorFactory factory, - int32_t priority) { +ImageGeneratorRegistry::~ImageGeneratorRegistry() = default; + +void ImageGeneratorRegistry::AddFactory(ImageGeneratorFactory factory, + int32_t priority) { image_generator_factories_.push_back({factory, priority}); std::stable_sort(image_generator_factories_.begin(), image_generator_factories_.end()); } -std::unique_ptr ImageGeneratorRegistry::createCompatible( - sk_sp buffer) { +std::unique_ptr +ImageGeneratorRegistry::CreateCompatibleGenerator(sk_sp buffer) { for (auto& factory : image_generator_factories_) { std::unique_ptr result = factory.callback(buffer); if (result) { diff --git a/lib/ui/painting/image_generator_registry.h b/lib/ui/painting/image_generator_registry.h index 9b9e0d7382258..60e4f897313ab 100644 --- a/lib/ui/painting/image_generator_registry.h +++ b/lib/ui/painting/image_generator_registry.h @@ -24,29 +24,30 @@ class ImageGeneratorRegistry { ImageGeneratorRegistry(); - ~ImageGeneratorRegistry() = default; + ~ImageGeneratorRegistry(); - void add(ImageGeneratorFactory factory, int32_t priority); + void AddFactory(ImageGeneratorFactory factory, int32_t priority); /// Walks the list of image generator builders in descending priority order /// until a compatible SkImageGenerator is able to be built. If no compatible /// image generator could be produced, `std::unique_ptr(nullptr)` is returned. - std::unique_ptr createCompatible(sk_sp buffer); + std::unique_ptr CreateCompatibleGenerator( + sk_sp buffer); fml::WeakPtr GetWeakPtr() const; private: - struct PrioritizedFactory_ { + struct PrioritizedFactory { ImageGeneratorFactory callback; - int32_t priority; + int32_t priority = 0; // Order by descending priority. - inline bool operator<(const PrioritizedFactory_& other) const { + constexpr bool operator<(const PrioritizedFactory& other) const { return priority > other.priority; } }; - std::vector image_generator_factories_; + std::vector image_generator_factories_; fml::WeakPtrFactory weak_factory_; }; diff --git a/lib/ui/painting/image_generator_registry_unittests.cc b/lib/ui/painting/image_generator_registry_unittests.cc index fee78559eae6e..ea2640a340079 100644 --- a/lib/ui/painting/image_generator_registry_unittests.cc +++ b/lib/ui/painting/image_generator_registry_unittests.cc @@ -12,11 +12,7 @@ namespace flutter { namespace testing { static sk_sp LoadValidImageFixture() { - // Load the fixture with fml mappings. - auto fixtures_directory = - fml::OpenDirectory(GetFixturesPath(), false, fml::FilePermission::kRead); - auto fixture_mapping = fml::FileMapping::CreateReadOnly( - fixtures_directory, "DashInNooglerHat.jpg"); + auto fixture_mapping = OpenFixtureAsMapping("DashInNooglerHat.jpg"); // Remap to sk_sp. SkData::ReleaseProc on_release = [](const void* ptr, void* context) -> void { @@ -25,7 +21,10 @@ static sk_sp LoadValidImageFixture() { auto data = SkData::MakeWithProc(fixture_mapping->GetMapping(), fixture_mapping->GetSize(), on_release, fixture_mapping.get()); - fixture_mapping.release(); + + if (data) { + fixture_mapping.release(); + } return data; } @@ -35,15 +34,15 @@ TEST_F(ShellTest, CreateCompatibleReturnsBuiltinImageGeneratorForValidImage) { // Fetch the generator and query for basic info ImageGeneratorRegistry registry; - auto result = registry.createCompatible(data); - auto info = result->getInfo(); + auto result = registry.CreateCompatibleGenerator(data); + auto info = result->GetInfo(); ASSERT_EQ(info.width(), 3024); ASSERT_EQ(info.height(), 4032); } TEST_F(ShellTest, CreateCompatibleReturnsNullptrForInvalidImage) { ImageGeneratorRegistry registry; - auto result = registry.createCompatible(SkData::MakeEmpty()); + auto result = registry.CreateCompatibleGenerator(SkData::MakeEmpty()); ASSERT_EQ(result, nullptr); } @@ -55,13 +54,13 @@ class FakeImageGenerator : public ImageGenerator { SkColorType::kRGBA_8888_SkColorType, SkAlphaType::kOpaque_SkAlphaType)){}; ~FakeImageGenerator() = default; - const SkImageInfo& getInfo() const { return info_; } + const SkImageInfo& GetInfo() const { return info_; } - bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const { + bool GetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const { return false; }; - SkISize getScaledDimensions(float scale) { + SkISize GetScaledDimensions(float scale) { return SkISize::Make(info_.width(), info_.height()); } @@ -73,31 +72,31 @@ TEST_F(ShellTest, PositivePriorityTakesPrecedentOverDefaultGenerators) { ImageGeneratorRegistry registry; const int fakeWidth = 1337; - registry.add( + registry.AddFactory( [fakeWidth](sk_sp buffer) { return std::make_unique(fakeWidth); }, 1); // Fetch the generator and query for basic info. - auto result = registry.createCompatible(LoadValidImageFixture()); - ASSERT_EQ(result->getInfo().width(), fakeWidth); + auto result = registry.CreateCompatibleGenerator(LoadValidImageFixture()); + ASSERT_EQ(result->GetInfo().width(), fakeWidth); } TEST_F(ShellTest, DefaultGeneratorsTakePrecedentOverNegativePriority) { ImageGeneratorRegistry registry; - registry.add( + registry.AddFactory( [](sk_sp buffer) { return std::make_unique(1337); }, -1); // Fetch the generator and query for basic info. - auto result = registry.createCompatible(LoadValidImageFixture()); + auto result = registry.CreateCompatibleGenerator(LoadValidImageFixture()); // If the real width of the image pops out, then the default generator was // returned rather than the fake one. - ASSERT_EQ(result->getInfo().width(), 3024); + ASSERT_EQ(result->GetInfo().width(), 3024); } } // namespace testing From 802a5585bad3df39a02febf1a1ef489be2a95143 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 11 May 2021 21:22:22 -0700 Subject: [PATCH 03/10] Add docstrings --- lib/ui/painting/image_generator.cc | 15 ++--- lib/ui/painting/image_generator.h | 63 ++++++++++++++----- lib/ui/painting/image_generator_registry.h | 7 ++- .../image_generator_registry_unittests.cc | 14 +++-- 4 files changed, 67 insertions(+), 32 deletions(-) diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index 79c0a0dc374ab..be6dd66a4e774 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -20,11 +20,12 @@ const SkImageInfo& BuiltinSkiaImageGenerator::GetInfo() const { bool BuiltinSkiaImageGenerator::GetPixels(const SkImageInfo& info, void* pixels, - size_t rowBytes) const { - return generator_->getPixels(info, pixels, rowBytes); + size_t row_bytes) const { + return generator_->getPixels(info, pixels, row_bytes); } -SkISize BuiltinSkiaImageGenerator::GetScaledDimensions(float desiredScale) { +SkISize BuiltinSkiaImageGenerator::GetScaledDimensions( + float desired_scale) const { return generator_->getInfo().dimensions(); } @@ -54,13 +55,13 @@ const SkImageInfo& BuiltinSkiaCodecImageGenerator::GetInfo() const { bool BuiltinSkiaCodecImageGenerator::GetPixels(const SkImageInfo& info, void* pixels, - size_t rowBytes) const { - return codec_generator_->getPixels(info, pixels, rowBytes); + size_t row_bytes) const { + return codec_generator_->getPixels(info, pixels, row_bytes); } SkISize BuiltinSkiaCodecImageGenerator::GetScaledDimensions( - float desiredScale) { - return codec_generator_->getScaledDimensions(desiredScale); + float desired_scale) const { + return codec_generator_->getScaledDimensions(desired_scale); } std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( diff --git a/lib/ui/painting/image_generator.h b/lib/ui/painting/image_generator.h index af4f69454e68f..23a083d9d1094 100644 --- a/lib/ui/painting/image_generator.h +++ b/lib/ui/painting/image_generator.h @@ -11,26 +11,57 @@ namespace flutter { -/// The minimal interface necessary for creating decoders that can be used for -/// single-frame image decoding with optional support for decoding into a -/// subscaled buffer. -/// -/// See `getScaledDimensions` for details about how subpixel image decoding can -/// be supported. +/// @brief The minimal interface necessary for defining a decoder that can be +/// used for single-frame image decoding. Image generators can +/// optionally support decoding into a subscaled buffer. +/// Implementers of `ImageGenerator` regularly keep internal state which +/// is not thread safe, and so aliasing and parallel access should never +/// be done with `ImageGenerator`s. +/// @see `ImageGenerator::GetScaledDimensions` class ImageGenerator { public: virtual ~ImageGenerator(); + /// @brief Returns basic information about the contents of the encoded + /// image. This information can almost always be collected by just + /// interpreting the header of a decoded image. + /// @return Size and color information describing the image. + /// @note This method is executed on the UI thread and used for layout + /// purposes by the framework, and so this method should not perform + /// long synchronous tasks. virtual const SkImageInfo& GetInfo() const = 0; + /// @brief Decode the image into a given buffer. + /// @param[in] info The desired size and color info of the decoded + /// image to be returned. The implementation of + /// `GetScaledDimensions` determines which sizes are + /// supported by the image decoder. + /// @param[in] pixels The location where the raw decoded image data + /// should be written. + /// @param[in] row_bytes The total number of bytes that should make up a + /// single row of decoded image data + /// (i.e. width * bytes_per_pixel). + /// @return True if the image was successfully decoded. + /// @note This method performs potentially long synchronous work, and so + /// it should never be executed on the UI thread. Image decoders + /// do not require GPU acceleration, and so threads without a GPU + /// context may also be used. + /// @see `GetScaledDimensions` virtual bool GetPixels(const SkImageInfo& info, void* pixels, - size_t rowBytes) const = 0; - - /// Given a scale value, returns the closest image size that can be used for - /// efficiently decoding the image. If subpixel image decoding is not - /// supported by the codec, this method just returns the original image size. - virtual SkISize GetScaledDimensions(float scale) = 0; + size_t row_bytes) const = 0; + + /// @brief Given a scale value, find the closest image size that can be + /// used for efficiently decoding the image. If subpixel image + /// decoding is not supported by the decoder, this method should + /// just return the original image size. + /// @param[in] scale The desired scale factor of the image for decoding. + /// @return The closest image size that can be used for efficiently + /// decoding the image. + /// @note This method is called prior to `GetPixels` in order to query + /// for supported sizes. + /// @see `GetPixels` + virtual SkISize GetScaledDimensions(float scale) const = 0; }; class BuiltinSkiaImageGenerator : public ImageGenerator { @@ -45,10 +76,10 @@ class BuiltinSkiaImageGenerator : public ImageGenerator { // |ImageGenerator| bool GetPixels(const SkImageInfo& info, void* pixels, - size_t rowBytes) const override; + size_t row_bytes) const override; // |ImageGenerator| - SkISize GetScaledDimensions(float desiredScale) override; + SkISize GetScaledDimensions(float desired_scale) const override; static std::unique_ptr MakeFromGenerator( std::unique_ptr generator); @@ -72,10 +103,10 @@ class BuiltinSkiaCodecImageGenerator : public ImageGenerator { // |ImageGenerator| bool GetPixels(const SkImageInfo& info, void* pixels, - size_t rowBytes) const override; + size_t row_bytes) const override; // |ImageGenerator| - SkISize GetScaledDimensions(float desiredScale) override; + SkISize GetScaledDimensions(float desired_scale) const override; static std::unique_ptr MakeFromData(sk_sp data); diff --git a/lib/ui/painting/image_generator_registry.h b/lib/ui/painting/image_generator_registry.h index 60e4f897313ab..fb6aaad8ce80f 100644 --- a/lib/ui/painting/image_generator_registry.h +++ b/lib/ui/painting/image_generator_registry.h @@ -14,9 +14,10 @@ namespace flutter { -/// Keeps a priority-ordered registry of image generator builders to be used -/// when decoding images. This object must be created, accessed, and collected -/// on the UI thread (typically the engine or its runtime controller). +/// @brief Keeps a priority-ordered registry of image generator builders to be +/// used when decoding images. This object must be created, accessed, and +/// collected on the UI thread (typically the engine or its runtime +/// controller). class ImageGeneratorRegistry { public: using ImageGeneratorFactory = diff --git a/lib/ui/painting/image_generator_registry_unittests.cc b/lib/ui/painting/image_generator_registry_unittests.cc index ea2640a340079..69965c30dd90b 100644 --- a/lib/ui/painting/image_generator_registry_unittests.cc +++ b/lib/ui/painting/image_generator_registry_unittests.cc @@ -56,11 +56,13 @@ class FakeImageGenerator : public ImageGenerator { ~FakeImageGenerator() = default; const SkImageInfo& GetInfo() const { return info_; } - bool GetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) const { + bool GetPixels(const SkImageInfo& info, + void* pixels, + size_t row_bytes) const { return false; }; - SkISize GetScaledDimensions(float scale) { + SkISize GetScaledDimensions(float scale) const { return SkISize::Make(info_.width(), info_.height()); } @@ -71,16 +73,16 @@ class FakeImageGenerator : public ImageGenerator { TEST_F(ShellTest, PositivePriorityTakesPrecedentOverDefaultGenerators) { ImageGeneratorRegistry registry; - const int fakeWidth = 1337; + const int fake_width = 1337; registry.AddFactory( - [fakeWidth](sk_sp buffer) { - return std::make_unique(fakeWidth); + [fake_width](sk_sp buffer) { + return std::make_unique(fake_width); }, 1); // Fetch the generator and query for basic info. auto result = registry.CreateCompatibleGenerator(LoadValidImageFixture()); - ASSERT_EQ(result->GetInfo().width(), fakeWidth); + ASSERT_EQ(result->GetInfo().width(), fake_width); } TEST_F(ShellTest, DefaultGeneratorsTakePrecedentOverNegativePriority) { From 5b741f8fa61684d62189e3b2a0ded6498bb47499 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 12 May 2021 14:50:30 -0700 Subject: [PATCH 04/10] Add more docstrings --- lib/ui/painting/image_generator_registry.h | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/image_generator_registry.h b/lib/ui/painting/image_generator_registry.h index fb6aaad8ce80f..287937a61a4a2 100644 --- a/lib/ui/painting/image_generator_registry.h +++ b/lib/ui/painting/image_generator_registry.h @@ -27,11 +27,27 @@ class ImageGeneratorRegistry { ~ImageGeneratorRegistry(); + /// @brief Install a new factory for image generators + /// @param[in] factory Callback that produces `ImageGenerator`s for + /// compatible input data. + /// @param[in] priority The priority used to determine the order in which + /// factories are tried. Higher values mean higher + /// priority. The built-in Skia codecs are installed at + /// priority 0. + /// @see `CreateCompatibleGenerator` void AddFactory(ImageGeneratorFactory factory, int32_t priority); - /// Walks the list of image generator builders in descending priority order - /// until a compatible SkImageGenerator is able to be built. If no compatible - /// image generator could be produced, `std::unique_ptr(nullptr)` is returned. + /// @brief Walks the list of image generator builders in descending + /// priority order until a compatible `ImageGenerator` is able to + /// be built. This method is safe to perform on the UI thread, as + /// checking for `ImageGenerator` compatibility is expected to be + /// a lightweight operation. The returned `ImageGenerator` can + /// then be used to fully decode the image on e.g. the IO thread. + /// @param[in] buffer The raw encoded image data. + /// @return An `ImageGenerator` that is compatible with the input buffer. + /// If no compatible `ImageGenerator` type was found, then + /// `std::unique_ptr(nullptr)` is returned. + /// @see `ImageGenerator` std::unique_ptr CreateCompatibleGenerator( sk_sp buffer); From 2ab297a809bf8b34c666c222cf7ed922597ac769 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 12 May 2021 16:06:51 -0700 Subject: [PATCH 05/10] Add extra var for sorting when priorities are equal --- lib/ui/painting/image_generator_registry.cc | 3 ++- lib/ui/painting/image_generator_registry.h | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/image_generator_registry.cc b/lib/ui/painting/image_generator_registry.cc index cc89ccb6d1387..15276d50469bc 100644 --- a/lib/ui/painting/image_generator_registry.cc +++ b/lib/ui/painting/image_generator_registry.cc @@ -4,6 +4,7 @@ #include +#include "flutter/fml/trace_event.h" #include "flutter/lib/ui/painting/image_generator_registry.h" #include "third_party/skia/include/codec/SkCodec.h" #include "third_party/skia/include/core/SkImageGenerator.h" @@ -46,7 +47,7 @@ ImageGeneratorRegistry::~ImageGeneratorRegistry() = default; void ImageGeneratorRegistry::AddFactory(ImageGeneratorFactory factory, int32_t priority) { - image_generator_factories_.push_back({factory, priority}); + image_generator_factories_.push_back({factory, priority, fml::tracing::TraceNonce()}); std::stable_sort(image_generator_factories_.begin(), image_generator_factories_.end()); } diff --git a/lib/ui/painting/image_generator_registry.h b/lib/ui/painting/image_generator_registry.h index 287937a61a4a2..d3de17709acb9 100644 --- a/lib/ui/painting/image_generator_registry.h +++ b/lib/ui/painting/image_generator_registry.h @@ -32,8 +32,11 @@ class ImageGeneratorRegistry { /// compatible input data. /// @param[in] priority The priority used to determine the order in which /// factories are tried. Higher values mean higher - /// priority. The built-in Skia codecs are installed at - /// priority 0. + /// priority. The built-in Skia decoders are installed + /// at priority 0, and so a priority > 0 takes precedent + /// over the builtin decoders. When multiple decoders + /// are added with the same priority, those which are + /// added earlier take precedent. /// @see `CreateCompatibleGenerator` void AddFactory(ImageGeneratorFactory factory, int32_t priority); @@ -58,9 +61,17 @@ class ImageGeneratorRegistry { ImageGeneratorFactory callback; int32_t priority = 0; + // Used as a fallback priority comparison when equal. + size_t ascending_nonce = 0; // Order by descending priority. constexpr bool operator<(const PrioritizedFactory& other) const { + // When priorities are equal, use the order by which the factories were + // registered. + if (priority == other.priority) { + return ascending_nonce < other.ascending_nonce; + } + // Order by descending priority. return priority > other.priority; } }; From b89fbe380dd158f5ccef7f6160eb390b8979942d Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 13 May 2021 14:48:45 -0700 Subject: [PATCH 06/10] Use ordered set --- lib/ui/painting/image_generator_registry.cc | 5 ++--- lib/ui/painting/image_generator_registry.h | 22 ++++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/ui/painting/image_generator_registry.cc b/lib/ui/painting/image_generator_registry.cc index 15276d50469bc..b56f2e461fcb3 100644 --- a/lib/ui/painting/image_generator_registry.cc +++ b/lib/ui/painting/image_generator_registry.cc @@ -47,9 +47,8 @@ ImageGeneratorRegistry::~ImageGeneratorRegistry() = default; void ImageGeneratorRegistry::AddFactory(ImageGeneratorFactory factory, int32_t priority) { - image_generator_factories_.push_back({factory, priority, fml::tracing::TraceNonce()}); - std::stable_sort(image_generator_factories_.begin(), - image_generator_factories_.end()); + image_generator_factories_.insert( + {factory, priority, fml::tracing::TraceNonce()}); } std::unique_ptr diff --git a/lib/ui/painting/image_generator_registry.h b/lib/ui/painting/image_generator_registry.h index d3de17709acb9..2630fb7a8f156 100644 --- a/lib/ui/painting/image_generator_registry.h +++ b/lib/ui/painting/image_generator_registry.h @@ -6,7 +6,7 @@ #define FLUTTER_LIB_UI_PAINTING_IMAGE_GENERATOR_REGISTRY_H_ #include -#include +#include #include "flutter/fml/mapping.h" #include "flutter/fml/memory/weak_ptr.h" @@ -63,19 +63,23 @@ class ImageGeneratorRegistry { int32_t priority = 0; // Used as a fallback priority comparison when equal. size_t ascending_nonce = 0; + }; - // Order by descending priority. - constexpr bool operator<(const PrioritizedFactory& other) const { - // When priorities are equal, use the order by which the factories were - // registered. - if (priority == other.priority) { - return ascending_nonce < other.ascending_nonce; + struct Compare { + constexpr bool operator()(const PrioritizedFactory& lhs, + const PrioritizedFactory& rhs) const { + // When priorities are equal, factories registered earlier take + // precedent. + if (lhs.priority == rhs.priority) { + return lhs.ascending_nonce < rhs.ascending_nonce; } // Order by descending priority. - return priority > other.priority; + return lhs.priority > rhs.priority; } }; - std::vector image_generator_factories_; + + using FactorySet = std::set; + FactorySet image_generator_factories_; fml::WeakPtrFactory weak_factory_; }; From 6f2955373f94c7352cc99935901185ad89fe0129 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Fri, 14 May 2021 19:12:04 -0700 Subject: [PATCH 07/10] Add support for multiframe codecs directly in ImageGenerator --- lib/ui/painting/image_generator.cc | 53 ++++++-- lib/ui/painting/image_generator.h | 121 +++++++++++++----- .../image_generator_registry_unittests.cc | 18 ++- 3 files changed, 149 insertions(+), 43 deletions(-) diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index be6dd66a4e774..18832d3d56fa6 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -18,10 +18,15 @@ const SkImageInfo& BuiltinSkiaImageGenerator::GetInfo() const { return generator_->getInfo(); } -bool BuiltinSkiaImageGenerator::GetPixels(const SkImageInfo& info, - void* pixels, - size_t row_bytes) const { - return generator_->getPixels(info, pixels, row_bytes); +uint BuiltinSkiaImageGenerator::GetFrameCount() const { + return 1; +} + +const ImageGenerator::FrameInfo BuiltinSkiaImageGenerator::GetFrameInfo( + uint frame_index) const { + return {.required_frame = std::nullopt, + .duration = 0, + .disposal_method = SkCodecAnimation::DisposalMethod::kKeep}; } SkISize BuiltinSkiaImageGenerator::GetScaledDimensions( @@ -29,6 +34,15 @@ SkISize BuiltinSkiaImageGenerator::GetScaledDimensions( return generator_->getInfo().dimensions(); } +bool BuiltinSkiaImageGenerator::GetPixels( + const SkImageInfo& info, + void* pixels, + size_t row_bytes, + uint frame_index, + std::optional prior_frame) const { + return generator_->getPixels(info, pixels, row_bytes); +} + BuiltinSkiaCodecImageGenerator::~BuiltinSkiaCodecImageGenerator() = default; std::unique_ptr BuiltinSkiaImageGenerator::MakeFromGenerator( @@ -53,10 +67,19 @@ const SkImageInfo& BuiltinSkiaCodecImageGenerator::GetInfo() const { return codec_generator_->getInfo(); } -bool BuiltinSkiaCodecImageGenerator::GetPixels(const SkImageInfo& info, - void* pixels, - size_t row_bytes) const { - return codec_generator_->getPixels(info, pixels, row_bytes); +uint BuiltinSkiaCodecImageGenerator::GetFrameCount() const { + return codec_generator_->getFrameCount(); +} + +const ImageGenerator::FrameInfo BuiltinSkiaCodecImageGenerator::GetFrameInfo( + uint frame_index) const { + SkCodec::FrameInfo info; + codec_generator_->getFrameInfo(frame_index, &info); + return {.required_frame = info.fRequiredFrame == SkCodec::kNoFrame + ? std::nullopt + : std::optional(info.fRequiredFrame), + .duration = static_cast(info.fDuration), + .disposal_method = info.fDisposalMethod}; } SkISize BuiltinSkiaCodecImageGenerator::GetScaledDimensions( @@ -64,6 +87,20 @@ SkISize BuiltinSkiaCodecImageGenerator::GetScaledDimensions( return codec_generator_->getScaledDimensions(desired_scale); } +bool BuiltinSkiaCodecImageGenerator::GetPixels( + const SkImageInfo& info, + void* pixels, + size_t row_bytes, + uint frame_index, + std::optional prior_frame) const { + SkCodec::Options options; + options.fFrameIndex = frame_index; + if (prior_frame.has_value()) { + options.fPriorFrame = prior_frame.value(); + } + return codec_generator_->getPixels(info, pixels, row_bytes, &options); +} + std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( sk_sp data) { auto codec = SkCodec::MakeFromData(data); diff --git a/lib/ui/painting/image_generator.h b/lib/ui/painting/image_generator.h index 23a083d9d1094..f6ceae4198d14 100644 --- a/lib/ui/painting/image_generator.h +++ b/lib/ui/painting/image_generator.h @@ -12,14 +12,28 @@ namespace flutter { /// @brief The minimal interface necessary for defining a decoder that can be -/// used for single-frame image decoding. Image generators can -/// optionally support decoding into a subscaled buffer. -/// Implementers of `ImageGenerator` regularly keep internal state which -/// is not thread safe, and so aliasing and parallel access should never -/// be done with `ImageGenerator`s. +/// used for both single and multi-frame image decoding. Image +/// generators can also optionally support decoding into a subscaled +/// buffer. Implementers of `ImageGenerator` regularly keep internal +/// state which is not thread safe, and so aliasing and parallel access +/// should never be done with `ImageGenerator`s. /// @see `ImageGenerator::GetScaledDimensions` class ImageGenerator { public: + /// @brief Info about a single frame in the context of a multi-frame image, + /// useful for animation and blending. + struct FrameInfo { + /// The frame index of the frame that, if any, this frame needs to be + /// blended with. + std::optional required_frame; + + /// Number of milliseconds to show this frame. + uint duration; + + /// How this frame should be modified before decoding the next one. + SkCodecAnimation::DisposalMethod disposal_method; + }; + virtual ~ImageGenerator(); /// @brief Returns basic information about the contents of the encoded @@ -31,25 +45,24 @@ class ImageGenerator { /// long synchronous tasks. virtual const SkImageInfo& GetInfo() const = 0; - /// @brief Decode the image into a given buffer. - /// @param[in] info The desired size and color info of the decoded - /// image to be returned. The implementation of - /// `GetScaledDimensions` determines which sizes are - /// supported by the image decoder. - /// @param[in] pixels The location where the raw decoded image data - /// should be written. - /// @param[in] row_bytes The total number of bytes that should make up a - /// single row of decoded image data - /// (i.e. width * bytes_per_pixel). - /// @return True if the image was successfully decoded. - /// @note This method performs potentially long synchronous work, and so - /// it should never be executed on the UI thread. Image decoders - /// do not require GPU acceleration, and so threads without a GPU - /// context may also be used. - /// @see `GetScaledDimensions` - virtual bool GetPixels(const SkImageInfo& info, - void* pixels, - size_t row_bytes) const = 0; + /// @brief Get the number of frames that the encoded image stores. This + /// method is always expected to be called before `GetFrameInfo`, as + /// the underlying image decoder may interpret frame information that + /// is then used when calling `GetFrameInfo`. + /// @return The number of frames that the encoded image stores. This will + /// always be 1 for single-frame images. + virtual uint GetFrameCount() const = 0; + + /// @brief Get information about a single frame in the context of a + /// multi-frame image, useful for animation and frame blending. + /// This method should only ever be called after `GetFrameCount` + /// has been called. This information is nonsensical for + /// single-frame images. + /// @param[in] frame_index The index of the frame to get information about. + /// @return Information about the given frame. If the image is + /// single-frame, a default result is returned. + /// @see `GetFrameCount` + virtual const FrameInfo GetFrameInfo(uint frame_index) const = 0; /// @brief Given a scale value, find the closest image size that can be /// used for efficiently decoding the image. If subpixel image @@ -62,6 +75,38 @@ class ImageGenerator { /// for supported sizes. /// @see `GetPixels` virtual SkISize GetScaledDimensions(float scale) const = 0; + + /// @brief Decode the image into a given buffer. + /// @param[in] info The desired size and color info of the decoded + /// image to be returned. The implementation of + /// `GetScaledDimensions` determines which sizes are + /// supported by the image decoder. + /// @param[in] pixels The location where the raw decoded image data + /// should be written. + /// @param[in] row_bytes The total number of bytes that should make up a + /// single row of decoded image data + /// (i.e. width * bytes_per_pixel). + /// @param[in] frame_index Which frame to decode. This is only useful for + /// multi-frame images. + /// @param[in] prior_frame Optional frame index parameter for multi-frame + /// images which specifies the previous frame that + /// should be use for blending. This hints to the + /// decoder that it should use a previously cached + /// frame instead of decoding dependency frame(s). + /// If an empty value is supplied, the decoder should + /// decode any necessary frames first. + /// @return True if the image was successfully decoded. + /// @note This method performs potentially long synchronous work, and so + /// it should never be executed on the UI thread. Image decoders + /// do not require GPU acceleration, and so threads without a GPU + /// context may also be used. + /// @see `GetScaledDimensions` + virtual bool GetPixels( + const SkImageInfo& info, + void* pixels, + size_t row_bytes, + uint frame_index = 0, + std::optional prior_frame = std::nullopt) const = 0; }; class BuiltinSkiaImageGenerator : public ImageGenerator { @@ -74,13 +119,21 @@ class BuiltinSkiaImageGenerator : public ImageGenerator { const SkImageInfo& GetInfo() const override; // |ImageGenerator| - bool GetPixels(const SkImageInfo& info, - void* pixels, - size_t row_bytes) const override; + uint GetFrameCount() const override; + + // |ImageGenerator| + const ImageGenerator::FrameInfo GetFrameInfo(uint frame_index) const override; // |ImageGenerator| SkISize GetScaledDimensions(float desired_scale) const override; + // |ImageGenerator| + bool GetPixels(const SkImageInfo& info, + void* pixels, + size_t row_bytes, + uint frame_index = 0, + std::optional prior_frame = std::nullopt) const override; + static std::unique_ptr MakeFromGenerator( std::unique_ptr generator); @@ -101,13 +154,21 @@ class BuiltinSkiaCodecImageGenerator : public ImageGenerator { const SkImageInfo& GetInfo() const override; // |ImageGenerator| - bool GetPixels(const SkImageInfo& info, - void* pixels, - size_t row_bytes) const override; + uint GetFrameCount() const override; + + // |ImageGenerator| + const ImageGenerator::FrameInfo GetFrameInfo(uint frame_index) const override; // |ImageGenerator| SkISize GetScaledDimensions(float desired_scale) const override; + // |ImageGenerator| + bool GetPixels(const SkImageInfo& info, + void* pixels, + size_t row_bytes, + uint frame_index = 0, + std::optional prior_frame = std::nullopt) const override; + static std::unique_ptr MakeFromData(sk_sp data); private: diff --git a/lib/ui/painting/image_generator_registry_unittests.cc b/lib/ui/painting/image_generator_registry_unittests.cc index 69965c30dd90b..12dcc5b31cb58 100644 --- a/lib/ui/painting/image_generator_registry_unittests.cc +++ b/lib/ui/painting/image_generator_registry_unittests.cc @@ -56,16 +56,24 @@ class FakeImageGenerator : public ImageGenerator { ~FakeImageGenerator() = default; const SkImageInfo& GetInfo() const { return info_; } - bool GetPixels(const SkImageInfo& info, - void* pixels, - size_t row_bytes) const { - return false; - }; + uint GetFrameCount() const { return 1; } + + const ImageGenerator::FrameInfo GetFrameInfo(uint frame_index) const { + return {std::nullopt, 0, SkCodecAnimation::DisposalMethod::kKeep}; + } SkISize GetScaledDimensions(float scale) const { return SkISize::Make(info_.width(), info_.height()); } + bool GetPixels(const SkImageInfo& info, + void* pixels, + size_t row_bytes, + uint frame_index, + std::optional prior_frame) const { + return false; + }; + private: SkImageInfo info_; }; From 237fcd5ac075fdc20ef60a589f2a013d05467698 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Fri, 14 May 2021 20:28:03 -0700 Subject: [PATCH 08/10] Add todo for installing image decoder factories though embedders --- lib/ui/painting/image_generator_registry.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ui/painting/image_generator_registry.cc b/lib/ui/painting/image_generator_registry.cc index b56f2e461fcb3..96eb4fce03b84 100644 --- a/lib/ui/painting/image_generator_registry.cc +++ b/lib/ui/painting/image_generator_registry.cc @@ -24,6 +24,7 @@ ImageGeneratorRegistry::ImageGeneratorRegistry() : weak_factory_(this) { }, 0); + // todo(bdero): https://github.com/flutter/flutter/issues/82603 #ifdef OS_MACOSX AddFactory( [](sk_sp buffer) { From 867a5709491c9c6949045e89cd2d2e40943b0acf Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Mon, 17 May 2021 10:25:28 -0700 Subject: [PATCH 09/10] Use unsigned int instead of uint --- lib/ui/painting/image_generator.cc | 27 ++++++------ lib/ui/painting/image_generator.h | 44 ++++++++++--------- .../image_generator_registry_unittests.cc | 8 ++-- 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index 18832d3d56fa6..e40ae56b73e4f 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -18,12 +18,12 @@ const SkImageInfo& BuiltinSkiaImageGenerator::GetInfo() const { return generator_->getInfo(); } -uint BuiltinSkiaImageGenerator::GetFrameCount() const { +unsigned int BuiltinSkiaImageGenerator::GetFrameCount() const { return 1; } const ImageGenerator::FrameInfo BuiltinSkiaImageGenerator::GetFrameInfo( - uint frame_index) const { + unsigned int frame_index) const { return {.required_frame = std::nullopt, .duration = 0, .disposal_method = SkCodecAnimation::DisposalMethod::kKeep}; @@ -38,8 +38,8 @@ bool BuiltinSkiaImageGenerator::GetPixels( const SkImageInfo& info, void* pixels, size_t row_bytes, - uint frame_index, - std::optional prior_frame) const { + unsigned int frame_index, + std::optional prior_frame) const { return generator_->getPixels(info, pixels, row_bytes); } @@ -67,19 +67,20 @@ const SkImageInfo& BuiltinSkiaCodecImageGenerator::GetInfo() const { return codec_generator_->getInfo(); } -uint BuiltinSkiaCodecImageGenerator::GetFrameCount() const { +unsigned int BuiltinSkiaCodecImageGenerator::GetFrameCount() const { return codec_generator_->getFrameCount(); } const ImageGenerator::FrameInfo BuiltinSkiaCodecImageGenerator::GetFrameInfo( - uint frame_index) const { + unsigned int frame_index) const { SkCodec::FrameInfo info; codec_generator_->getFrameInfo(frame_index, &info); - return {.required_frame = info.fRequiredFrame == SkCodec::kNoFrame - ? std::nullopt - : std::optional(info.fRequiredFrame), - .duration = static_cast(info.fDuration), - .disposal_method = info.fDisposalMethod}; + return { + .required_frame = info.fRequiredFrame == SkCodec::kNoFrame + ? std::nullopt + : std::optional(info.fRequiredFrame), + .duration = static_cast(info.fDuration), + .disposal_method = info.fDisposalMethod}; } SkISize BuiltinSkiaCodecImageGenerator::GetScaledDimensions( @@ -91,8 +92,8 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( const SkImageInfo& info, void* pixels, size_t row_bytes, - uint frame_index, - std::optional prior_frame) const { + unsigned int frame_index, + std::optional prior_frame) const { SkCodec::Options options; options.fFrameIndex = frame_index; if (prior_frame.has_value()) { diff --git a/lib/ui/painting/image_generator.h b/lib/ui/painting/image_generator.h index f6ceae4198d14..0b1937eb18611 100644 --- a/lib/ui/painting/image_generator.h +++ b/lib/ui/painting/image_generator.h @@ -25,10 +25,10 @@ class ImageGenerator { struct FrameInfo { /// The frame index of the frame that, if any, this frame needs to be /// blended with. - std::optional required_frame; + std::optional required_frame; /// Number of milliseconds to show this frame. - uint duration; + unsigned int duration; /// How this frame should be modified before decoding the next one. SkCodecAnimation::DisposalMethod disposal_method; @@ -51,7 +51,7 @@ class ImageGenerator { /// is then used when calling `GetFrameInfo`. /// @return The number of frames that the encoded image stores. This will /// always be 1 for single-frame images. - virtual uint GetFrameCount() const = 0; + virtual unsigned int GetFrameCount() const = 0; /// @brief Get information about a single frame in the context of a /// multi-frame image, useful for animation and frame blending. @@ -62,7 +62,7 @@ class ImageGenerator { /// @return Information about the given frame. If the image is /// single-frame, a default result is returned. /// @see `GetFrameCount` - virtual const FrameInfo GetFrameInfo(uint frame_index) const = 0; + virtual const FrameInfo GetFrameInfo(unsigned int frame_index) const = 0; /// @brief Given a scale value, find the closest image size that can be /// used for efficiently decoding the image. If subpixel image @@ -105,8 +105,8 @@ class ImageGenerator { const SkImageInfo& info, void* pixels, size_t row_bytes, - uint frame_index = 0, - std::optional prior_frame = std::nullopt) const = 0; + unsigned int frame_index = 0, + std::optional prior_frame = std::nullopt) const = 0; }; class BuiltinSkiaImageGenerator : public ImageGenerator { @@ -119,20 +119,22 @@ class BuiltinSkiaImageGenerator : public ImageGenerator { const SkImageInfo& GetInfo() const override; // |ImageGenerator| - uint GetFrameCount() const override; + unsigned int GetFrameCount() const override; // |ImageGenerator| - const ImageGenerator::FrameInfo GetFrameInfo(uint frame_index) const override; + const ImageGenerator::FrameInfo GetFrameInfo( + unsigned int frame_index) const override; // |ImageGenerator| SkISize GetScaledDimensions(float desired_scale) const override; // |ImageGenerator| - bool GetPixels(const SkImageInfo& info, - void* pixels, - size_t row_bytes, - uint frame_index = 0, - std::optional prior_frame = std::nullopt) const override; + bool GetPixels( + const SkImageInfo& info, + void* pixels, + size_t row_bytes, + unsigned int frame_index = 0, + std::optional prior_frame = std::nullopt) const override; static std::unique_ptr MakeFromGenerator( std::unique_ptr generator); @@ -154,20 +156,22 @@ class BuiltinSkiaCodecImageGenerator : public ImageGenerator { const SkImageInfo& GetInfo() const override; // |ImageGenerator| - uint GetFrameCount() const override; + unsigned int GetFrameCount() const override; // |ImageGenerator| - const ImageGenerator::FrameInfo GetFrameInfo(uint frame_index) const override; + const ImageGenerator::FrameInfo GetFrameInfo( + unsigned int frame_index) const override; // |ImageGenerator| SkISize GetScaledDimensions(float desired_scale) const override; // |ImageGenerator| - bool GetPixels(const SkImageInfo& info, - void* pixels, - size_t row_bytes, - uint frame_index = 0, - std::optional prior_frame = std::nullopt) const override; + bool GetPixels( + const SkImageInfo& info, + void* pixels, + size_t row_bytes, + unsigned int frame_index = 0, + std::optional prior_frame = std::nullopt) const override; static std::unique_ptr MakeFromData(sk_sp data); diff --git a/lib/ui/painting/image_generator_registry_unittests.cc b/lib/ui/painting/image_generator_registry_unittests.cc index 12dcc5b31cb58..736e5c7c6c04a 100644 --- a/lib/ui/painting/image_generator_registry_unittests.cc +++ b/lib/ui/painting/image_generator_registry_unittests.cc @@ -56,9 +56,9 @@ class FakeImageGenerator : public ImageGenerator { ~FakeImageGenerator() = default; const SkImageInfo& GetInfo() const { return info_; } - uint GetFrameCount() const { return 1; } + unsigned int GetFrameCount() const { return 1; } - const ImageGenerator::FrameInfo GetFrameInfo(uint frame_index) const { + const ImageGenerator::FrameInfo GetFrameInfo(unsigned int frame_index) const { return {std::nullopt, 0, SkCodecAnimation::DisposalMethod::kKeep}; } @@ -69,8 +69,8 @@ class FakeImageGenerator : public ImageGenerator { bool GetPixels(const SkImageInfo& info, void* pixels, size_t row_bytes, - uint frame_index, - std::optional prior_frame) const { + unsigned int frame_index, + std::optional prior_frame) const { return false; }; From 3063a98e7e78e70d37990a2b607999ac2a65c3aa Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Mon, 17 May 2021 10:40:59 -0700 Subject: [PATCH 10/10] Fix name in windows generator factory --- lib/ui/painting/image_generator_registry.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting/image_generator_registry.cc b/lib/ui/painting/image_generator_registry.cc index 96eb4fce03b84..f02762014b13d 100644 --- a/lib/ui/painting/image_generator_registry.cc +++ b/lib/ui/painting/image_generator_registry.cc @@ -34,10 +34,10 @@ ImageGeneratorRegistry::ImageGeneratorRegistry() : weak_factory_(this) { }, 0); #elif OS_WIN - add( + AddFactory( [](sk_sp buffer) { auto generator = SkImageGeneratorWIC::MakeFromEncodedWIC(buffer); - return BuiltinSkiaImageGenerator::makeFromGenerator( + return BuiltinSkiaImageGenerator::MakeFromGenerator( std::move(generator)); }, 0);