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

Indic cleanup #3648

Merged
merged 38 commits into from
Jun 11, 2022
Merged

Indic cleanup #3648

merged 38 commits into from
Jun 11, 2022

Conversation

behdad
Copy link
Member

@behdad behdad commented Jun 9, 2022

This supersedes #3635 and moves all exception handling of indic/khmer/myanmar shapers to the indic table generator.

@behdad
Copy link
Member Author

behdad commented Jun 9, 2022

I've tested with harfbuzz-testing-wikipedia corpus that the Myanmar shaping against NotoSansMyanmar didn't change. I'll test Khmer and Devanagari and report as well.

@behdad
Copy link
Member Author

behdad commented Jun 9, 2022

I confirmed that Khmer and Devanagari corpus also render the same before/after this PR.

@behdad
Copy link
Member Author

behdad commented Jun 9, 2022

What still can be done is to export categories from Indic machine like the other machines, and use those in the Indic shaper instead of the OT_* categories.

@behdad
Copy link
Member Author

behdad commented Jun 9, 2022

What still can be done is to export categories from Indic machine like the other machines, and use those in the Indic shaper instead of the OT_* categories.

Done.

@behdad
Copy link
Member Author

behdad commented Jun 9, 2022

@dscorbett This is ready for review. Thanks.

src/gen-indic-table.py Outdated Show resolved Hide resolved
src/hb-ot-shaper-indic.cc Show resolved Hide resolved
src/hb-ot-shaper-indic.hh Outdated Show resolved Hide resolved
src/hb-ot-shaper-indic.hh Show resolved Hide resolved
src/hb-ot-shaper-indic.hh Outdated Show resolved Hide resolved
src/gen-indic-table.py Show resolved Hide resolved
src/gen-indic-table.py Outdated Show resolved Hide resolved
src/gen-indic-table.py Show resolved Hide resolved
src/gen-indic-table.py Outdated Show resolved Hide resolved
src/gen-indic-table.py Outdated Show resolved Hide resolved
@behdad
Copy link
Member Author

behdad commented Jun 10, 2022

Thanks @dscorbett for all the feedback. This PR is a refactoring of the code without any semantic changes. May I merge this and then address your comments in subsequent PRs?

@behdad
Copy link
Member Author

behdad commented Jun 10, 2022

Okay I'm starting to apply your requested changes here.

@behdad
Copy link
Member Author

behdad commented Jun 10, 2022

Okay. I think I've addressed all.

@behdad
Copy link
Member Author

behdad commented Jun 10, 2022

Okay. I think I've addressed all.

Other than the removing the duplication completely.

@behdad
Copy link
Member Author

behdad commented Jun 10, 2022

Let's merge this and continue separately.

@dscorbett
Copy link
Collaborator

Other than the Myanmar medials’ comments, this is ready to merge.

@behdad
Copy link
Member Author

behdad commented Jun 11, 2022

Thanks.

@behdad behdad merged commit b5bdb9f into main Jun 11, 2022
@behdad behdad deleted the indic-cleanup branch June 11, 2022 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants