pf4: Unslash tabular zeros via subset & range #18145
Conversation
|
The original subset and modified fonts, in otf, woff2, and base64 (in the woff2 directory): |
|
You can see it working in normal weights... but it looks like I messed up the medium weight:
I'll fix this tomorrow. |
|
We also don't use tabular numbers everywhere that we should. Look at disks in this screenshot (the numbers on the usage bars, before "free" are all proportional):
|
There was a problem hiding this comment.
Whoa @garrett that's cool.
Small question. I see in the pixel tests, even zeros without the slash changed enough to make pixel tests fail. Is that expected?
Yes, see the pixel diff @ #18125 (comment). I didn't get the hinting exactly right, as I had to recreate the inner part without the slash. It's all sub-pixel and nobody would notice except the pixel tests. After a bit of digging around: The zeros for bold in this PR are actually using bold, not semi-bold. PatternFly is using different weight numbers than I had expected. Fixing now. (They purposefully push the semibold as bold and bold as heavy... and don't use the heavy. I guess because they thought the bold was a bit too much.) |
garrett
force-pushed
the
fonts-tabular-deslash
branch
from
c6864d3 to
faa442a
Compare
|
I've dropped the unused weights and recalibrated the weight values (as PF uses nonstandard weights to make "bold" show a medium weight instead, as the bold is pretty heavy for a UI). The adjusted weights are the screenshot was off last night. It has been fixed in the PR. |
|
@garrett when you slide in this screenshot https://cockpit-logs.us-east-1.linodeobjects.com/pull-18145-20230110-084945-faa442a8-fedora-36/pixeldiff.html#TestStorageBasic-testBasic-partition |
|
@KKoukiou: No, they're not smaller width. I made sure to preserve the widths. Perhaps tabular isn't being respected and it's dropping in the normal 0, which is a different width? That's the only explanation I can see for differing widths. It seemed to work perfectly in the first try. The pixel diff I made @ #18125 (comment) shows that my original proof of concept had the same exact widths. I did try to optimize the default one a bit more to get the filesize down... I might've messed up the metadata when optimizing. If that's the case, it's so minor that perhaps we should just standardize the 0 to the tabular width regardless. This would have the side effect of making the file size (and therefore base64 string) smaller too, since it would be just 1 glyph instead of 2. Or I could try to make sure I don't delete metadata mapping. Also, I think this is just wrong for hashes: Hashes should use probably monospace, not tabular numbers — that is, they should be fixed width and align. (And therefore, they'd have the 0 with the slash, as I didn't touch monospace.) But something like that should change outside this PR. |
|
The new zero width is obviously bigger than the tabular one. it's visible in most of the screenshots I am not trying to be picky, but this cancels the point of tabular - fixed width nums |
garrett
force-pushed
the
fonts-tabular-deslash
branch
from
faa442a to
6b3a103
Compare
|
Original was correct; I did mess up trying to optimize the default. I swapped out the normal one to the original test and it works now. (I didn't try to optimize the variants, so those should be fine.) Slash from main:
This PR (now, fixed):
Diff:
|
KKoukiou
added
the
no-test
KKoukiou
force-pushed
the
fonts-tabular-deslash
branch
from
6b3a103 to
5868919
Compare
Partially addresses cockpit-project#18125 This is the least intrusive change; fonts are embedded as to: - Not require buildsystem changes to ship font files - Not inflate number of connections from browser to Cockpit - Be easily portable to cockpit-machines, cockpit-podman, etc. It's messy when viewing the stylesheet, so they're at the bottom of the file. Additionally, subsetting and mapping these custom versions of the fonts are much smaller in size than forking the font and using our own custom version, which would be worse. Note: PatternFly, and therefore Cockpit, uses non-standard weights so medium (semi-bold) becomes bold. Additionally, only regular and medium weights are used, so we're only including these two weights and their italic variants in the override. (Leaving out light, bold, and heavy weights and their italic variants).
KKoukiou
force-pushed
the
fonts-tabular-deslash
branch
from
5868919 to
24a8759
Compare
Partially addresses #18125
This is the least intrusive change; fonts are embedded as to:
It's messy when viewing the stylesheet, so they're at the bottom of the file.
Additionally, subsetting and mapping these custom versions of the fonts are much smaller in size than forking the font and using our own custom version, which would be worse.