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.

Sign up for GitHub

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

Jump to bottom

[makeotfexe] port tx subroutinizer to makeotfexe #882

Merged
merged 1 commit into from
Aug 6, 2019
Merged

[makeotfexe] port tx subroutinizer to makeotfexe #882

merged 1 commit into from
Aug 6, 2019

Conversation

cjchapman
Copy link
Contributor

resolves #331

miguelsousa
miguelsousa reviewed Aug 5, 2019
View reviewed changes
c/makeotf/makeotf_lib/source/typecomp/subr.c Outdated
DAWG using the concatenation of all the charstrings from all the fonts as
input. Subseqeuntly the completed suffix DAWG is traversed in order to count
CDAWG using the concatenation of all the charstrings from all the fonts as
input. Subseqeuntly the completed suffix CDAWG is traversed in order to count
Copy link
Member

Choose a reason for hiding this comment

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

Subseqeuntly -> Subsequently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed that. I'm ignoring the Codacy warnings for now, as I want to keep this code reasonably close to the tx code. If I fix either, I want to fix both. (And yes, ultimately, I'd like to just have one shared library for this, but at least this is a step in that direction.)

@cjchapman cjchapman force-pushed the cjc-port-tx-subroutinizer-to-makeotf branch from 854321b to e03220e Compare August 5, 2019 22:12
@miguelsousa miguelsousa requested a review from blueshade7 August 6, 2019 00:30
blueshade7
blueshade7 reviewed Aug 6, 2019
View reviewed changes
Copy link
Contributor

@blueshade7 blueshade7 left a comment

Choose a reason for hiding this comment

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

By comparing subr.c to cffwrite_subr.c, I realized that CFF2 changes are not included in subr.c. Is it because makeotf needs not to support CFF2 maybe at least for now? Other than that, it looks ok.

@cjchapman
Copy link
Contributor Author

@blueshade7 That's correct. We use makeotf only for CFF. We use buildcff2vf for CFF2.

@cjchapman cjchapman requested a review from blueshade7 August 6, 2019 20:24
blueshade7
blueshade7 approved these changes Aug 6, 2019
View reviewed changes
Copy link
Contributor

@blueshade7 blueshade7 left a comment

Choose a reason for hiding this comment

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

then good

@cjchapman cjchapman merged commit 661a343 into develop Aug 6, 2019
@cjchapman cjchapman deleted the cjc-port-tx-subroutinizer-to-makeotf branch August 6, 2019 21:06
@cjchapman cjchapman mentioned this pull request Aug 6, 2019
Closed
@madig madig mentioned this pull request Aug 9, 2019
Closed
@josh-hadley josh-hadley mentioned this pull request Mar 26, 2020
Closed
@cjchapman cjchapman mentioned this pull request Jun 2, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miguelsousa miguelsousa miguelsousa left review comments

@blueshade7 blueshade7 blueshade7 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[makeotfexe] Replace the subroutinizer with the one from tx
3 participants
@cjchapman @miguelsousa @blueshade7