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

DOM Renderer: Render background separately #4818

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mofux
Copy link
Contributor

@mofux mofux commented Sep 20, 2023

This PR attempts to resolve clipping issues with the DOM renderer when a cell has a background color set and the rendered character is wider than its cell.

It does so by creating separate spans for backgrounds. These background spans are positioned absolute and are added to the row div before any of the text spans (so the text spans will always be drawn above).

Text spans can now be merged even if the background color differs, so in many scenarios we won't even produce more spans than before.

As a little bonus, the text spans are now display: inline which should provide a little performance boost and might be beneficial when dealing with ligatures.

TODO

  • Do not create background span if there is no background
  • Remove selection layer and draw it as part of the background
  • Make sure to handle cursor and decorations properly
  • Adjust tests
  • Analyze how performance differs to old DOM renderer

@jerch
Copy link
Member

jerch commented Sep 20, 2023

beneficial when dealing with ligatures.

And BIDI. 😸

@Tyriar
Copy link
Member

Tyriar commented Sep 21, 2023

This PR attempts to resolve some rendering issues with the DOM renderer when a cell has a background color set.

Can you elaborate on the issues this will fix?

@bmeares
Copy link

bmeares commented Nov 4, 2023

Perhaps this will fix the issues with rendering double-width chars (#4813)? Linking for reference

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.

4 participants