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

Clearing formatting results in different background color for different renderers #4120

Closed
Tyriar opened this issue Sep 15, 2022 · 17 comments · Fixed by #4438
Closed

Clearing formatting results in different background color for different renderers #4120

Tyriar opened this issue Sep 15, 2022 · 17 comments · Fixed by #4438
Labels
area/addon/canvas type/bug Something is misbehaving
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Sep 15, 2022

VS Code issue: microsoft/vscode#160497

image

The rendering of the repro in the linked issue is different in webgl+canvas vs dom

@Tyriar
Copy link
Member Author

Tyriar commented Dec 24, 2022

Ansi colors test button show a difference too:

webgl:

image

canvas:

image

@benz0li
Copy link

benz0li commented Mar 15, 2023

@Tyriar Is there an approximate timeframe for resolving this issue? Is it weeks or rather months?

I want to keep the default setting in code-server; and documenting workarounds is not ideal.

@jerch
Copy link
Member

jerch commented Mar 15, 2023

This is mostly BCE related (also see https://invisible-island.net/ncurses/ncurses.faq.html#bce_mismatches). From my quick tests it seems the canvas and the webgl renderer work properly, but the DOM renderer does not correctly apply BCE to new lines.

Regarding the issue itself:
If BCE is set, the BG color to be used for erasing is not the terminals default BG color anymore, but the currently active one. Now the tricky part - which color should be applied to initialize new lines at the bottom? We currently do that with the active BG color as well, therefore this weird output, if the terminal had to spawn new lines at the bottom. While thats kinda surprising and differs from output within the initial viewport, it is in fact the expected behavior. From my quick tests only the DOM renderer does not do that correctly, prolly due to early finishing the lines spans (have not investigated).

Did not extensively test this, thus there is a chance I missed some other real bug here. I also cannot repro the differences from the color test above, no clue why.

@jerch
Copy link
Member

jerch commented Mar 15, 2023

On how to workaround this BCE madness - lets take a simple example:

echo -e '\x1b[41ma\nb'

Intention here is pretty clear - print 2 lines with a and b with red BG.

A BCE aware TE would print this:

  • within original viewport
    image
  • at the bottom creating a new terminal line:
    image
    Due to the need of a new line the currently active BG (red) got applied all over the second line. This raises a few questions:
    • Why not for the first line? Well the first line gets created from shell prompt while the default BG color is still active, thus it gets filled with default BG color. Not so for the second line, BG is red at the time of line creation, so the line turns red.
    • What about the third line? The third line (where the new shell prompt is) also does not show the BCE effect for another reason - the PS1 snippet here in use erases the line with defaults before writing anything. If used with an empty PS1 var, the effect can be triggered (using sh to avoid any shell trickery):
      image

To avoid the BCE effect, we can explicitly erase the line with BG defaults, either:

  • before writing to it (right after the NL)
    echo -e '\x1b[41ma\n\x1b[49m\x1b[2K\x1b[41mb'
  • or by erasing left over cells to the right after writing (before the NL)
    echo -e '\x1b[41ma\nb\x1b[49m\x1b[K'

Both should give us the wanted output (though keep in mind, that the second variant might destroy output to the right of the cursor, so it is not applicable for complex line manipulations with cursor forth and back moves).

@benz0li
Copy link

benz0li commented Mar 15, 2023

From my quick tests it seems the canvas and the webgl renderer work properly

Canvas renderer does not work properly.


The following settings in code-server (aka VS Code) lead to proper rendering of the terminal in Safari:

  • "terminal.integrated.gpuAcceleration": "on" – if supported
  • "terminal.integrated.gpuAcceleration": "off"

The following settings lead to improper rendering in Safari:

  • "terminal.integrated.gpuAcceleration": "auto"
    ℹ️ Proper rendering in Firefox, because auto uses wegbl
  • "terminal.integrated.gpuAcceleration": "canvas"

@jerch: I have whitelisted your GitHub user at https://coder.jupyter.b-data.ch/ so you may have a look at it yourself.
👉 Image: R (base:test-devtools)

@benz0li
Copy link

benz0li commented Mar 15, 2023

No worries. I am documenting the workaround, then.

@jerch
Copy link
Member

jerch commented Mar 15, 2023

You mean webgl works properly but canvas does not? Then I am prolly on the wrong track here, for me canvas and webgl behave exactly the same. I wonder if this is a safari only issue (there are some issues with safari at several ends...)

@benz0li
Copy link

benz0li commented Mar 15, 2023

You mean webgl works properly but canvas does not?

Yes.

I wonder if this is a safari only issue (there are some issues with safari at several ends...)

No. There is the same issue with the canvas renderer in Firefox.

@jerch
Copy link
Member

jerch commented Mar 15, 2023

@benz0li Can you give a repro for the issue, how to trigger it?

@benz0li
Copy link

benz0li commented Mar 15, 2023

@benz0li Can you give a repro for the issue, how to trigger it?

@jerch: I have whitelisted your GitHub user at https://coder.jupyter.b-data.ch/ so you may have a look at it yourself.
👉 Image: R (base:test-devtools). Open code-server and create a new terminal.

@jerch
Copy link
Member

jerch commented Mar 15, 2023

I see the following (in FF):

  • webgl
    image
  • canvas
    image
  • DOM
    image

From he looks only the webgl renderer has the right output atm.

canvas has multiple issues:

  • First line of [a, b] should not have turned red.
  • There are positioning and coloring issues in the powerline prompt.

DOM has one issue not applying BCE at all. Prompt looks right to me (beside minor offset issues, which cannot be solved, it is a font renderer issue of the browser).

So yes there are differences in canvas and webgl. Which xterm.js version is here used? And can you test a different version? (My tests above used 5.1.0, so this either is a regression or got already solved in 5.1.0...)

@Tyriar
Copy link
Member Author

Tyriar commented Mar 15, 2023

Is there an approximate timeframe for resolving this issue? Is it weeks or rather months?

@benz0li I'm strapped for time at the moment so probably won't be able to look at it for a while, so more like months unless someone else steps in.

@benz0li
Copy link

benz0li commented Mar 15, 2023

Which xterm.js version is here used?

This issue popped up when VS Code 1.75.0 came out.

xterm.js 5.2.0-beta.21; according to https://github.com/microsoft/vscode/blob/e2816fe719a4026ffa1ee0189dc89bdfdbafb164/package.json#L90

@jerch
Copy link
Member

jerch commented Mar 15, 2023

Yupp, thats a regression in master branch, can repro it there as well. Hmm.

@benz0li
Copy link

benz0li commented Mar 15, 2023

No issue with VS Code versions using xterm 5.1.0-beta.49 (and maybe other 5.1.0-betas).

This must have been introduced with xterm.js 5.2.0.

@jerch
Copy link
Member

jerch commented Mar 15, 2023

Found a possible culprit:

const code = cell.getCode();
if (code === 0 || code === 32) {
continue;
}

Commented out these lines, the proper behavior is restored. I introduced these lines in 0b56b56 to greatly speedup the canvas renderer, but was not aware, that it also gets used for BG coloring. This needs a more involved patch to have both good perf and correct behavior, so for now I gonna revert that change --> PR: #4438.

@benz0li
Copy link

benz0li commented Mar 15, 2023

@jerch Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/addon/canvas type/bug Something is misbehaving
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants