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

Webgl cursor renderer makes sure a char is not drawn outside of the cell #4102

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

JasonXJ
Copy link

@JasonXJ JasonXJ commented Sep 6, 2022

This fixes the rendering artefact reported here in #3878.

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't actually do anything as only cursor and link render layers are used in the webgl renderer. All text drawing in the webgl renderer is done in WebglCharAtlas and GlyphRenderer.

@JasonXJ
Copy link
Author

JasonXJ commented Sep 7, 2022

CursorRenderLayer draws the char with block cursor. So let's look at its _render() method (code link). On line 181, we clear the cursor cell, and at line 185, we call _renderBockCursor(), which renders the char. The char can be extra wide, so if we don't clip it, we will leave something on the screen the next time you move the cursor left (because the _clearCursor() only clear the single cell).

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, I should have read the linked comment properly. I can't actually reproduce this when going back to the normal buffer, but I can reproduce the artifact issue when zoomed in, but I can see how it could happen. It seems to be another case of rounding problems that get covered when devicePixelRatio < 1:

https://github.com/silamon/xterm.js/blob/e8e2f16cffe89e9a08899a2a37c2a4cfa580cca9/addons/xterm-addon-webgl/src/renderLayer/CursorRenderLayer.ts#L197-L200

I guess your fix is actually better here as a black cursor with a wide character rendering an inverted foreground color would probably look better if the part beyond the cursor was the un-inverted color.

Thanks for looking into this!

@Tyriar Tyriar added this to the 5.0.0 milestone Sep 7, 2022
@Tyriar Tyriar self-assigned this Sep 7, 2022
@Tyriar Tyriar merged commit 327ece0 into xtermjs:master Sep 7, 2022
@JasonXJ JasonXJ deleted the clip-cursor-cell branch December 7, 2022 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants