This fixes a couple outstanding CoreText rendering issues with more complicated characters. Separated them into separate commits to make the changes easier to identify.
Rename the "preserve line spacing" setting to "use macOS calculation"
The setting was added to "preserve" the font's original line spacing intents, but after further investigation I cannot find why Apple's calculation for font line spacing to be so wide. Furthermore it's quite odd because the default line spacing is wide, but the only thing that setting does is by re-initializing the font under Core Text (instead of using NSFont like the default) using CTFont and somehow the issue is fixed. Inspecting font metrics (using ttx) also didn't seem to give any hints why the spacing are so wide (e.g. ascent / descent / line gap etc all look fine). A StackOverflow comment seems to suggest that Apple is simply adding a 1.2x scale to some fonts (https://stackoverflow.com/questions/5511830/how-does-line-spacing-work-in-core-text-and-why-is-it-different-from-nslayoutm) which seems to match the observation. My guess is that Apple looks at some fonts and think they are too aggressive in their font metrics design and "helpfully" introduces a 1.2x multiplier to space them out, where Core Text is lower level than AppKit and therefore does not have this auto-inflation.
By renaming the option it should make it clearer that this is a somewhat arbitrary distinction instead of it being an inherent property of the font.
Remove the ceil() call around fontDescent in Core Text renderer
I have looked and I do not believe there is a good reason to do this rounding up at all. I believe the motivation is to align the baseline with the pixel boundary, but I don't believe that is necessary, given that Core Text will properly perform the correct antialiasing for us. Most texts aren't directly aligned at the baseline anyway. Before, that ceil() option is a reason why sometimes fonts would feel pushed upwards, clipping emojis to the top, and creating a lopsided effect for fonts like Fira Code (h11), since we draw the boxes aligned to the line height at the bottom of the descender, meaning the ceil() has an effect of pushing the text up.
Fix composing characters renderered at an offset
MacVim previously didn't really render composing characters with multiple glyphs correctly. For simple ones like 'â' it would work fine because Core Text just generates one glyph for it, but for more complicated ones like 'x⃗' the renderer didn't properly query the advance of the glyphs to put them at the correct position. Refactor the logic to keep track of the current cell/glyphs and make sure to track the advances fo the glyphs so the composing chars' glyphs would be laid out properly on top of the cell.
We need to be careful with the tracking, because in Vim we force the text rendering to be monospaced, and so we maintain the tracking within a single grapheme (which can consist of multiple glyphs), but when we move to the next grapheme we reset the position so we can get proper monospaced rendering even for non-monospaced texts like CJK or stylized texts.
Fix #995
Fix #1172
Fix CoreText clipping issues with tall texts
This fixes the issue where particularly tall characters will get clipped at the row boundary. This happens because even though a font describes the line height with font metrics, individual glyphs do not have to respect them, and we can see with emoji rendering sometimes they can poke upwards past the line height. Also, it's trivially easy to construct composing characters that become really tall, e.g. "x゙̂⃗", or Tibetan scripts like "ཧཱུྃ".
To fix this, we do the following:
Remove the explicit clipping call at rendering.
Fix partial redraw to not lead to clipping / corruption. This is quite tricky, because let's say we have a character that is tall enough to touch other lines, if those lines are redraw but not the line with the tall char, the redraw will paint over the parts of the glyphs poking through. Alternatively if we redraw the line with the tall chars we also need to expand the redraw region to make sure other lines get repainted as well. To fix this properly, we should do a proper glyph calculation when we receive the draw command before we issue before we call setNeedsDisplayInRect, but since right now we only extract glyph info later (during drawRect call), it's too late. We just do the hacky solution by storing a variable redrawExpandRows that tracks how many lines to expand for all lines. It's a little hacky since this will affect all lines permanently regardless if they have tall characters or not. The proper fix may come later as an optimization (or when we do hardware rendering via Metal).
Re-order the rendering so we have a two pass system, where we first draw the background fill color for all rows, then the text. This helps prevent things like Vim's window split or cursorline from obscuring the text.
Add a preference to turn on clipping (old behavior). This helps prevent odd issues with really tall texts (e.g. Zalgo text) making it hard to see what's going on. The preference MMRendererClipToRow is not exposed in UI for now as it's a relatively niche. It will be exposed later when we have a dedicated render tab in settings.
Note that a lot of these characters only show their full height by doing set maxcombine=8 because the default (2) is quite low.
This fixes a couple outstanding CoreText rendering issues with more complicated characters. Separated them into separate commits to make the changes easier to identify.
Rename the "preserve line spacing" setting to "use macOS calculation"
The setting was added to "preserve" the font's original line spacing intents, but after further investigation I cannot find why Apple's calculation for font line spacing to be so wide. Furthermore it's quite odd because the default line spacing is wide, but the only thing that setting does is by re-initializing the font under Core Text (instead of using NSFont like the default) using CTFont and somehow the issue is fixed. Inspecting font metrics (using ttx) also didn't seem to give any hints why the spacing are so wide (e.g. ascent / descent / line gap etc all look fine). A StackOverflow comment seems to suggest that Apple is simply adding a 1.2x scale to some fonts (https://stackoverflow.com/questions/5511830/how-does-line-spacing-work-in-core-text-and-why-is-it-different-from-nslayoutm) which seems to match the observation. My guess is that Apple looks at some fonts and think they are too aggressive in their font metrics design and "helpfully" introduces a 1.2x multiplier to space them out, where Core Text is lower level than AppKit and therefore does not have this auto-inflation.
By renaming the option it should make it clearer that this is a somewhat arbitrary distinction instead of it being an inherent property of the font.
Remove the ceil() call around fontDescent in Core Text renderer
I have looked and I do not believe there is a good reason to do this rounding up at all. I believe the motivation is to align the baseline with the pixel boundary, but I don't believe that is necessary, given that Core Text will properly perform the correct antialiasing for us. Most texts aren't directly aligned at the baseline anyway. Before, that ceil() option is a reason why sometimes fonts would feel pushed upwards, clipping emojis to the top, and creating a lopsided effect for fonts like Fira Code (h11), since we draw the boxes aligned to the line height at the bottom of the descender, meaning the ceil() has an effect of pushing the text up.
Fix composing characters renderered at an offset
MacVim previously didn't really render composing characters with multiple glyphs correctly. For simple ones like 'â' it would work fine because Core Text just generates one glyph for it, but for more complicated ones like 'x⃗' the renderer didn't properly query the advance of the glyphs to put them at the correct position. Refactor the logic to keep track of the current cell/glyphs and make sure to track the advances fo the glyphs so the composing chars' glyphs would be laid out properly on top of the cell.
We need to be careful with the tracking, because in Vim we force the text rendering to be monospaced, and so we maintain the tracking within a single grapheme (which can consist of multiple glyphs), but when we move to the next grapheme we reset the position so we can get proper monospaced rendering even for non-monospaced texts like CJK or stylized texts.
Fix #995 Fix #1172
Fix CoreText clipping issues with tall texts
This fixes the issue where particularly tall characters will get clipped at the row boundary. This happens because even though a font describes the line height with font metrics, individual glyphs do not have to respect them, and we can see with emoji rendering sometimes they can poke upwards past the line height. Also, it's trivially easy to construct composing characters that become really tall, e.g. "x゙̂⃗", or Tibetan scripts like "ཧཱུྃ".
To fix this, we do the following:
Remove the explicit clipping call at rendering.
Fix partial redraw to not lead to clipping / corruption. This is quite tricky, because let's say we have a character that is tall enough to touch other lines, if those lines are redraw but not the line with the tall char, the redraw will paint over the parts of the glyphs poking through. Alternatively if we redraw the line with the tall chars we also need to expand the redraw region to make sure other lines get repainted as well. To fix this properly, we should do a proper glyph calculation when we receive the draw command before we issue before we call
setNeedsDisplayInRect, but since right now we only extract glyph info later (during drawRect call), it's too late. We just do the hacky solution by storing a variableredrawExpandRowsthat tracks how many lines to expand for all lines. It's a little hacky since this will affect all lines permanently regardless if they have tall characters or not. The proper fix may come later as an optimization (or when we do hardware rendering via Metal).Re-order the rendering so we have a two pass system, where we first draw the background fill color for all rows, then the text. This helps prevent things like Vim's window split or cursorline from obscuring the text.
Add a preference to turn on clipping (old behavior). This helps prevent odd issues with really tall texts (e.g. Zalgo text) making it hard to see what's going on. The preference
MMRendererClipToRowis not exposed in UI for now as it's a relatively niche. It will be exposed later when we have a dedicated render tab in settings.Note that a lot of these characters only show their full height by doing
set maxcombine=8because the default (2) is quite low.Part of the fix for #995