Use fast hash function on key for better distribution.
Direct compare glyph in eql to avoid Packed.from() if not neccessary.
16% -> 6.4% reduction during profiling runs.
I noticed that there was an off-by-one error in cell height adjustment
when the number of pixels to add/subtract is odd. The metrics measured
from the top would be shifted by one less than they should, so, for
example, the underline position would move one pixel closer to the
baseline than it had been (or one pixel further away if subtracting).
Also noticed that the overline position was missing here, so added that.
Use fast hash function on key for better distribution.
Direct compare glyph in eql to avoid Packed.from() if not neccessary.
16% -> 6.4% reduction during profiling runs.
Fixes#8483, fixes#2991
With this change, `font.face.getMetrics` is now infallible, and real
bitmap fonts are properly handled and can be configured as the primary
font (or used as fallbacks), as long as the backend (FreeType or
CoreText) knows how to interpret them, since we now fall back on the
backend for any metrics we can't extract from sfnt tables (which means
we don't need any to be present in the first place).
Also, doing this uncovered a double-free issue in our FreeType
`renderGlyph` code, which thankfully wasn't too hard to track down and
fix.
> [!NOTE]
> We should vendor a true bitmap font in each of the native formats
supported by each backend and add tests for the metrics being computed
right and the glyphs being rendered correctly. Idk if we wanna block
this PR on that or not.
This caused a malloc fault due to a double free when deiniting the face
if we didn't do this, which makes sense- making it possible to actually
load bitmap fonts revealed this bug which was sitting dormant the whole
time before that ever since I refactored the freetype rasterization.
Before we had a bad day if we tried to get the metrics of a bitmap font,
which would happen if we ever used one as fallback because we started
doing it for all fonts when we added fallback font scaling. This is a
pretty easy fix and finally allows users to configure true bitmap fonts
as their primary font as long as FreeType/CoreText can handle it.
This contains the various changes necessary to get the full unit test
suite passing Valgrind, and configures CI to run this.
I disabled relatively few (less than 10) tests under Valgrind because
they're way too slow: all `verifyIntegrity` tests, because those run
anyways in debug and check their own memory health, a font test that
fills out font map, and the sprite render test. Everything else runs
as-is.
I found a number of issues, most were in the tests themselves. A couple
in actual code. A funny one was some undefined memory on tabstop resize
if you exceed the default number of tabstops. I don't know any real
world program that ever even did that (memory issue aside), and that
whole file hasn't been touched since 2022, so that was funny.
No memory leaks in actual code, but a number of leaks in tests. All
resolved.
I think we're still missing some reports because of the Zig bug:
https://github.com/ziglang/zig/issues/19148 so I'm gong to audit our
codebase after this and look for cases of that.
When constraints increased or decreased the size significantly, the
fractional position was getting messed up by the scale. This change
separates that out so that it applies correctly.
This release contains performance and memory use improvements.
Some of the sprite font test renders had to be updated due to very minor
differences in the anti-aliasing, since the default anti-aliasing method
in z2d has been changed to MSAA rather than SSAA.
Similar tests should be added throughout the codebase for any function
that's supposed to gracefully handle OOM conditions. This one was added
because grow previously had a use-after-free bug under OOM, which this
would have caught.
Reordered to form a more logical sequence of steps, cleaned up and
clarified comments, fixed invalid `appendAssumeCapacity` call which
erroneously passed `alloc`, so this compiles again.
Grow needs to allocate and might fail midway. It tries to handle this
using "undo" pattern, and restoring old state on error. But this is
exactly what steps into UAF, as, on error, both errdefer and defer are
run, and the old data is freed.
Instead, use a more robust "reservation" pattern, where we first
fallibly resrve all the resources we need, without applying any changes,
and than do the actual change once we are sure that cannot fail.
I've cleaned up the code we use for scaling and positioning glyphs for
raster, under both CoreText and FreeType. Before we had some
imprecision, and under CoreText we were sometimes stretching glyphs in
unseemly ways. These changes make it so that our constraints can
position and size glyphs *exactly* and we don't have any chopped-off
row/column issues for CoreText. With this, PowerLine Extra symbols now
always align *perfectly* with the cell height:
||Before|After|
|-:|-|-|
|**CoreText**|<img width="105" height="245" alt="image"
src="https://github.com/user-attachments/assets/d3c1b1cb-a798-4e18-a0e0-59551893369c"
/>|<img width="106" height="246" alt="image"
src="https://github.com/user-attachments/assets/dac10c49-9ec1-4f4f-8825-a5e8c2fd3402"
/>|
|**FreeType**|<img width="105" height="245" alt="image"
src="https://github.com/user-attachments/assets/160e1e35-4a3c-42d0-9042-215301e636a1"
/>|<img width="106" height="245" alt="image"
src="https://github.com/user-attachments/assets/89bf1538-7271-4baf-88c0-51ebc4d360df"
/>|
The other changes are mainly just cleanup stuff, though one of the
changes makes it so that we do once again properly apply constraints to
symbols from the dingbats block (it was a regression, noted in #7955,
that we stopped doing that).
### Future work
This has been a problem since we introduced the custom constraints, but
I noticed it while preparing the before/after images: the left-edge PLE
symbols (meant to connect to a full block on the right) expand out to
the *right*, so if they're followed immediately by another character
than they actually get squished and don't match the right-edge symbols:
<img width="75" height="114" alt="image"
src="https://github.com/user-attachments/assets/1420b9a5-9950-4210-9934-8ef7cd7a1e19"
/>
I have a WIP change to move constraint logic to the shapers, and at that
point we can maybe do something to allow the constraint to grow in to
whitespace on the left side instead of on the right side.
We now also have absolute perfect control over the raster position under
FreeType as well. This means that, for example, powerline extended chars
are appropriately clamped to the cell edges at all sizes.
This should be purely an improvement over what we had before, and now it
also matches what we do for CoreText.
Freetype encodes some font names internally in formats other than UTF-8.
This only affects debug logs but it was annoying me so I fixed it. There
may be other encodings that might need to be dealt with but I took care
of the one that I ran across.
The monochrome hinter is very aggressive but makes text actually look
tolerable when rendered in monochrome. If for some god forsaken reason
we get complaints about this, that someone wanted improperly hinted mono
glyphs, we can introduce additonal configuration; but for now, this is
just a straight improvement.
The old method was nice, but had an issue that's intractible without
significant reworking in how we do shaping: combining glyphs need to
position relative to the glyph they're combining with, but if we re-
center that glyph, it will be off by some amount.
Apologies to Apple, the previous comments in this section of the code
were not correct-- `shouldSubpixelQuantizeFonts` does pretty much what
the minimal documentation for it says it does, it simply quantizes the
position of the glyph and nothing more. Various bugs when testing while
writing the old code that led me to include those comments made me not
realize that the positioning is actually a lot simpler than it seems.
With this version of the positioning there are never any cut-off rows or
columns of pixels on the edges of anything and everything scales as it
should... I hope. I checked pretty thoroughly this time and I'm like 99%
sure this is correct in all cases.
Closes#5372
Discretionary ligatures (denoted by the OpenType feature tag `dlig`) are
sometimes used by programming fonts (e.g. Iosevka) to provide more
"complex" and uncommon ligatures that may be useful in a programming
context. Unfortunately, this has some nasty side effects with certain
Japanese fallback fonts (#5372) due to perhaps a misaligned understanding
of the OpenType spec[^spec].
The spec details that `dlig` ligatures should only be used to contract
sequences of glyphs together into one glyph, and that it should be used
only for "special effect", **at the user's preference** (emphasis mine).
Indeed, it also suggests that:
> UI suggestion: This feature should be off by default.
All of this, combined with the fact that historical, nowadays unused and
even unintelligible Kanji ligatures are explicitly included as examples
of discretionary ligatures, shows that in the Japanese context at least
that the "level of discretion" is significantly higher than what is found
in programming fonts, where it is more understood to be "opinionated and
uncommon", rather than "obsolete and unreadable".
Furthermore, it appears that a lot of common programming fonts don't even
make use of the `dlig` feature — JetBrains Mono, FiraCode and MonoLisa
lack a `dlig` feature altogether, while Inconsolata seems to only use it
for ligatures that are more commonly found in `liga` or `calt`, such as
the `->` ligature. To a lot of people, then, this change would literally
alter nothing.
Therefore, it's my opinion that we should disable `dlig` by default.
It's arguably not being used correctly in the programming font space
(or at least not in a way that's coherent with other fonts), and it only
provides a marginal benefit while potentially rendering entire sentences
in Japanese (and possibly other languages) unreadable out of the box.
If someone upgrades to tip or 1.2 and then asks "why aren't the ligatures
working anymore", then at least they can always just turn on `dlig` by
themselves.
[^spec]: https://learn.microsoft.com/en-us/typography/opentype/spec/features_ae#tag-dlig
This adds functionality for choosing different normalization metrics for
each fallback font. It's not exposed as a config option, but could be in
the future, which would probably go a long way towards addressing
concerns like #7929.
The currently available reference metrics are, in priority order:
`ic_width, ex_height, cap_height, line_height, em_size`. The default
value is `ic_width`.
By priority order, I mean that if the chosen metric is not defined in
the fallback font, we move to the next metric in the list---we don't
normalize by an estimated metric from the fallback font (however, we're
happy to use an estimated metric from the primary font, that's how
`ic_width` normalization between CJK and Latin fonts work). This extends
the pattern that was used between `ic_width` and `ex_height` in the
existing hardcoded rule. `line_height` is always defined, so the buck
stops there.
What motivated me to implement this was the fact that, with the existing
hardcoded rule, the embedded symbols-only Nerd Font was always scaled up
by a factor of 1.2, which turned out to be an important reason why it's
been difficult to make icon scaling work to everyone's satisfaction.
Accordingly, the symbols-only font is the first to take advantage of the
new functionality. If this PR is merged, #7917 is no longer needed. (To
limit the scope of this PR, it only includes the minimal changes to let
icon scaling take advantage of this functionality. I may submit a
follow-up PR with some further icon scaling improvement enabled by
this.)
I changed my mind, this is a pretty small change and relevant to the
intent of the PR. This brings the appearance of the embedded symbols
much closer to patched fonts.
With this, the sizes of most symbols are nearly identical to a patched
font, the only big difference is positioning (and TBH I think we do a
better job positioning than the patcher does, since we have knowledge
about the cell size).
Move size adjustment logic out of `Entry`, I understand the impulse to
put it there but it results in passing a lot of stuff around which isn't
great.
Rework `add(...)` in to `add(...)` and `addDeferred(...)`, faces are
passed directly now instead of passing an entry, and an options struct
is used instead of positional arguments for things like style, fallback,
and size adjustment.
Change size adjustment test back to a half pixel tolerance instead of 5%
because the previous commit (allowing fractional pixel sizes) fixed the
root cause of large differences.
This makes the `new_window` action properly inherit properties from the
parent surface that initiated the action. Today, that is only the pwd
and font size.
A variety of naming, commenting, and formatting improvements + a few
explicit error sets. This commit has no functional changes, though it
does remove a couple functions that didn't really need to exist.
