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

Conversation

@kjlubick
Copy link
Contributor

Addresses #137565 (at least for Linux, which I tested locally. May need another follow-up when we try to revert this Skia change).

If we want a way to get a specific, repeatable font, we can modify this (perhaps by exposing MakeTypefaceFromName).

As Skia continues to migrate its SkFontMgr API away from having a default factory, third_party/txt/.../platform.h seems like a useful, central place to have these font things go.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@kjlubick kjlubick requested a review from flar October 30, 2023 20:58
Copy link
Contributor

@flar flar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the license file is updated as indicated in the task failure message.

Copy link
Member

@jason-simmons jason-simmons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll create a PR that updates the affected tests to select a specific font.

@kjlubick kjlubick closed this Oct 31, 2023
@kjlubick
Copy link
Contributor Author

#47493 is the better fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants