-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Agu Display: Version 1.103 added #8487
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Updated |
81fe77a
to
e165274
Compare
This comment was marked as outdated.
This comment was marked as outdated.
e165274
to
d51351c
Compare
FontBakery reportfontbakery version: 0.13.0a5 Check results[19] AguDisplay[MORF].ttf💥 ERROR Familyname must be unique according to namecheck.fontdata.com
[code: namecheck-service] 🔥 FAIL METADATA.pb: Check URL on copyright string is the same as in repository_url field.
But: font copyright string has 'https://github.com/theseunbadejo/nsibidi-libre/blob/main/AUTHORS.txt' and OFL text has 'https://github.com/googlefonts/googlefonts-project-template' [code: mismatch]🔥 FAIL Shapes languages in all GF glyphsets.
|
ofl/agudisplay/AguDisplay[MORF].ttf | |
---|---|
Dehinted Size | 757.7kb |
Hinted Size | 757.7kb |
Increase | 24 bytes |
Change | 0.0 % |
ℹ️ INFO Font contains all required tables?
- ℹ️ INFO
This font contains the following optional tables:
- loca
- prep
- GPOS
- GSUB
- gasp
[code: optional-tables]
ℹ️ INFO Is the Grid-fitting and Scan-conversion Procedure ('gasp') table set to optimize rendering?
- ℹ️ INFO
These are the ppm ranges declared on the gasp table:
PPM <= 65535: flag = 0x0F - Use grid-fitting - Use grayscale rendering - Use gridfitting with ClearType symmetric smoothing - Use smoothing along multiple axes with ClearType®
[code: ranges][1] Family checks
Summary
💥 ERROR | ☠ FATAL | 🔥 FAIL | ⏩ SKIP | ℹ️ INFO | ✅ PASS | 🔎 DEBUG | |
---|---|---|---|---|---|---|---|
1 | 0 | 2 | 10 | 56 | 7 | 176 | 0 |
0% | 0% | 1% | 4% | 22% | 3% | 70% | 0% |
Note: The following loglevels were omitted in this report:
- SKIP
- PASS
- DEBUG
@n8willis I've restored the first PR comment information including the checklist, it is key to our process, please do not delete it.
|
Looking at the built binaries, the VF version appears to have "Uzo" as the subfamily name. I thought that had been sorted out previously. I may have thinking about the static instances. TTX
|
@m4rc1e Is that just an artifact of Uzo being at MORF=0? |
@vv-monsalve Were you intending to tag me on this?
Just for the record, I did not delete it. |
Bad tagging indeed! Thank you for the clarification. Fixed it now. |
As far as I can tell from the timestamps, the comment was "edited" at the same moment the first update push landed & the page refreshed. So maybe the update-as-someone-checks-checkboxes collides in transit with the page refresh. I don't know. It makes one wonder though. |
Uhm! You're probably right. IIRC, Emma reported it before on another PR. @m4rc1e seems the packager is still doing this. |
Some of these combinations it's reporting a problem with seem to come from this list of combos that don't have a Unicode codepoint. Like, right near the top, "Shaper didn't attach gravecomb to m". It's very trivial to see that m has the correct anchor on it. I don't see what the recommended fix is in that glyphsets issue. Is there something established here? |
Separately, on the subject of not changing the coverage required after the font was commissioned (and this is a commissioned project), we do have the GlyphsApp custom filter in the project-template repo, which was updated with GF African in September 2023. |
Add
to the feature prefixes. That way ufo2ft will make the appropriate mark-to-base rules for the script. This will help, but other problems really do need new anchors (e.g. verticallineabovecomb, Eopen/eopen, schwa/Schwa, etc. have no anchors). |
(Just for future reference, I do understand how shaping functions. I've even written about it at times.) Sorry that you weren't able to join the call where we brought this up. The font doesn't have any non-standard features; it has been using the auto-generated That all having been said, the question I could really use some clarification on is what's supposed to be done about this list:
of things. As I recall, Eben said that it was a |
I've explained what you need to do to fix that list. Because you understand shaping, you will know why this helps. |
So, a TTX dump of GPOS indicates that those get added already (which does seem like it would be the expected behavior).
Well, some of that might happen at the same time, but as I understand where the conversation ended up, then upstream isn't going to be asked to add things beyond the original contracted feature set. It's certainly great that the glyphsets have received a serious cleanup and refactoring of the tools, but the goalposts of the contract shouldn't be moving at this stage. |
Language systems get added to the font binary, yes. But that's not what I said. I said, add them to the feature prefixes. Please believe that there's a reason I suggested doing that. I cloned the Agu Display repo as at the commit above and built the font. I saw that mark attachment was not working for the combinations above. Why not, I wondered? So I looked into it. I did some debugging. I used the Then I did some more debugging - I loaded the font into FontGoggles, and changed the script from Automatic to So I tried adding the language systems into the feature prefixes, checked the generated feature file and the font in FontGoggles, and all was well. Debugging: I can highly recommend it. |
Well, perhaps fortuitous that this landed while I was in the middle of writing a reply, not before:
You make pointed comments like this — and like this one:
directed my way, and in mid-thread context I'm not clear if this's intended to be humorous or what, but it really just sounds like you want to have an argument, and I'm not interested in that. I wrote my comment in reply to Viviana's comment, which was about general feature code, because it started out with high-level, overview stuff. Viviana and I have never actually met, and we really do not know each other at all apart from these comments. So I was very literally explaining in my reply to her that I was already familiar with the basics and that it wasn't necessary to start there. And then followed up by relaying what we had figured out so far in additional detail. Your own comment I replied to below that, in its own message. For the record on that one, upstream did already include that prefix insertion several days ago, and it didn't solve it. So I don't get what these remarks are supposed to communicate. But I'm not interested in the argument at all, so if that's the intent please consider me out. OTOH, if it's meant to be humor, then it is not coming across. If neither, I suppose it's still not coming across. |
@vv-monsalve The checklist in the PR template says that this field is for "projects that have a primary non-Latin based language support target". Is that not the case (or not the case anymore)? |
I've pulled together time for us to discuss this project tomorrow. Please pause discussion in this thread until we have a chance to speak together. I will post the next message in this thread with the plan of record to land this in main once we have an opportunity to speak with each other. |
Updated |
d51351c
to
cd57731
Compare
FontBakery reportfontbakery version: 0.13.0a5 Check results[17] AguDisplay[MORF].ttf🔥 FAIL Shapes languages in all GF glyphsets.
|
ofl/agudisplay/AguDisplay[MORF].ttf | |
---|---|
Dehinted Size | 755.0kb |
Hinted Size | 755.0kb |
Increase | 24 bytes |
Change | 0.0 % |
ℹ️ INFO Font contains all required tables?
- ℹ️ INFO
This font contains the following optional tables:
- loca
- prep
- GPOS
- GSUB
- gasp
[code: optional-tables]
ℹ️ INFO Is the Grid-fitting and Scan-conversion Procedure ('gasp') table set to optimize rendering?
- ℹ️ INFO
These are the ppm ranges declared on the gasp table:
PPM <= 65535: flag = 0x0F - Use grid-fitting - Use grayscale rendering - Use gridfitting with ClearType symmetric smoothing - Use smoothing along multiple axes with ClearType®
[code: ranges][1] Family checks
Summary
💥 ERROR | ☠ FATAL | 🔥 FAIL | ⏩ SKIP | ℹ️ INFO | ✅ PASS | 🔎 DEBUG | |
---|---|---|---|---|---|---|---|
0 | 0 | 1 | 10 | 56 | 7 | 178 | 0 |
0% | 0% | 0% | 4% | 22% | 3% | 71% | 0% |
Note: The following loglevels were omitted in this report:
- SKIP
- PASS
- DEBUG
Nate will publish the list of unsupported African languages as defined by a local shaperglot CLI reporting run here. Then we will decide on next steps for the v1.0 release. |
The attached file is the output summary from upstream's 1.03 release (https://github.com/theseunbadejo/Agu-Display/releases/tag/1.03 -- as of commit cd57731 above) . |
Hi @n8willis. No, it's not needed. After reviewing previous PRs, I got briefly confused last week, but for this font, it is not required. Please ignore that comment. I'll delete the bullet for added clarity. |
@n8willis
Thanks! |
shaperglot lang checks across those four languages using the latest PyPI release of the shaperglot tool. It looks like we only need to resolve the upper case Eng default +/- Bambara
Igbo
Ditto my above question for Bambara Ganda
Dyula
|
Sorry; I didn't see this until just now.
That's correct, as far as I can determine. I have started a PR on the upstream repo that should have taken care of the other issue, with soft-dotted i/j in I don't currently have an ETA on that, but there is already a base glyph that mostly shares construction with the other Eng skeleton. So it's likely just a matter of ensuring that the other Eng form works visually with the 3 variation masters and the rotating/substituting decorations in the alternate layers.
The existing Eng form was Sami; the new one to be added is the African form. That will become the default; the plan is to make use of Glyphs's auto-feature-generation func. Assuming that works as expected, moving the old Eng to Eng.loclNSM and adding the new African Eng as the default is (I believe) all that's required. But even if the auto-feature doesn't work as advertised, this way there's just one Plus, folks have said that the full set of African languages that would need Although on this:
as I understand it, the check assumes Sami is the default and just looks for "does the glyph change if the language is set == Bambara". So the check will not succeed for fonts where the African form is the default, and that just needs to be noted and acknowledged when examining the report. |
Noted. It sounds like we need to wait on this so that we pick up the missing languages that I noted above in our v1.0 release. Do we have an estimate on how long it will take to develop this glyph? |
This is my understanding as well. I think that we can satisfy the v1.0 release requirements if we add the African UC Eng as the default design in the encoded position. That is all that we need. If we can support the non-African localized UC Eng design through OT |
@n8willis please convert out of Draft PR status when #8487 (comment) is complete and ping the on call Maintainer (Marc next week) so that he knows that your PR is ready for review. Thanks! |
Taken from the upstream repo https://github.com/theseunbadejo/Agu-Display at commit theseunbadejo/Agu-Display@17a7ce9.
Updated |
cd57731
to
6913b39
Compare
FontBakery reportfontbakery version: 0.13.0a5 Check results[18] AguDisplay[MORF].ttf🔥 FAIL Shapes languages in all GF glyphsets.
|
ofl/agudisplay/AguDisplay[MORF].ttf | |
---|---|
Dehinted Size | 761.4kb |
Hinted Size | 761.5kb |
Increase | 24 bytes |
Change | 0.0 % |
ℹ️ INFO Font contains all required tables?
- ℹ️ INFO
This font contains the following optional tables:
- loca
- prep
- GPOS
- GSUB
- gasp
[code: optional-tables]
ℹ️ INFO Is the Grid-fitting and Scan-conversion Procedure ('gasp') table set to optimize rendering?
- ℹ️ INFO
These are the ppm ranges declared on the gasp table:
PPM <= 65535: flag = 0x0F - Use grid-fitting - Use grayscale rendering - Use gridfitting with ClearType symmetric smoothing - Use smoothing along multiple axes with ClearType®
[code: ranges][1] Family checks
Summary
💥 ERROR | ☠ FATAL | 🔥 FAIL | ⏩ SKIP | ℹ️ INFO | ✅ PASS | 🔎 DEBUG | |
---|---|---|---|---|---|---|---|
0 | 0 | 1 | 11 | 56 | 7 | 177 | 0 |
0% | 0% | 0% | 4% | 22% | 3% | 70% | 0% |
Note: The following loglevels were omitted in this report:
- SKIP
- PASS
- DEBUG
The Eng has been updated, with the African construction as the default and the Sami construction activated in @m4rc1e the designer still has some updated Article content he wants to deliver (it's a bit boilerplate now). Same with the Bio form. Should I hold off on un-drafting the state of this until that's pushed? |
Eng thing looks good, I wonder how we can fix the FB check. Article and Bio data should not delay this push to meet the EoY goal, we can update those in January. |
I could be mistaken, but it seems like checking for
Great; thanks. On the move now! |
No description provided.