Skip to content

fvmetrics.c: partial revert of 1033bc6 #5167

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

Merged
merged 1 commit into from
Dec 31, 2022
Merged

Conversation

jtanx
Copy link
Contributor

@jtanx jtanx commented Dec 31, 2022

Fixes #4955

Type of change

  • Bug fix

@frank-trampe
Copy link
Contributor

@ctrlcctrlv, would you review this one? It's going to take either of us a while to process the relevant code, but you have seen more of it than I have and probably have better test cases.

@ctrlcctrlv
Copy link
Member

Yes, I can.

@ctrlcctrlv
Copy link
Member

God, this part of the code is such a slog and has such a complicated history, but I think I understand it. The most important commit is bbde2a5 as that's where George tried to fix this originally in 2012 and is likely a good version, but it predates certain additions like fvt_addlayer. The version @jtanx proposes seems to match in the ways that matters.

To be certain, though, I did the following runtime tests:

  • lbearing to 200 image
  • both bearings to 113 image
  • rbearing to 90 image
  • width to 500 image
  • and then both back to 200 image

Given all that, I think this is fine to merge. It's certainly less broken than master, and I found no issues.

@ctrlcctrlv ctrlcctrlv merged commit e89f68a into fontforge:master Dec 31, 2022
@jtanx jtanx deleted the gg2 branch December 31, 2022 08:41
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.

Wrong behavior of: Metrics - Set Both Bearing
3 participants