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

Resize from the left causes flickering and characters misplaced #1701

Closed
wspl opened this issue Sep 19, 2018 · 11 comments
Closed

Resize from the left causes flickering and characters misplaced #1701

wspl opened this issue Sep 19, 2018 · 11 comments
Labels
type/bug Something is misbehaving

Comments

@wspl
Copy link

wspl commented Sep 19, 2018

I added a resize feature to my application. It supports resizing the Terminal from both left and right. But I found a weird phenomenon that the resize performance on the left is not the same as the right. Resize from the left will cause the terminal text flickering, and fast resize causes characters misplaced. (Slowly wouldn't)

Details

  • Browser and browser version: Electron 3.0.0 Chromium 66
  • OS version: macOS Sierra
  • xterm.js version: 3.7.0

Steps to reproduce

  1. Make a resizable container for xterm.js Terminal
  2. resize the container from left side

Video

https://drive.google.com/open?id=1KU9z5loHpnf1XY8qRyMUwHqbpf9eyB5w

@Tyriar
Copy link
Member

Tyriar commented Sep 19, 2018

@wspl I cannot reproduce the flickering on the same electron, os and xterm.js version. The only thing that should be different moving from the left is that the actual element is moving. I would guess this is related to Electron/Chrome unless your application is doing something extra?

For the characters misplaced, I've seen this before. I think it's related to the experience of wrapping the prompt, which pushes the above line up. This isn't much better in other terminal so leaving as designed for now:

screen shot 2018-09-19 at 7 44 06 am

@mofux
Copy link
Contributor

mofux commented Sep 19, 2018

Try to log the rows and cols as you resize the terminal, my guess would be that if you resize from the left, it will log 0 or NaN values because the container resize implementation you are using makes the xterm container "jump" between 0-size and the actual size.

@wspl
Copy link
Author

wspl commented Sep 20, 2018

@mofux Actually, the resize implementation is based on ResizeObserver API. I printed the logs and there are no any 0 or NaN:
image

@wspl
Copy link
Author

wspl commented Sep 20, 2018

@Tyriar I reproduced the problem in fiddle: https://jsfiddle.net/q40m3cox/24/

@mofux
Copy link
Contributor

mofux commented Sep 20, 2018

Thanks for the fiddle, I can reproduce the flickering. I don't think you are doing anything wrong - strange. I'm using a very similar way to resize xterm in my app - and it is not flickering. Was xterm.js hosted inside an iframe in your electron app that you posted the video from?

@wspl
Copy link
Author

wspl commented Sep 20, 2018

@mofux No. If I move the code from jsfiddle to a pure .html file without iframe, the flickering can be reproduced too.

<!DOCTYPE html>
<html>
<head>
  <script src="https://cdn.jsdelivr.net/npm/xterm@3.7.0/dist/xterm.js"></script>
  <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/xterm@3.7.0/dist/xterm.css">
  <style>
    .box {
      display: flex;
      height: 400px;
    }
    .left-resizable {
      resize: both;
      overflow: auto;
      min-width: 100px;
      background: yellow;
      margin-right: 10px;
    }
    .right {
      flex-grow: 1;
      position: relative;
    }
    #terminal {
      position: absolute;
    }
  </style>
</head>

<body>
  <div class="box">
    <div class="left-resizable"></div>
    <div class="right">
      <div id="terminal"></div>
    </div>
  </div>
  <script>
    const term = new Terminal()
    term.open(document.getElementById('terminal'))
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
    term.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')

    const observer = new ResizeObserver(entries => {
      console.log(term)
      const cellWidth = term._core.renderer.dimensions.actualCellWidth
      const cellHeight = term._core.renderer.dimensions.actualCellHeight
      const cols = Math.floor(entries[0].contentRect.width / cellWidth)
      const rows = Math.floor(entries[0].contentRect.height / cellHeight)

      term.resize(cols, rows)
    })
    observer.observe(document.querySelector('.right'))
  </script>
</body>
</html>

@mofux
Copy link
Contributor

mofux commented Sep 20, 2018

Yep, also reproduces for me. I've tested in Chrome on Mac and Linux, it happens in both.

@mofux mofux reopened this Sep 20, 2018
@Tyriar
Copy link
Member

Tyriar commented Sep 21, 2018

I can repro on a much more stripped down example https://jsfiddle.net/dasw0gb3/

Not sure why it's happening here and not in my implementation but maybe it's something to do with the canvas texture getting cleared and needing to get redrawn whenever you call resize? Note that the DOM renderer does not show this.

Resize emits resize:

this.emit('resize', {cols: x, rows: y});

Which tells the renderer to resize

this.register(this.addDisposableListener('resize', () => this.renderer.onResize(this.cols, this.rows)));

Which resizes the render layers:

this._renderLayers.forEach(l => l.resize(this._terminal, this.dimensions));

This queued a new build on the next animation frame (maybe this is it?):

// Force a refresh
if (this._isPaused) {
this._needsFullRefresh = true;
} else {
this._terminal.refresh(0, this._terminal.rows - 1);
}


I tried the standalone example in #1701 (comment) and I don't see it 😕

@jerch
Copy link
Member

jerch commented Sep 22, 2018

Repro'ed with all chromium versions I have here on Ubuntu (65, 68, 69, 71, thanks to puppeteer). The timeline shows heavy painting and GPU activity, scripting is < 10%. Firefox shows no perceivable flickering at all. In electron 2 there is flickering as well but less obvious.

Maybe this is just "bad timing" of the browser blitting the actual canvas content into to the screen? At least there are several places, where the canvas has an intermediate "black'd out" state:

this.clearAll();

this.clearCells(0, firstRow, terminal.cols, lastRow - firstRow + 1);

If thats the culprit it could be solved by offscreen prerendering and doing the clear & draw thing coupled together as close as possible. (Maybe just for the resize part, I think general prerendering has a negative impact on runtime).

@Tyriar
Copy link
Member

Tyriar commented Sep 22, 2018

Well I can see this happening if the frame rate is low, because the canvas would be cleared (new texture when it's resized) and then it's redrawn next in an animation frame (which takes longer if the FPS is low). A draw immediately after resize should fix it, then couple that with a check to make sure it's not drawn again until the next animation frame to make sure perf doesn't get out of hand if resize is called many times.

@Tyriar Tyriar added type/bug Something is misbehaving area/renderer and removed as-designed labels Sep 22, 2018
@Tyriar
Copy link
Member

Tyriar commented Oct 7, 2019

Can't repro anymore

@Tyriar Tyriar closed this as completed Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is misbehaving
Projects
None yet
Development

No branches or pull requests

4 participants