Closed thewilkybarkid closed 3 years ago
That's a great find, and I agree with your solution suggestion (at least for now). We should be able to be smarter about it if we also trace font-variant-numeric
, but that requires more work. Probably better to sidestep that until we get around to doing axis trimming for variable fonts.
Would you mind sharing a reduced test case? Then we can add it to our image-based regression test suite to make sure we don't mess it up again 😎
Something like
<!doctype html>
<html>
<head>
<style>
@font-face {
font-family: IBMPlexSans;
src: url(IBMPlexSans-Regular.woff) format("woff");
}
/**
* CSS for IBM Plex Sans
* Generated by Wakamai Fondue - https://wakamaifondue.com
* by Roel Nieskens/PixelAmbacht - https://pixelambacht.nl
*/
/* Set custom properties for each layout feature */
:root {
--ibm-plex-sans-aalt: "aalt" off;
--ibm-plex-sans-dnom: "dnom" off;
--ibm-plex-sans-frac: "frac" off;
--ibm-plex-sans-numr: "numr" off;
--ibm-plex-sans-ordn: "ordn" off;
--ibm-plex-sans-salt: "salt" off;
--ibm-plex-sans-sinf: "sinf" off;
--ibm-plex-sans-ss01: "ss01" off;
--ibm-plex-sans-ss02: "ss02" off;
--ibm-plex-sans-ss03: "ss03" off;
--ibm-plex-sans-ss04: "ss04" off;
--ibm-plex-sans-ss05: "ss05" off;
--ibm-plex-sans-subs: "subs" off;
--ibm-plex-sans-sups: "sups" off;
--ibm-plex-sans-zero: "zero" off;
}
/* If class is applied, update custom property and
apply modern font-variant-* when supported */
.ibm-plex-sans-aalt {
--ibm-plex-sans-aalt: "aalt" on;
}
.ibm-plex-sans-dnom {
--ibm-plex-sans-dnom: "dnom" on;
}
.ibm-plex-sans-frac {
--ibm-plex-sans-frac: "frac" on;
}
@supports (font-variant-numeric: diagonal-fractions) {
.ibm-plex-sans-frac {
--ibm-plex-sans-frac: "____";
font-variant-numeric: diagonal-fractions;
}
}
.ibm-plex-sans-numr {
--ibm-plex-sans-numr: "numr" on;
}
.ibm-plex-sans-ordn {
--ibm-plex-sans-ordn: "ordn" on;
}
@supports (font-variant-numeric: ordinal) {
.ibm-plex-sans-ordn {
--ibm-plex-sans-ordn: "____";
font-variant-numeric: ordinal;
}
}
.ibm-plex-sans-salt {
--ibm-plex-sans-salt: "salt" on;
}
.ibm-plex-sans-sinf {
--ibm-plex-sans-sinf: "sinf" on;
}
.ibm-plex-sans-ss01 {
--ibm-plex-sans-ss01: "ss01" on;
}
.ibm-plex-sans-ss02 {
--ibm-plex-sans-ss02: "ss02" on;
}
.ibm-plex-sans-ss03 {
--ibm-plex-sans-ss03: "ss03" on;
}
.ibm-plex-sans-ss04 {
--ibm-plex-sans-ss04: "ss04" on;
}
.ibm-plex-sans-ss05 {
--ibm-plex-sans-ss05: "ss05" on;
}
.ibm-plex-sans-subs {
--ibm-plex-sans-subs: "subs" on;
}
@supports (font-variant-position: sub) {
.ibm-plex-sans-subs {
--ibm-plex-sans-subs: "____";
font-variant-position: sub;
}
}
.ibm-plex-sans-sups {
--ibm-plex-sans-sups: "sups" on;
}
@supports (font-variant-position: super) {
.ibm-plex-sans-sups {
--ibm-plex-sans-sups: "____";
font-variant-position: super;
}
}
.ibm-plex-sans-zero {
--ibm-plex-sans-zero: "zero" on;
}
@supports (font-variant-numeric: slashed-zero) {
.ibm-plex-sans-zero {
--ibm-plex-sans-zero: "____";
font-variant-numeric: slashed-zero;
}
}
/* Apply current state of all custom properties
whenever a class is being applied */
.ibm-plex-sans-aalt,
.ibm-plex-sans-dnom,
.ibm-plex-sans-frac,
.ibm-plex-sans-numr,
.ibm-plex-sans-ordn,
.ibm-plex-sans-salt,
.ibm-plex-sans-sinf,
.ibm-plex-sans-ss01,
.ibm-plex-sans-ss02,
.ibm-plex-sans-ss03,
.ibm-plex-sans-ss04,
.ibm-plex-sans-ss05,
.ibm-plex-sans-subs,
.ibm-plex-sans-sups,
.ibm-plex-sans-zero {
font-feature-settings: var(--ibm-plex-sans-aalt), var(--ibm-plex-sans-dnom),
var(--ibm-plex-sans-frac), var(--ibm-plex-sans-numr), var(--ibm-plex-sans-ordn),
var(--ibm-plex-sans-salt), var(--ibm-plex-sans-sinf), var(--ibm-plex-sans-ss01),
var(--ibm-plex-sans-ss02), var(--ibm-plex-sans-ss03), var(--ibm-plex-sans-ss04),
var(--ibm-plex-sans-ss05), var(--ibm-plex-sans-subs), var(--ibm-plex-sans-sups),
var(--ibm-plex-sans-zero);
}
code {
font-family: IBMPlexSans, monospace;
font-size: 400%;
}
</style>
</head>
<body>
<code class="ibm-plex-sans-aalt">
a á ă â ä ạ à ả ā ą å ǻ ã ắ ặ ằ ẳ ẵ ấ ậ ầ ẩ ẫ ğ ĝ ģ ġ ß α ά а ӓ ӑ ¯ ˙ ¨ ˝ ´ ` ˆ ˇ ˚ ΄ ̉
g g 0 0 ˜ ˜ ˜ ˘ ˘ ˘
</code>
<code class="ibm-plex-sans-dnom">
0 1 2 3 4 5 6 7 8 9
</code>
<code class="ibm-plex-sans-frac">
0 1 2 3 4 5 6 7 8 9 /
⁄ ₀ ₁ ₂ ₃ ₄ ₅ ₆ ₇ ₈ ₉
² ³ ¹ ⁰ ⁴ ⁵ ⁶ ⁷ ⁸ ⁹
⁄² ⁄³ ⁄¹ ⁄⁰ ⁄⁴ ⁄⁵ ⁄⁶ ⁄⁷ ⁄⁸ ⁄⁹ ₀² ₀³ ₀¹ ₀⁰ ₀⁴ ₀⁵ ₀⁶ ₀⁷ ₀⁸ ₀⁹ ₁² ₁³ ₁¹ ₁⁰ ₁⁴ ₁⁵ ₁⁶ ₁⁷ ₁⁸ ₁⁹ ₂² ₂³ ₂¹ ₂⁰ ₂⁴ ₂⁵ ₂⁶ ₂⁷ ₂⁸ ₂⁹ ₃² ₃³ ₃¹ ₃⁰ ₃⁴ ₃⁵ ₃⁶ ₃⁷ ₃⁸ ₃⁹ ₅² ₅³ ₅¹ ₅⁰ ₅⁴ ₅⁵ ₅⁶ ₅⁷ ₅⁸ ₅⁹ ₆² ₆³ ₆¹ ₆⁰ ₆⁴ ₆⁵ ₆⁶ ₆⁷ ₆⁸ ₆⁹ ₇² ₇³ ₇¹ ₇⁰ ₇⁴ ₇⁵ ₇⁶ ₇⁷ ₇⁸ ₇⁹ ₈² ₈³ ₈¹ ₈⁰ ₈⁴ ₈⁵ ₈⁶ ₈⁷ ₈⁸ ₈⁹ ₉² ₉³ ₉¹ ₉⁰ ₉⁴ ₉⁵ ₉⁶ ₉⁷ ₉⁸ ₉⁹
</code>
<code class="ibm-plex-sans-numr">
0 1 2 3 4 5 6 7 8 9
</code>
<code class="ibm-plex-sans-ordn">
a o
</code>
<code class="ibm-plex-sans-salt">
a g 0 á ă â ä ạ à ả ā ą å ǻ ã ắ ặ ằ ẳ ẵ ấ ậ ầ ẩ ẫ ğ ĝ ģ ġ ß α ά а ӓ ӑ
</code>
<code class="ibm-plex-sans-sinf">
0 1 2 3 4 5 6 7 8 9
</code>
<code class="ibm-plex-sans-ss01">
a á ă â ä ạ à ả ā ą å ǻ ã ắ ặ ằ ẳ ẵ ấ ậ ầ ẩ ẫ α ά а ӓ ӑ
</code>
<code class="ibm-plex-sans-ss02">
g ğ ĝ ģ ġ
</code>
<code class="ibm-plex-sans-ss03">
0
</code>
<code class="ibm-plex-sans-ss04">
0
</code>
<code class="ibm-plex-sans-ss05">
ß
</code>
<code class="ibm-plex-sans-subs">
0 1 2 3 4 5 6 7 8 9
</code>
<code class="ibm-plex-sans-sups">
0 1 2 3 4 5 6 7 8 9
</code>
<code class="ibm-plex-sans-zero">
0
</code>
</body>
</html>
might work? Covers all the types in the IBMPlexSans-Regular.woff
font in the existing unusedVariants
case. (So would represent a leave-all-features-on case.)
That's a great test case, thanks! It looks like not even --layout-features=*
is enough to get it passing, though: https://github.com/Munter/subfont/compare/issue141
All the reference images based tests that omit the fallback to the original font fail with an image diff like this:
More ideas for pyftsubset
switches that might fix it?
Fiddling around with --no-subset-tables+=...
doesn't seem to help either.
The fact that it fails when not including the original font as a fallback suggests to me that we're leaving out some glyphs that we shouldn't, not just features? In my experience browsers' decision to load the next font-family
in line is always based on whether the first loaded one(s) contained the glyphs it needed.
I've tried running it locally (node lib/cli.js testdata/referenceImages/fontVariant/index.html -o issue141 --relative-urls --no-fallbacks
) and it appears to work correctly....
Correctly in the sense that it produces the exact same rendering before and after? 🤔
Yep, here's the resulting font details:
ccmp
and liga
are missing from the list, but I guess there are no characters that use them?
Okay, that's super weird -- but good news I guess :)
Can you also get the test I added to pass? mocha -g 'omitFallbacks: true.*font-variant' --bail
I wonder if we're using the same version of fonttools?
ccmp
andliga
are missing from the list, but I guess there are no characters that use them?
IIRC liga
has been superseded by the GSUB
chunk, so I think it's just a legacy table that gets dropped.
The test was failing for me too; tried to run it outside to have a poke around only to find it work. 😅
There must be something different between the CLI and test runner...
Hmm, I can reproduce your finding -- that --layout-features=*
is enough to make the page render the same before/after even with --no-fallbacks
.
That's obviously great news! I'll try to find out what the difference is 😅
Argh, of course it was something silly: https://github.com/Munter/subfont/commit/e65e74ac3d13e7b0b4a92b1515855f523e10debc
Now it's down to just the harfbuzz-based subsetting failing, so we'll need to figure out how to fix it there as well.
... Looks a bit more involved, so I'll save that for another day.
I've released the fix in 5.4.0. Thanks for the great bug report and for helping out.
Ah, turns out the problem is already fixed in harfbuzz, but the latest version of harfbuzzjs is on an old version. For now I've made a fork of harfbuzzjs, updated to it, and released 5.4.1.
I've just noticed that a
font-variant-numeric: oldstyle-nums proportional-nums
property doesn't work as theonum
andpnum
features are removed (looks like the-layout-features
option isn't set so the defaults are applied).Trying to work out which exact features are used sounds like a rather... fun challenge, so maybe this could be an option for now?