Skip to content
This repository has been archived by the owner on Apr 7, 2024. It is now read-only.

Place mark feature before mkmk. Fixes #377 #379

Merged
merged 1 commit into from
May 31, 2021
Merged

Conversation

simoncozens
Copy link
Contributor

See discussion in #377.

@marekjez86 marekjez86 merged commit cfe13f8 into main May 31, 2021
@marekjez86 marekjez86 deleted the fix-looped-manual-mkmk branch May 31, 2021 02:17
@marekjez86
Copy link
Contributor

@simoncozens @ohbendy : I'm not sure the marks are placed properly now

here's the sample with the newly built font (I haven't published it yet). Ben's old submission build (when he sent us the sources we got also binaries) is reddish and the newly updated source font is greenish (see the mark positioning -- IMHO, it appears to be off by 1, but I might be wrong). Tested it for NotoLoopedLao but all of the Looped fonts have the same property.

Screen Shot 2021-05-30 at 9 55 16 PM

@simoncozens
Copy link
Contributor Author

Ok. I’m happy to look at this some more, but it highlights the need for the kind of shaping regression test framework that I recently added to fontbakery. (See https://simoncozens.github.io/tdd-for-otl/ )

Could you split off these sources to their own repos? That way I can add specific test suites for them.

@ohbendy
Copy link
Contributor

ohbendy commented May 31, 2021

Yep unfortunately that's broken in a different way, glad you didn't publish yet!

Agree there should be a testing stage. IMO the best person to do the checking would be the designers, because different people will choose to handle things in different ways, they will know the exact checks to run on each set of fonts, and will know what the desired result is. We can also check in different environments (e.g InDesign, Word, browsers) as unexpected things can occur.

@moyogo
Copy link
Contributor

moyogo commented May 31, 2021

I'll create notofonts/Noto-Lao and notofonts/Noto-Thai.
Looking at the Noto Looped Thai now, there are some glyphs that are not in the UI font and the PAL and SAN language systems are missing from UI as well.

@ohbendy
Copy link
Contributor

ohbendy commented May 31, 2021

That is correct. Marek advised that the minority languages won't appear in a UI environment.

@moyogo
Copy link
Contributor

moyogo commented May 31, 2021

OK. That makes sense. Some lookups were still there.
I merged the sources files. I guess we can drop thoses language systems when slicing the UI fonts.

@ohbendy
Copy link
Contributor

ohbendy commented May 31, 2021

Sorry for not removing those lookups, thanks for checking.

Not sure what you've merged; the document fonts and UI fonts have different outlines and behaviour.

@moyogo
Copy link
Contributor

moyogo commented Jun 1, 2021

By merging I mean put the masters of NotoLoopedLaoUI in NotoLoopedLao with an axis for the UI variation.

The behaviour differences are:

  • the master specific mark positioning with the lookup LaoContexualKerning,
  • a few missing glyphs in one or the other and the corresponding lookups.

For the master specific mark, an alternative solution would be to use a contextual composite glyph with the expected kerning. With the merged sources, lookup LaoContexualKerning would only work with the feature syntax update for font variation Simon has worked on, and the contextual composite glyph would work with current feature syntax.

Otherwise, the fix for the mark and mkmk features is to have the kern feature before the mark and mkmk feature.

I've already done most of this in the ui-as-axis branches on https://github.com/notofonts/Noto-Thai and https://github.com/notofonts/Noto-Lao.
I'll add tests to see what I may have broken in the process.

@ohbendy
Copy link
Contributor

ohbendy commented Jun 1, 2021

Ok. Please let me test the compiled fonts before publishing.

@simoncozens
Copy link
Contributor Author

The point of splitting to a separate repo is that you and I can design a test suite together, and then we can gate publishing on that test suite passing.

@ohbendy
Copy link
Contributor

ohbendy commented Jun 8, 2021

put the masters of NotoLoopedLaoUI in NotoLoopedLao with an axis for the UI variation

The document and UI fonts also have different values for family alignment zones, WinAscent/WinDescent, underline and strikethrough thickness and position.

Some lookups were still there.

I just checked all the sources I'd sent and couldn't find anything that shouldn't have been there. Glyphs would have generated an error if there were lookups for nonexistent glyphs.

you and I can design a test suite together

Sorry, no. Please just let me test the fonts before publishing. I have all the testing documents I need already, they're not part of the deliverables we agreed. This work was completed in December. I can't work for free on designing more tools for testing.

@simoncozens
Copy link
Contributor Author

Please just let me test the fonts before publishing. I have all the testing documents I need already, they're not part of the deliverables we agreed.

Understood. At the very least, I want to add regression tests to ensure that the original bug you reported (#377) are fixed and that whatever is going on in Marek's example above is also fixed, and to ensure that these problems don't recur in future. But I can write those myself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants