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

[test] Add generated tests for emoji clusters #3087

Merged
merged 2 commits into from
Jul 29, 2021
Merged

Conversation

khaledhosny
Copy link
Collaborator

@khaledhosny khaledhosny commented Jul 28, 2021

Fixes #3017

Uses AdobeBlank2.ttf from:
https://github.com/adobe-fonts/adobe-blank-2

instead of a dummy empty font so that everything maps to GID 1 and control code points are kept instead of being dropped because there is
not space glyph (otherwise we’d need to identify control code points somehow when generating the expectations).

Fixes #3017

Uses AdobeBlank2.ttf from:

  https://github.com/adobe-fonts/adobe-blank-2

instead of a dummy empty font so that everything maps to GID 1 and
control code points are kept instead of being dropped because there is
not space glyph (otherwise we’d need to identify control code points
somehow when generating the expectations).
@behdad
Copy link
Member

behdad commented Jul 28, 2021

Thank you!!!

@behdad
Copy link
Member

behdad commented Jul 28, 2021

How much slower will it be if we don't chunk?

@khaledhosny
Copy link
Collaborator Author

How much slower will it be if we don't chunk?

About 4 times slower on my machine, but still below 1 second over all.

@khaledhosny
Copy link
Collaborator Author

khaledhosny commented Jul 28, 2021

(apparently batch mode makes things pretty fast, the standalone test that from @rsheeter repo took about 30 seconds.)

@behdad
Copy link
Member

behdad commented Jul 28, 2021

(apparently batch mode makes things pretty fast, the standalone test that from @rsheeter repo took about 30 seconds.)

Yes, I was wondering since we are using batch mode.

If it's still under 1s, I"m inclined to leave it unchunked.

And I'd be happy to speed batch mode up instead.

@khaledhosny
Copy link
Collaborator Author

To give exact numbers, it goes from 0.15s to 1.15s (it was 0.8s when I checked earlier), but yeah not worth it and easier to read failure messages should be worth it.

@behdad
Copy link
Member

behdad commented Jul 28, 2021

To give exact numbers, it goes from 0.15s to 1.15s (it was 0.8s when I checked earlier), but yeah not worth it and easier to read failure messages should be worth it.

I see. But yeah, do it the proper way and I'll take a stab at optimizing batch mode. I think currently it still reloads the face every time. Just not doing that would help.

@behdad behdad merged commit 9a7ff54 into main Jul 29, 2021
@behdad behdad deleted the emoji-cluster-tests branch July 29, 2021 00:25
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.

Not all emoji sequences recommended for general interchange (RGI) cluster
2 participants