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

Add baseline to font measurement #2320

Merged
merged 18 commits into from
Jul 27, 2021
Merged

Conversation

andydotxyz
Copy link
Member

Description:

Add font information to (driver) text measurement.
The MeasureText call remains consistent so no public API breakage (except driver, as discussed elsewhere).
There could be a new version of that to expose baseline info, but I don't yet see the need.

What this also did was fix precision of text calculations so a lot of tests/images have changed slightly.

Fixes #1859

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.
  • Public APIs match existing style.

Update RichText to realign lines where the baseline varies across the row
@andydotxyz andydotxyz requested a review from fpabl0 July 12, 2021 20:50
driver.go Outdated Show resolved Hide resolved
internal/driver/glfw/driver.go Outdated Show resolved Hide resolved
internal/driver/glfw/driver_test.go Show resolved Hide resolved
internal/driver/gomobile/driver.go Outdated Show resolved Hide resolved
widget/richtext.go Outdated Show resolved Hide resolved
widget/richtext.go Show resolved Hide resolved
widget/richtext.go Outdated Show resolved Hide resolved
widget/richtext.go Outdated Show resolved Hide resolved
driver.go Outdated Show resolved Hide resolved
widget/richtext.go Outdated Show resolved Hide resolved
widget/richtext.go Outdated Show resolved Hide resolved
widget/richtext.go Outdated Show resolved Hide resolved
@andydotxyz andydotxyz requested a review from fpabl0 July 22, 2021 12:13
internal/cache/svg.go Outdated Show resolved Hide resolved
internal/cache/text.go Outdated Show resolved Hide resolved
internal/cache/text.go Outdated Show resolved Hide resolved
@andydotxyz andydotxyz requested a review from fpabl0 July 26, 2021 11:41
internal/cache/text.go Outdated Show resolved Hide resolved
internal/cache/text.go Outdated Show resolved Hide resolved
internal/cache/text.go Show resolved Hide resolved
internal/painter/font.go Outdated Show resolved Hide resolved
@andydotxyz
Copy link
Member Author

Sorry I keep coming to this and fixing a naming then we find some more.
Can you please let me know when the review is complete and I will fix all the items found?

@fpabl0
Copy link
Member

fpabl0 commented Jul 27, 2021

Sorry, it is quite difficult to navigate through 537 files. I thought that when FontMetrics rename was accepted, fontSizeCache would be renamed as it seems to be the more logical change. Same in RenderedTextSize as fontSize parameter was selected in cache functions it should use the same name, but that is only for clarity, other than that everything looks good :).

@andydotxyz
Copy link
Member Author

Not a problem, the font painter names fixed but the cache keys are size/style not metric so I think that one is OK.

Copy link
Member

@fpabl0 fpabl0 left a comment

Choose a reason for hiding this comment

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

LGTM! :) Sorry any inconveniences and delays.

@andydotxyz
Copy link
Member Author

Sorry any inconveniences and delays

Not at all, thanks for your help getting this over the line.

@andydotxyz andydotxyz merged commit f80990b into fyne-io:develop Jul 27, 2021
@andydotxyz andydotxyz deleted the fix/1859 branch July 27, 2021 20:40
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