-
Notifications
You must be signed in to change notification settings - Fork 6k
SKP based shader warmup #20643
SKP based shader warmup #20643
Changes from all commits
d2010c9
9728537
6c9924f
cd1fe95
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| #include <future> | ||
| #include <memory> | ||
|
|
||
| #include "assets/directory_asset_bundle.h" | ||
| #include "flutter/flow/layers/layer_tree.h" | ||
| #include "flutter/flow/layers/picture_layer.h" | ||
| #include "flutter/flow/layers/transform_layer.h" | ||
|
|
@@ -2208,5 +2209,72 @@ TEST_F(ShellTest, EngineRootIsolateLaunchesDontTakeVMDataSettings) { | |
| isolate_create_latch.Wait(); | ||
| } | ||
|
|
||
| TEST_F(ShellTest, AssetManagerSingle) { | ||
| fml::ScopedTemporaryDirectory asset_dir; | ||
| fml::UniqueFD asset_dir_fd = fml::OpenDirectory( | ||
| asset_dir.path().c_str(), false, fml::FilePermission::kRead); | ||
|
|
||
| std::string filename = "test_name"; | ||
| std::string content = "test_content"; | ||
|
|
||
| bool success = fml::WriteAtomically(asset_dir_fd, filename.c_str(), | ||
| fml::DataMapping(content)); | ||
| ASSERT_TRUE(success); | ||
|
|
||
| AssetManager asset_manager; | ||
| asset_manager.PushBack( | ||
| std::make_unique<DirectoryAssetBundle>(std::move(asset_dir_fd), false)); | ||
|
|
||
| auto mapping = asset_manager.GetAsMapping(filename); | ||
| ASSERT_TRUE(mapping != nullptr); | ||
|
|
||
| std::string result(reinterpret_cast<const char*>(mapping->GetMapping()), | ||
| mapping->GetSize()); | ||
|
|
||
| ASSERT_TRUE(result == content); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: add a new line here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
|
||
| TEST_F(ShellTest, AssetManagerMulti) { | ||
| fml::ScopedTemporaryDirectory asset_dir; | ||
| fml::UniqueFD asset_dir_fd = fml::OpenDirectory( | ||
| asset_dir.path().c_str(), false, fml::FilePermission::kRead); | ||
|
|
||
| std::vector<std::string> filenames = { | ||
| "good0", | ||
| "bad0", | ||
| "good1", | ||
| "bad1", | ||
| }; | ||
|
|
||
| for (auto filename : filenames) { | ||
| bool success = fml::WriteAtomically(asset_dir_fd, filename.c_str(), | ||
| fml::DataMapping(filename)); | ||
| ASSERT_TRUE(success); | ||
| } | ||
|
|
||
| AssetManager asset_manager; | ||
| asset_manager.PushBack( | ||
| std::make_unique<DirectoryAssetBundle>(std::move(asset_dir_fd), false)); | ||
|
|
||
| auto mappings = asset_manager.GetAsMappings("(.*)"); | ||
| ASSERT_TRUE(mappings.size() == 4); | ||
|
|
||
| std::vector<std::string> expected_results = { | ||
| "good0", | ||
| "good1", | ||
| }; | ||
|
|
||
| mappings = asset_manager.GetAsMappings("(.*)good(.*)"); | ||
| ASSERT_TRUE(mappings.size() == expected_results.size()); | ||
|
|
||
| for (auto& mapping : mappings) { | ||
| std::string result(reinterpret_cast<const char*>(mapping->GetMapping()), | ||
| mapping->GetSize()); | ||
| ASSERT_NE( | ||
| std::find(expected_results.begin(), expected_results.end(), result), | ||
| expected_results.end()); | ||
| } | ||
| } | ||
|
|
||
| } // namespace testing | ||
| } // namespace flutter | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,15 +19,19 @@ | |
| #include "flutter/shell/common/switches.h" | ||
| #include "flutter/shell/version/version.h" | ||
| #include "flutter/testing/testing.h" | ||
| #include "include/core/SkFont.h" | ||
| #include "include/core/SkPicture.h" | ||
| #include "include/core/SkPictureRecorder.h" | ||
| #include "include/core/SkSerialProcs.h" | ||
| #include "include/core/SkTextBlob.h" | ||
|
|
||
| #if defined(OS_FUCHSIA) | ||
| #include "lib/sys/cpp/component_context.h" | ||
| #include "third_party/skia/include/ports/SkFontMgr_fuchsia.h" | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need an #endif here or the namespace declarations are stripped out, this will cause problems on non-fuchsia builds
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this ifdef block wraps basically the entire file including the close braces on the namespace. I'll take an action item to comment the endif with "// defined(OS_FUCHSIA)" though |
||
| namespace flutter { | ||
| namespace testing { | ||
|
|
||
| #if defined(OS_FUCHSIA) | ||
|
|
||
| static void WaitForIO(Shell* shell) { | ||
| std::promise<bool> io_task_finished; | ||
| shell->GetTaskRunners().GetIOTaskRunner()->PostTask( | ||
|
|
@@ -103,8 +107,13 @@ class SkpWarmupTest : public ShellTest { | |
|
|
||
| SkDeserialProcs procs = {0}; | ||
| procs.fImageProc = DeserializeImageWithoutData; | ||
| procs.fTypefaceProc = DeserializeTypefaceWithoutData; | ||
| sk_sp<SkPicture> picture = | ||
| SkPicture::MakeFromStream(stream.get(), &procs); | ||
| if (!picture) { | ||
| FML_LOG(ERROR) << "Failed to deserialize " << filename; | ||
| return true; | ||
| } | ||
| pictures.push_back(std::move(picture)); | ||
| fd.reset(); | ||
| } | ||
|
|
@@ -242,7 +251,51 @@ TEST_F(SkpWarmupTest, Image) { | |
| TestWarmup(draw_size, builder); | ||
| } | ||
|
|
||
| #endif | ||
| // Re-enable once https://bugs.chromium.org/p/skia/issues/detail?id=10404 | ||
| // is fixed and integrated, or a workaround is found. | ||
| TEST_F(SkpWarmupTest, DISABLED_Text) { | ||
| auto context = sys::ComponentContext::Create(); | ||
| fuchsia::fonts::ProviderSyncPtr sync_font_provider; | ||
| context->svc()->Connect(sync_font_provider.NewRequest()); | ||
| auto font_mgr = SkFontMgr_New_Fuchsia(std::move(sync_font_provider)); | ||
| auto raw_typeface = | ||
| font_mgr->matchFamilyStyle(nullptr, SkFontStyle::Normal()); | ||
| auto typeface = sk_sp<SkTypeface>(raw_typeface); | ||
|
|
||
| SkFont font(typeface, 12); | ||
| auto text_blob = | ||
| SkTextBlob::MakeFromString("test", font, SkTextEncoding::kUTF8); | ||
|
|
||
| SkISize draw_size = | ||
| SkISize::Make(text_blob->bounds().width(), text_blob->bounds().height()); | ||
| // We reuse this builder to draw the same content sever times in this test | ||
| LayerTreeBuilder builder = [&draw_size, &text_blob, | ||
| this](std::shared_ptr<ContainerLayer> root) { | ||
| SkPictureRecorder recorder; | ||
|
|
||
| auto canvas = | ||
| recorder.beginRecording(draw_size.width(), draw_size.height()); | ||
|
|
||
| auto color_space = SkColorSpace::MakeSRGB(); | ||
| auto paint = SkPaint(SkColors::kWhite, color_space.get()); | ||
| canvas->drawTextBlob(text_blob, draw_size.width() / 2, | ||
| draw_size.height() / 2, paint); | ||
|
|
||
| auto picture = recorder.finishRecordingAsPicture(); | ||
|
|
||
| fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>( | ||
| this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); | ||
| auto picture_layer = std::make_shared<PictureLayer>( | ||
| SkPoint::Make(0, 0), SkiaGPUObject<SkPicture>(picture, queue), | ||
| /* is_complex */ false, | ||
| /* will_change */ false); | ||
| root->Add(picture_layer); | ||
| }; | ||
|
|
||
| TestWarmup(draw_size, builder); | ||
| } | ||
|
|
||
| } // namespace testing | ||
| } // namespace flutter | ||
|
|
||
| #endif // defined(OS_FUCHSIA) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why the old line didn't work. I guess they should both save space, so is it because
typeface->serialize(SkTypeface::SerializeBehavior::kDontIncludeData)somehow interfered with the Fuchsia typeface whileSkData::MakeEmpty()won't?Is this line also the reason why we need
DeserializeTypefaceWithoutData?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit is from a while back so I don't 100% remember. IIRC even serializing typefaces with SerializeBehavior::kDontIncludeData caused the deserialized SkPictures to not be renderable (or maybe failed to deserialize, i dont quite remember) and talking with the Skia people about it I was advised that if all I wanted to do was warm up shaders they could vend be a special standin typeface which warms up all the typeface shaders and had me file https://bugs.chromium.org/p/skia/issues/detail?id=10404 for it. So basically this just serialized nothing and then on the deserialize path creates the default typeface. Once that skia feature is in DeserializeTypefaceWithoutData will return the special warmup typeface rather than the default typeface.