Skip to content

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