xterm.js icon indicating copy to clipboard operation
xterm.js copied to clipboard

[WebGL and Canvas Rendering] Font does not scale correctly with DPR higher than 1

Open CrimsonFez opened this issue 2 years ago • 13 comments

When using WebGL or Canvas addons and displaying on a device with a DPR higher than 1 the text inside the container becomes larger than it should be.

DPR 4 image

DPR 1 image

When using WebGL I also get this error in Firefox when I set the DPR higher than 1.

WebGL warning: drawElementsInstanced: Drawing to a destination rect smaller than the viewport rect. (This warning will only be given once)

Details

  • Firefox and Brave Desktop with Responsive Design Mode enabled both exhibit this
  • OS version: Fedora 38
  • xterm.js version: 5.2.1
  • Canvas addon version: 0.4.0
  • WebGL addon version: 0.15.0
  • I'm using VueJS 3 to build my SPA

Steps to reproduce

  1. Create a site that uses xtermjs with either canvas or webgl
  2. Test the site with Responsive Design Mode

CrimsonFez avatar Aug 25 '23 01:08 CrimsonFez

DPR 4 means 400% zoom so it looks like it's behaving as expected to me? Am I misunderstanding something?

Tyriar avatar Aug 25 '23 14:08 Tyriar

I would expect that the font sizing in the terminal would be the same as the font sizing of regular elements. From what I understand about DPR, currently this is unusable on a mobile device.

I currently don't have the ability to test my site on a real mobile device. But I would expect that Responsive Design Mode does a good job of emulating a mobile device screen size.

Please correct me if I'm wrong about these things. I'm pretty new to web development.

CrimsonFez avatar Aug 25 '23 16:08 CrimsonFez

I might be wrong but, it seams to me like the browser is adjusting the DPR of the canvas before xterm does. Then, xterm is doing its own DPR adjustment which would cause a much larger zoom than intended.

Things should be the same size between DPR 1 and DPR 4 because the browser is adjusting the device resolution, not the resolution of the CSS.

CrimsonFez avatar Aug 25 '23 17:08 CrimsonFez

You're right this is an issue, I think it only applies to the canvas renderer though.

Canvas:

image

Webgl:

image

The WebGL warning: drawElementsInstanced: Drawing to a destination rect smaller than the viewport rect. (This warning will only be given once) warning only happens once when enabling the addon, it works fine when switching DPR after the fact. Created https://github.com/xtermjs/xterm.js/issues/4731 to track that.

Tyriar avatar Aug 26 '23 13:08 Tyriar

@Tyriar I get that behavior on both canvas and webgl. Only the DOM renderer does not show it here. I am not 100% sure, if thats just an issue of the responsive view in devtools, or if there is really something off with dimension calc (still dont think the latter is the case, as we prolly would see much issue reports then).

jerch avatar Aug 26 '23 13:08 jerch

Weird, I also suspect something might be off with the devtools as I've never seen a report of this in practice.

Tyriar avatar Aug 26 '23 13:08 Tyriar

Also its stable across browsers, Firefox shows the same weird behavior. :exploding_head:

jerch avatar Aug 26 '23 13:08 jerch

I also meet this problem that caused by DPR, maybe we can set DPR to canvas width and height, like this:

 var dpr = window.devicePixelRatio || 1;
  // Get the size of the canvas in CSS pixels.
  var rect = canvas.getBoundingClientRect();
  // Give the canvas pixel dimensions of their CSS
  // size * the device pixel ratio.
  canvas.width = rect.width * dpr;
  canvas.height = rect.height * dpr;

yangguansen avatar Nov 29 '23 09:11 yangguansen

The issue is reproducible in Chrome if you emulate iPhone 12 and toggle WebGL on and off you can see different sizes of the cells:

https://github.com/xtermjs/xterm.js/assets/1040420/66909753-60f6-4a13-b4c1-ac3532f98ef9

The issue originates in DevicePixelObserver:

https://github.com/xtermjs/xterm.js/blob/2edc65baaa50f8b99c67f3df6f8d5d93c70a69e6/src/browser/renderer/shared/DevicePixelObserver.ts#L28-L29

In the demo above, width and height are 1242 and 2516, if I change the calculation to:

const width = entry.contentBoxSize[0].inlineSize * window.devicePixelRatio;
const height = entry.contentBoxSize[0].blockSize * window.devicePixelRatio;

I get 1863 3774 which produces the proper visual sizing, consistent with Canvas/DOM:

https://github.com/xtermjs/xterm.js/assets/1040420/4c269509-7c8a-4d6c-83c5-986748aea076

Another fix that works is just removing these lines completely: https://github.com/xtermjs/xterm.js/blob/2edc65baaa50f8b99c67f3df6f8d5d93c70a69e6/addons/addon-webgl/src/WebglRenderer.ts#L128-L131

The issue doesn't happen on Safari since it doesn't implement the devicePixelContentBoxSize.

szymonkaliski avatar Apr 22 '24 10:04 szymonkaliski

@szymonkaliski Thx for your investigation.

Your finding with

contentBoxSize[0].inlineSize * window.devicePixelRatio
// vs.
devicePixelContentBoxSize[0].inlineSize

looks promising as a good fix. Is that guaranteed to yield the same numbers for the normal use cases?

Since I did not write the DPR stuff in xterm.js, lets hear @Tyriar about it and possible implications.

jerch avatar Apr 22 '24 10:04 jerch

The purpose of observeDecivePixelDimensions and devicePixelContentBoxSize is to get pixel-perfect actual device pixel count for scaling that is not 1.0 DPR. This was inconsistent and impossible to determine before that API exists, even a single pixel different would make the canvas blurry.

If you change it to entry.contentBoxSize[0].inlineSize * window.devicePixelRatio I think it would just bring back that long time problem.

Tyriar avatar Apr 22 '24 10:04 Tyriar

@Tyriar Ic, so rounding issues will occur again here. Would it be possible to catch the devtools view with a tiny heuristic, something like this (pseudocode):

if abs(contentBoxSize * devicePixelRatio - devicePixelContentBoxSize) > smallish_pixel_uncertainty:
  # we have a big discrepancy prolly due to devtools messing DPR up
  return round(contentBoxSize * devicePixelRatio)  # or floor/ceil, whatever is better here

# normal use cases
return devicePixelContentBoxSize

jerch avatar Apr 22 '24 11:04 jerch