Revert #78 #69 #122

bbrto21 · 2021-06-22T04:47:05Z

This PR includes :

Revert "Create fallback font manager to solve performance drops (Create fallback font manager to solve performance drops #78)"
Revert "Fix font breaking issues (Fix font breaking issues #69)"
Add SamsungOneUI to DefaultFontFamilies

Now, re-enable fontconfig as default

…ter-tizen#78)" This reverts commit c761164.

This reverts commit 5552dd5.

swift-kim · 2021-06-22T05:00:19Z

Could you compare app launching performance when font_config is enabled vs. disabled? You can use my tool for measurement if you want.

third_party/txt/src/txt/platform_linux.cc

Signed-off-by: Boram Bae <[email protected]>

bbrto21 · 2021-06-22T05:27:33Z

Could you compare app launching performance when font_config is enabled vs. disabled? You can use my tool for measurement if you want.

I'll try it!

bwikbs · 2021-06-22T05:27:59Z

You can use my tool for measurement if you want.

I was really surprised. You have a personal PPA! 😱

bbrto21 · 2021-06-22T07:25:29Z

@swift-kim I measured it using your tool.

As is -> Enable Fontconfig(On TM1)

Application	STIME (ms)	PSS (KB)
com.example.flutter_example_gallery	2160 -> 2669 (+23.6%)	66509 -> 67236 (+1.1%)

As is -> Enable Fontconfig(On R840)

Application	STIME (ms)	PSS (KB)
com.example.flutter_example_gallery	756 -> 888 (+17.5%)	94463 -> 95399 (+1.0%)

bwikbs

I think that using font config is almost the only solution to display fonts correctly.
Thanks for your working!

HakkyuKim · 2021-06-23T02:15:12Z

Oh.. I will add "SamsungOneUI". But why? Could you give me the detailed reason?

@bbrto21
Adding "SamsungOneUI" will be on a separate PR, right?

bbrto21 · 2021-06-23T02:28:16Z

Oh.. I will add "SamsungOneUI". But why? Could you give me the detailed reason?

@bbrto21
Adding "SamsungOneUI" will be on a separate PR, right?

I missed that, thank you!

swift-kim · 2021-06-23T03:07:49Z

@bbrto21 The result on R840 seems to be a bit misleading because the Evas GL renderer is used on the device and AUL cannot detect the time when the first frame of the app is drawn correctly.

I measured the launching time again using the --trace-startup option in profile mode and the result is as follows. Of course there are some errors although I've tried to minimize them.

On TW3 (tizen-6.0-unified_20201216.3_wearable-wayland-armv7l-tw3),

(Gallery/native)
  "timeToFrameworkInitMicros": 685318 -> 755597             (+70 ms),
  "timeToFirstFrameRasterizedMicros": 1856285 -> 2271489    (+415 ms),
  "timeToFirstFrameMicros": 1328676 -> 1798041              (+469 ms),
  "timeAfterFrameworkInitMicros": 643358 -> 1042444         (+399 ms)

(template/dotnet)
  "timeToFrameworkInitMicros": 439135 -> 468878             (+30 ms),
  "timeToFirstFrameRasterizedMicros": 934794 -> 1278863     (+344 ms),
  "timeToFirstFrameMicros": 757955 -> 1235623               (+478 ms),
  "timeAfterFrameworkInitMicros": 318820 -> 766745          (+448 ms)

On rpi4-arm64 (tizen-6.0-unified_20210331.1_iot-headed-3parts-aarch64-rpi),

(Gallery/native)
  "timeToFrameworkInitMicros": 316548 -> 316397             (-0 ms),
  "timeToFirstFrameRasterizedMicros": 1232617 -> 1384432    (+152 ms),
  "timeToFirstFrameMicros": 588777 -> 721575                (+133 ms),
  "timeAfterFrameworkInitMicros": 272229 -> 405178          (+133 ms)

(template/dotnet)
  "timeToFrameworkInitMicros": 99613 -> 109041              (+1 ms),
  "timeToFirstFrameRasterizedMicros": 492748 -> 598000      (+105 ms),
  "timeToFirstFrameMicros": 183035 -> 347693                (+165 ms),
  "timeAfterFrameworkInitMicros": 83422 -> 238652           (+155 ms)

The performance regression is quite obvious when fontconfig is enabled. Are we sure we want to apply this change anyway?

bbrto21 · 2021-06-23T03:41:36Z

The performance regression is quite obvious when fontconfig is enabled. Are we sure we want to apply this change anyway?

IMHO, Obviously the first start time increases, I think it's still an acceptable starting time.
and I believe that enabling fontconfig as default is the right way

bwikbs · 2021-06-23T03:53:31Z

The performance regression is quite obvious when fontconfig is enabled. Are we sure we want to apply this change anyway?

IMO, This is something worth taking.
Because font setting on the device is set by font config.

xuelian-bai · 2021-06-29T00:18:43Z

What's the advantage of enabling font config?
I'm profiling tvplus, before applying this patch, Engine::SetupDefaultFontManager took 29ms, after applying this patch, this function took about 170.866ms.

xuelian-bai · 2021-06-29T00:27:37Z

is it for missing text issue? anyway, if Engine::SetupDefaultFontManager took much time, it will postphone DartIsolate::CreateRootIsolate()(They are in the same thread), then postphone everything.

xuelian-bai · 2021-06-29T00:49:37Z

sorry, please ignore the above comments, I tested again twice, time consumption is different, I will check more.

bwikbs · 2021-06-29T00:51:27Z

is it for missing text issue?

yes

if Engine::SetupDefaultFontManager took much time, it will postphone DartIsolate::CreateRootIsolate()(They are in the same thread), then postphone everything.

