Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NaN font metrics by initializing typeface for TextStyle #1087

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

SergeevPavel
Copy link

otherwise fontMetrics that we are using below might be unreliable
see: https://github.com/JetBrains/skia/blob/6843251caf4eb4134ab91e98c8063df3cab792b1/modules/skparagraph/src/TextStyle.cpp#L172

if fTypeface isn't initialized we will get some default metrics, it may be e.g. all zeroes in some configurations, and it may lead to Inf and NaN values

Proposed Changes

Testing

Test: Describe how you tested your changes. Note that this line (with Test:) is required, your PR will not build without it!

Issues Fixed

Fixes: [Optional] The bug on https://issuetracker.google.com being fixed

Google CLA

You need to sign the Google Contributor’s License Agreement at https://cla.developers.google.com/.
This is needed since we synchronise most of the code with Google’s AOSP repository. Signing this agreement allows us to synchronise code from your Pull Requests as well.

otherwise fontMetrics that we are using below  might be unreliable

This comment was marked as resolved.

@SergeevPavel
Copy link
Author

@MatkovIvan could you also merge it when it's done? I have no write access to this repo

@MatkovIvan MatkovIvan changed the title initialize typeface for TextStyle Initialize typeface for TextStyle Feb 15, 2024
@MatkovIvan MatkovIvan merged commit 38adecb into JetBrains:jb-main Feb 15, 2024
4 of 5 checks passed
@MatkovIvan MatkovIvan changed the title Initialize typeface for TextStyle Fix NaN font metrics by initializing typeface for TextStyle Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants