Tall characters have their top and/or bottom cut off

[Tyriar]

This applies to tall g's and some other characters on some fonts and also to diacritical marks in various languages.

VS Code issue: microsoft/vscode#35901

[Tyriar]

This is by far the most reported issue against the terminal in VS Code

[Jeyanthinath]

@Tyriar Can you do it a bit quicker as it is nearly impossible to use vscode for demos as it is very hard to explain the audience about the _ is hidden.

It really hurts the daily usage. :(

[Tyriar]

@Jeyanthinath I'm busy dealing with accessibility at the moment. There are several workarounds mentioned in microsoft/vscode#35901 (change font size, change font family)

[ioquatix]

Is this the issue we are having here? ioquatix/script-runner#93 (comment)

[Tyriar]

@ioquatix I noticed there may be issues with the default 18.04 Ubuntu font. It may be the similar #1170

[segevfiner]

This reproduces with DejaVu Sans Mono for Powerline, and also likely with the normal DejaVu Sans Mono, at 14px, using the canvas renderer. Which is the default configuration in VS Code on Ubuntu 16.04, and likely others.

The problem seems to be that Element.getBoundingClientRect in chromium returns height off by one:

JSON.stringify(term._core.charMeasure._measureElement.getBoundingClientRect())
{"x":-139870,"y":81.875,"width":8.4375,"height":16,"top":81.875,"right":-139861.5625,"bottom":97.875,"left":-139870}

In Firefox:

// Grrrr: https://bugzilla.mozilla.org/show_bug.cgi?id=1100611
term._core.charMeasure._measureElement.getBoundingClientRect()
DOMRect { x: -139592, y: 86.86666870117188, width: 8, height: 17, top: 86.86666870117188, right: -139584, bottom: 103.86666870117188, left: -139592 }

This simple hack fixes this, but it will also result in height being one pixel larger when the getBoundingClientRect does return the correct result. So it's not really a fix, just a way to prove this is really the problem.

diff --git a/src/ui/CharMeasure.ts b/src/ui/CharMeasure.ts
index 5ad1de7..7be6d83 100644
--- a/src/ui/CharMeasure.ts
+++ b/src/ui/CharMeasure.ts
@@ -48,7 +48,7 @@ export class CharMeasure extends EventEmitter implements ICharMeasure {
     }
     if (this._width !== geometry.width || this._height !== geometry.height) {
       this._width = geometry.width;
-      this._height = Math.ceil(geometry.height);
+      this._height = Math.ceil(geometry.height + 1);
       this.emit('charsizechanged');
     }
   }

Maybe there is a way to get Chromium to return the correct result here (Different measurement character, CSS style, etc.), or maybe there is an alternative way to calculate this metric which is more reliable.

Versions:
xterm.js master at 3419480
Chromium 68.0.3440.75
Firefox 61.0.1
Ubuntu 16.04.5 x64

P.S. I added source-map-loader locally to the webpack setup so that I would be able to debug the demo in the browser. Might want to consider this for the committed setup.

EDIT: It seems this isn't a rounding error in Chromium. The issue is that getBoundingClientRect doesn't include font descenders, possibly also ascenders/cap height. You can see this by making xterm-char-measure-element visible using the developer tools and adding an underscore to it. You will see that it's drawn below the bounding box that the developers tools show you. No idea if that is because of problematic font metrics or intentional. It seems like there is no proper API to get font metrics in the web. 😞

EDIT 2: It might also be a rounding error in chrome when calculating the font height which results in the underscore being cut off, since it is at the bottom of the font. After all, it doesn't happen in all font sizes which means that chrome might be trying to calculate the correct size.

[Tyriar]

@segevfiner I see this issue in Chrome as well, where the selection does not include all the text. If Chrome turns on advanced TextMetric props we might be able to fix it by using that instead of DOM to measure.

[Tyriar]

@segevfiner if source-map-loader fixes source maps in the demo, yes please! 😍

[segevfiner]

@segevfiner if source-map-loader fixes source maps in the demo, yes please! 😍

#1601

[segevfiner]

@segevfiner I see this issue in Chrome as well, where the selection does not include all the text. If Chrome turns on advanced TextMetric props we might be able to fix it by using that instead of DOM to measure.

It will probably take some time for this API to stabilize. Maybe some polyfill or some library implemented a measurement technique that works... Maybe https://github.com/motiz88/canvas-text-metrics-polyfill or https://www.npmjs.com/package/text-metrics (Quick Google search, There are likely more). Needs testing to see if any library gets it right.

[Tyriar]

@segevfiner good to know. I was actually putting something together that's very similar to that for text measuring the other week (getImageData and analysing pixels). I won't have time to look at it again for a while, but that's good validation that this is the right direction.

[segevfiner]

I tried enabling "Experimental Web Platform features" and checking if we can get the correct value from it, but it seems like even that API returns a value that is one pixel short.

The used test code: textmetrics-test.zip

P.S. Wow... The property names they choose for these are quite obscure. 😖

[Tyriar]

@segevfiner if that's buggy too I guess it needs to Chrome bug to get fixed first.

Not sure I could come up with better names for 12 things that describe a bounding box 😉

[zwhitchcox]

Is there any progress on this?This happens for me in chrome and firefox

[Tyriar]

@zwhitchcox it's fixed in #1790, this probably won't ship for some time though.

[martin31821]

Seeing this in chromium 71 as well now :(

[Tyriar]

This issue was closed off as it's fixed in the experimental WebGL renderer addon. It's not ready yet but I'm closing this off so I don't forget about it. See this query for current webgl addon issues https://github.com/xtermjs/xterm.js/issues?q=is%3Aissue+is%3Aopen+label%3Aarea%2Faddon%2Fwebgl