Check the presence of glyphs when selecting font #113

bwikbs · 2021-06-14T07:15:43Z

PR due to default branch change

previous PR : Check the presence of glyphs when selecting font #106

This PR is to solve the font issue by re-enable font config and changing font selection logic(with workaround)
PR that uses the font config as the default and reverts our modification will be prepared separately.
Also, it contains the following minor change.

Change fallback font name to TizenDefaultFont : The font config setting in the Tizen device has an alias called TizenDefaultFont, and it is connected to the actual font in device. So if we decide to use the font config we can use it as a fallback font family.

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]>

Signed-off-by: MuHong Byun <[email protected]>

…, it is checked more. Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]>

Signed-off-by: MuHong Byun <[email protected]>

bwikbs · 2021-06-14T07:21:22Z

update) It's my fault. No difference after upgrade.

After uploading to 2.2.1, a new problem occurred. 😢

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]>

bbrto21

The workaround added to the sorting algorithm seems to be the best we can do right now as long as we should use SamsungOneUI font family

swift-kim

Are you going to request this change for merge (maybe partially) in the upstream?

third_party/txt/src/minikin/FontCollection.cpp

third_party/txt/src/minikin/FontFamily.cpp

third_party/txt/src/txt/font_collection.cc

swift-kim · 2021-06-17T06:46:53Z

third_party/txt/src/txt/platform_linux.cc

 std::vector<std::string> GetDefaultFontFamilies() {
+#ifdef FLUTTER_USE_FONTCONFIG
+  return {"TizenDefaultFont"};
+#else


Do we need this change (caf01dd) if we're not to re-enable fontconfig?

No, TizenDefaultFont is not an actual font. That's an alias in fontconfig

Then you might revert the commit.

I'm sorry but I don't get the point. Because..?

The commit in this context is caf01dd.

I'm still not sure if I understood your intentions...

This PR is base on fontconfig. And PR that uses font config again and revert unnecessary our modification will be prepared another PR (Perhaps @bbrto21 Am I right boram? 😄 ).
The reason for doing that is because it would be good to track the related fontconfig in the future.(Just my opinion)

I'm sorry for the lack of explanation of this.

#106 (review) by @bbrto21:

I think you need to re-enable fontconfig in azure-pipelines.

#106 (comment) by @bwikbs:

Reverting existing code and applying fontconfig seem to be a little different topic, so I would like to talk about it in another PR.

If this PR depends on fontconfig, why not re-enable the fontconfig option in azure-pipelines.yml? The change inside the #ifdef FLUTTER_USE_FONTCONFIG block doesn't affect anything without the option enabled, so I thought you could safely revert the change. I'm sorry but I still don't get your point.

@swift-kim Well... You're right. I just wanted to collect the related re-enable font config contents in one PR.
There is no great reason...(To be honest, I didn't want to revert and clean-up the existing ones. And because it seems that @bbrto21 has already done that work, I was going to ask for him)
I didn't think much about it.. If it's too weird, I'll enable the font config right away.

Okay, if there will be a follow-up PR, you may just briefly mention what changes will be made in that PR and how TizenDefaultFont is related to those changes.

I've updated PR description!

bwikbs · 2021-06-17T07:21:35Z

Are you going to request this change for merge (maybe partially) in the upstream?

@swift-kim I think I can do some of it ( yes, maybe.. I didn't really think about how far it would go.), but overall no. Most of the problems come from the unusual setting of product.

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Swift Kim <[email protected]>

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> Co-authored-by: Swift Kim <[email protected]>

* Check the presence of glyphs when selecting font Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> * Change default font family name Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> * Fix coding style Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> * Cache the glyphs and fonts used when matching fonts Signed-off-by: MuHong Byun <[email protected]> * Apply review's comment Signed-off-by: MuHong Byun <[email protected]> * When matching fonts, if the existing character is a special character, it is checked more. Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> * Apply review's comment Signed-off-by: MuHong Byun <[email protected]> * Fixed sorting to make emoji fonts go backwards Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> * Apply review's comment Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Swift Kim <[email protected]> * Apply review's comment Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> Co-authored-by: Swift Kim <[email protected]> Co-authored-by: Boram Bae <[email protected]> Co-authored-by: Swift Kim <[email protected]>

* Check the presence of glyphs when selecting font * Change default font family name * Fix coding style * Cache the glyphs and fonts used when matching fonts * When matching fonts, if the existing character is a special character, it is checked more. * Fixed sorting to make emoji fonts go backwards Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> Co-authored-by: Swift Kim <[email protected]>

bwikbs and others added 7 commits June 14, 2021 15:21

Check the presence of glyphs when selecting font

21741dc

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]>

Change default font family name

caf01dd

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]>

Fix coding style

1b3125e

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]>

Cache the glyphs and fonts used when matching fonts

5135217

Signed-off-by: MuHong Byun <[email protected]>

Apply review's comment

139fab6

Signed-off-by: MuHong Byun <[email protected]>

When matching fonts, if the existing character is a special character…

5713f1e

…, it is checked more. Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]>

Apply review's comment

5ed9fd7

Signed-off-by: MuHong Byun <[email protected]>

bwikbs mentioned this pull request Jun 14, 2021

Check the presence of glyphs when selecting font #106

Closed

Fixed sorting to make emoji fonts go backwards

1b2bbf0

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]>

bwikbs force-pushed the fix_font_issue_2.2.1 branch from d8a2cb2 to 1b2bbf0 Compare June 14, 2021 07:43

bwikbs requested review from a team, HakkyuKim and bbrto21 June 17, 2021 04:47

bbrto21 approved these changes Jun 17, 2021

View reviewed changes

swift-kim reviewed Jun 17, 2021

View reviewed changes

bwikbs and others added 2 commits June 17, 2021 16:43

Apply review's comment

bfe940f

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Swift Kim <[email protected]>

Apply review's comment

16e3397

Signed-off-by: MuHong Byun <[email protected]> Co-authored-by: Boram Bae <[email protected]> Co-authored-by: Swift Kim <[email protected]>

swift-kim merged commit 44d5ce1 into flutter-tizen:flutter-2.2.1-tizen Jun 22, 2021

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

Wire up enhanced command buffer error reporting. (#113)

2dd3797

Check the presence of glyphs when selecting font #113

Check the presence of glyphs when selecting font #113

Uh oh!

Conversation

bwikbs commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bwikbs commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bbrto21 left a comment

Choose a reason for hiding this comment

Uh oh!

swift-kim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwikbs commented Jun 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bwikbs commented Jun 14, 2021 •

edited

Loading

bwikbs commented Jun 14, 2021 •

edited

Loading