Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jason-simmons
Copy link
Member

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@jason-simmons
Copy link
Member Author

@chinmaygarde

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Apr 4, 2018
@jason-simmons jason-simmons merged commit 31e5af6 into flutter:master Apr 4, 2018
cbracken added a commit to cbracken/flutter that referenced this pull request Apr 4, 2018
Includes:
* libtxt: add missing dependency for Mac/iOS builds (flutter/engine#4931)
* libtxt: use Helvetica as a fallback font on iOS 8 and earlier (flutter/engine#4930)
* Make Rect.largest match the "largest" rect used in DefaultLayerBuilder (flutter/engine#4926)
* [vulkan] Add VK_ERROR_NOT_PERMITTED_EXT
* Roll buildroot to 8dddd90 (flutter/engine#4928)
* Set the asset bundle path when initializing the shell in the embedder API (flutter/engine#4925)
cbracken added a commit to flutter/flutter that referenced this pull request Apr 4, 2018
Includes:
* libtxt: add missing dependency for Mac/iOS builds (flutter/engine#4931)
* libtxt: use Helvetica as a fallback font on iOS 8 and earlier (flutter/engine#4930)
* Make Rect.largest match the "largest" rect used in DefaultLayerBuilder (flutter/engine#4926)
* [vulkan] Add VK_ERROR_NOT_PERMITTED_EXT
* Roll buildroot to 8dddd90 (flutter/engine#4928)
* Set the asset bundle path when initializing the shell in the embedder API (flutter/engine#4925)
@Hixie
Copy link
Contributor

Hixie commented Apr 4, 2018

Why is this necessary? We should be automatically falling back on all the fonts on the system anyway, so changing the ultimate backstop shouldn't make a difference.

@jason-simmons
Copy link
Member Author

Minikin expects to be given a collection of all relevant fonts for a given text block before doing layout. The libtxt font subsystem builds the collection by asking the Skia font managers to supply fonts for the requested font family. If no font manager can match the family, then libtxt asks the font managers to match the platform's default font family. If that also fails, then layout can't proceed.

Later we added a patch to Minikin that queries Skia to find a font matching any character that isn't covered by the font collection. If Skia can't find a font family, then we could try experimenting with a model where we provide an empty font collection and rely on per character fallback to find some usable font.

Note that Minikin currently requires that font collections be nonempty:
https://github.com/flutter/engine/blob/master/third_party/txt/src/minikin/FontCollection.cpp#L87

I'm not sure how deeply embedded this assumption is in Minikin.

@Hixie
Copy link
Contributor

Hixie commented Apr 8, 2018

Ah, interesting.

DaveShuckerow pushed a commit to DaveShuckerow/flutter that referenced this pull request May 14, 2018
Includes:
* libtxt: add missing dependency for Mac/iOS builds (flutter/engine#4931)
* libtxt: use Helvetica as a fallback font on iOS 8 and earlier (flutter/engine#4930)
* Make Rect.largest match the "largest" rect used in DefaultLayerBuilder (flutter/engine#4926)
* [vulkan] Add VK_ERROR_NOT_PERMITTED_EXT
* Roll buildroot to 8dddd90 (flutter/engine#4928)
* Set the asset bundle path when initializing the shell in the embedder API (flutter/engine#4925)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants