-
Notifications
You must be signed in to change notification settings - Fork 6k
Add system font change listener for windows #12276
Changes from all commits
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 |
|---|---|---|
|
|
@@ -235,6 +235,17 @@ TEST(EmbedderTestNoFixture, CanGetCurrentTimeInNanoseconds) { | |
| ASSERT_LT((point2 - point1), fml::TimeDelta::FromMilliseconds(1)); | ||
| } | ||
|
|
||
| TEST_F(EmbedderTest, CanReloadSystemFonts) { | ||
|
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 test make sure the hook from embedding -> embedding_engine -> shell works. It does not test for functionality. |
||
| auto& context = GetEmbedderContext(); | ||
| EmbedderConfigBuilder builder(context); | ||
| builder.SetSoftwareRendererConfig(); | ||
| auto engine = builder.LaunchEngine(); | ||
| ASSERT_TRUE(engine.is_valid()); | ||
|
|
||
| auto result = FlutterEngineReloadSystemFonts(engine.get()); | ||
| ASSERT_EQ(result, kSuccess); | ||
| } | ||
|
|
||
| TEST_F(EmbedderTest, CanCreateOpenGLRenderingEngine) { | ||
| EmbedderConfigBuilder builder(GetEmbedderContext()); | ||
| builder.SetOpenGLRendererConfig(SkISize::Make(1, 1)); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -147,6 +147,13 @@ void Win32FlutterWindow::OnClose() { | |||
| messageloop_running_ = false; | ||||
| } | ||||
|
|
||||
| void Win32FlutterWindow::OnFontChange() { | ||||
|
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. @kpozin Here is the current design for system font api.
Noted that you will probably need to implement fuchsia version of loading default fonts. Let me know if this looks ok to you.
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. Yes, this seems fine to me.
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. Thanks for confirming, I will let you know if anything change.
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. @kpozin a little update engine/shell/platform/embedder/embedder.h Line 1211 in aadd5a3
I make it so it will send the system message as part of the api. However, you will still need to implement fuchsia version of loading default fonts. https://github.com/flutter/engine/blob/master/third_party/txt/src/txt/platform_fuchsia.cc |
||||
| if (engine_ == nullptr) { | ||||
| return; | ||||
| } | ||||
| FlutterEngineReloadSystemFonts(engine_); | ||||
| } | ||||
|
|
||||
| // Sends new size information to FlutterEngine. | ||||
| void Win32FlutterWindow::SendWindowMetrics() { | ||||
| if (engine_ == nullptr) { | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // 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 "third_party/skia/include/ports/SkTypeface_win.h" | ||
| #include "txt/platform.h" | ||
|
|
||
| namespace txt { | ||
|
|
||
| std::string GetDefaultFontFamily() { | ||
| return "Arial"; | ||
|
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. This change seems unrelated. Is this improving font fallback handling? If so, it would be great to split this out into its own change as part of #39915 Also, I'm surprised to see a file being added to third_party. What is the source for
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 is essential for reloading system fonts. The original GetDefaultFontFamily in third_party/txt/src/txt/platform.cc uses SkFontMgr::RefDefault() which will cache the result and there is no way to clear it. When a new system font is installed, it will not load the latest system fonts but return previous cached. We have to implement the windows version of it. There is no upstream for txt. The txt's source lives in here. One thing that worries me is that this might be migrated to skia library at some point @jason-simmons Do you think it is ok to change it now?
Member
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. Yes - go ahead and modify this in libtxt. if we later migrate to Skia's text shaper then we will need to port this feature over to the integration with Skia. |
||
| } | ||
|
|
||
| sk_sp<SkFontMgr> GetDefaultFontManager() { | ||
| return SkFontMgr_New_DirectWrite(); | ||
| } | ||
|
|
||
| } // namespace txt | ||
Uh oh!
There was an error while loading. Please reload this page.
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 tests the actual functionality