We knew there would be a performance problem, and we introduced it because there was no other way to fix that issue.

First of All, we will look for areas that can be optimized, and if it is still a problem, we are considering the option of not using only that app.

bwikbs · 2021-06-29T00:54:08Z

I tested again twice, time consumption is different.

Maybe it's because of the font config cache.

bbrto21 · 2021-06-29T01:02:49Z

@xuelian-bai
From my understanding, unless we enable the fontconfig, we can't fully address the missing text problem. What we've done before this PR has only been a partial solution.
Yes, you are right, and we mentioned above, there is definitely a decrease in performance when initialize engine after this PR merged. But I believe that this performance is in a general state of engine basically, If we really need a performance improvement for initialization, I think the current state is the starting line to improve performance

HakkyuKim · 2021-06-29T01:22:15Z

FYI, the details of the text rendering issue: #96.

I hope it's a one-time 141ms delay on start-up. At the time, we decided to be correct than be performant but if the performance drop is noticeable, please share your findings so we can make improvements.

Additionally, there was another issue that got solved by using font-config: flutter-tizen/flutter-tizen#104.

swift-kim · 2021-06-29T01:50:34Z

@xuelian-bai As a workaround, if your app doesn't use a custom font and you really want to optimize the startup performance, you can use a custom build of the engine without fontconfig enabled by reverting this commit.

xuelian-bai · 2021-06-29T02:13:29Z

Ok, thanks for explain, now I know it's necessary for this patch.
Here is the detailed testing result(tested on NikeM, profile mode), it's werid for the second launch, anyway, I will try to optimize launch performance based on this patch.

launch	Before	After(Enable font config)
first launch(cold boot)	322.271 ms	293.018 ms
second launch	38.831 ms	161.433 ms
third launch	24.956ms	30.158 ms

* Revert "Create fallback font manager to solve performance drops (#78)" This reverts commit c761164. * Revert "Fix font breaking issues (#69)" This reverts commit 5552dd5. * Add SamsungOneUI to DefaultFontFamilies Signed-off-by: Boram Bae <[email protected]>

bbrto21 added 2 commits June 22, 2021 13:00

Revert "Create fallback font manager to solve performance drops (flut…

0d74486

…ter-tizen#78)" This reverts commit c761164.

Revert "Fix font breaking issues (flutter-tizen#69)"

cf361f2

This reverts commit 5552dd5.

bwikbs reviewed Jun 22, 2021

View reviewed changes

third_party/txt/src/txt/platform_linux.cc Outdated Show resolved Hide resolved

Add SamsungOneUI to DefaultFontFamilies

0cad48a

Signed-off-by: Boram Bae <[email protected]>

bbrto21 marked this pull request as draft June 22, 2021 05:08

bbrto21 marked this pull request as ready for review June 22, 2021 07:28

bwikbs approved these changes Jun 23, 2021

View reviewed changes

swift-kim approved these changes Jun 23, 2021

View reviewed changes

bwikbs merged commit 399c885 into flutter-tizen:flutter-2.2.1-tizen Jun 23, 2021

This was referenced Jun 23, 2021

Font not rendering in some cases #96

Closed

[TV emulator] cmap table size error on app launch flutter-tizen/flutter-tizen#104

Closed

swift-kim pushed a commit that referenced this pull request Sep 1, 2022

Remove base.h from base/BUILD (#122)

93035a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert #78 #69 #122

Revert #78 #69 #122

bbrto21 commented Jun 22, 2021 •

edited

Loading

swift-kim commented Jun 22, 2021

bbrto21 commented Jun 22, 2021

bwikbs commented Jun 22, 2021

bbrto21 commented Jun 22, 2021 •

edited

Loading

bwikbs left a comment

HakkyuKim commented Jun 23, 2021

bbrto21 commented Jun 23, 2021

swift-kim commented Jun 23, 2021

bbrto21 commented Jun 23, 2021

bwikbs commented Jun 23, 2021

xuelian-bai commented Jun 29, 2021

xuelian-bai commented Jun 29, 2021

xuelian-bai commented Jun 29, 2021

bwikbs commented Jun 29, 2021 •

edited

Loading

bwikbs commented Jun 29, 2021

bbrto21 commented Jun 29, 2021 •

edited

Loading

HakkyuKim commented Jun 29, 2021

swift-kim commented Jun 29, 2021

xuelian-bai commented Jun 29, 2021

Revert #78 #69 #122

Revert #78 #69 #122

Conversation

bbrto21 commented Jun 22, 2021 • edited Loading

swift-kim commented Jun 22, 2021

bbrto21 commented Jun 22, 2021

bwikbs commented Jun 22, 2021

bbrto21 commented Jun 22, 2021 • edited Loading

bwikbs left a comment

Choose a reason for hiding this comment

HakkyuKim commented Jun 23, 2021

bbrto21 commented Jun 23, 2021

swift-kim commented Jun 23, 2021

bbrto21 commented Jun 23, 2021

bwikbs commented Jun 23, 2021

xuelian-bai commented Jun 29, 2021

xuelian-bai commented Jun 29, 2021

xuelian-bai commented Jun 29, 2021

bwikbs commented Jun 29, 2021 • edited Loading

bwikbs commented Jun 29, 2021

bbrto21 commented Jun 29, 2021 • edited Loading

HakkyuKim commented Jun 29, 2021

swift-kim commented Jun 29, 2021

xuelian-bai commented Jun 29, 2021

bbrto21 commented Jun 22, 2021 •

edited

Loading

bbrto21 commented Jun 22, 2021 •

edited

Loading

bwikbs commented Jun 29, 2021 •

edited

Loading

bbrto21 commented Jun 29, 2021 •

edited

Loading