Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canvas flickers when canvas position is moving. #4922

Open
ethan-vanderheijden opened this issue Dec 21, 2023 · 6 comments
Open

Canvas flickers when canvas position is moving. #4922

ethan-vanderheijden opened this issue Dec 21, 2023 · 6 comments
Labels
area/addon/canvas area/addon/webgl type/enhancement Features or improvements to existing features

Comments

@ethan-vanderheijden
Copy link

ethan-vanderheijden commented Dec 21, 2023

If the xterm canvas moves as the screen size changes, the canvas starts flickering:

prepared_bug.mp4

The cause of the flicker is that the width of the canvas oscillates between 1897 and 1898 (maybe due to some rounding error?). Here's a zoomed-in video of the canvas HTML element as the flicker is happening:

bug_style_zoomed.mp4

Note: the cause of this issue is not that the window resizes but that the canvas position changes. Here's a video of the window resizing but there is no flicker present because the canvas position stays the same:

no_bug_prepared.mp4

Details

  • Browser and browser version: Chrome 120 and Firefox 120
  • OS version: Windows 11
  • xterm.js version: 5.3.0
  • xterm-addon-webgl version: 0.16.0

Steps to reproduce

Put xterm inside a responsive layout. When you resize the window and the layout shifts, xterm will start flickering. You can actually reproduce this at xtermjs.org.

@jerch
Copy link
Member

jerch commented Dec 22, 2023

@ethan-vanderheijden Does this only happen with the webgl renderer?

Note that your screencasts are somewhat strange - in your first video it seems, that the terminal container element should change width, as the left border moves slower than the right one, but the canvas dimensions do not follow that expectation (instead show that 1 pixel rounding wobbling). Same with the last video. You dont seem to have the fit addon active?

Would be good, if you can investigate the rounding issue, as it already might be the reason for the flickering. Just moving the canvas around should not cause that. Is this done plain in HTML, or is there any other framework involved (react, vue etc)?

@ethan-vanderheijden
Copy link
Author

I can reproduce the issue with addon-webgl and addon-canvas.

I disabled the fit addon for my screencasts so the canvas width should stay constant. I am rendering Xterm inside a React component, but I don't think React is responsible for changing the canvas's width. After writing this issue, I realized I could reproduce the flickering using the demo at xtermjs.org.

bug_homepage_prepared.mp4

Can you reproduce this flickering too? Or is it just me?

Strangely enough, I can confirm that the flickering is produced by the canvas's positioning. If I set the xterm div to position: relative and manually change the left attribute, then every time I move the div left by 1 pixel, the width of the canvas wobbles.

bug_positioning_prepared.mp4

In the video, I had to turn off and on the left attribute before xterm detected the position change and updated the canvas's width. Although I am using React, I doubt it is responsible for setting the canvas width because only the outer element is rendered using React. Also, the demo at xtermjs.org doesn't seem to be using React and still exhibits flickering.

In the meantime, I'll dig through the source code and try to figure out what piece of code is setting the canvas width. It might have to wait until January, however.

@ethan-vanderheijden
Copy link
Author

ethan-vanderheijden commented Dec 23, 2023

Ok, I've located the bug:

const width = entry.devicePixelContentBoxSize[0].inlineSize;

The bug only happens if my browser's devicePixelRatio is greater than 1 and smaller than 1.66667

It looks like adddon-canvas and addon-webgl both use this method to watch for changes to the canvas's size. When the canvas does change size, entry.devicePixelContentBoxSize[0] is used to get the new canvas size in real device pixels. However, if my devicePixelRatio is between 1 and 1.66667, the browser thinks the canvas's size is wobbling when its position changes and mistakenly throws a resize event. In other words, the browser is responsible for the rounding error, not xterm.

This is sort of an unfixable bug because devicePixelContentBoxSize is the most accurate way to get an element's size, and if it has rounding issues of its own, there's nothing that can be done. The hack-ish fix would be to ignore canvas resizes unless the difference between the new width/height and old width/height is >2 pixels.

@jerch
Copy link
Member

jerch commented Dec 23, 2023

@ethan-vanderheijden Yes I can repro the size wobbling in chrome and firefox, if I change the DPR.

I remember having a similar issue for the tiling in the image addon creating stripes, the solution there was to always floor left/top borders, and to ceil right/bottom borders (thus allowing a tiny bit of "overscan").
Not sure if a similar trick is possible here, ideally we find a more robust width/height calc function, that leads to a stable number independent of the (fractional) canvas offsets.

@jerch jerch added type/enhancement Features or improvements to existing features area/addon/webgl area/addon/canvas and removed needs more info labels Dec 23, 2023
@ethan-vanderheijden
Copy link
Author

I'm not sure if we can find a more robust width/height calc function because we aren't calculating the width/height ourselves, instead we are pulling the pixel size directly from the browser API. The browser API isn't giving us an incorrect size either as the rendered size of the canvas really is wobbling (I took a screenshot and measured it). It's not a bug in that sense, more like a browser "quirk". devicePixelContentBoxSize isn't inaccurate, more like it's a little too accurate.

Since this is such a minor bug, it might not be worth fixing. After all, with the fit addon enabled, xterm already flickers when resized, so any additional flickering due to the canvas size wobbling is imperceptible.

@tisilent
Copy link
Contributor

tisilent commented Dec 24, 2023

When resizing, adjust to a better display effect, which may occur when using canvas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/addon/canvas area/addon/webgl type/enhancement Features or improvements to existing features
Projects
None yet
Development

No branches or pull requests

3 participants