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

Finish up DEC Special Graphics character set #891

Merged
merged 5 commits into from Jun 20, 2021
Merged

Finish up DEC Special Graphics character set #891

merged 5 commits into from Jun 20, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jun 19, 2021

Hi! This is my first PR ever in Rust, so it likely needs work. But the point of it:

  1. DEC Special Graphics is saved/restored by DECSC/DECRC. (And it's actually not just one bool, it should be the selected character sets for G0, G1, G2, G3, and GR; and by default selectable via ASCII SO and SI.)
  2. Added the remaining mappings. I do not know why some of the glyphs aren't making it to the screen correctly. These are all standard Unicode glyphs: ◆▒␉␌␍␊°±␤␋┘┐┌└┼⎺⎻─⎼⎽├┤┴┬│≤≥π≠£· Many of these are also accessible to ncurses applications via ACS_DIAMOND, ACS_CKBOARD, etc.

With these changes (and assuming you know how to fix the glyphs for ␉ ␌ ␍ ␊ ␋), this would finish out the screen features SAVE/RESTORE CURSOR screen, and get better on the VT100 character sets screen.

@ghost
Copy link
Author

ghost commented Jun 19, 2021

Reference also #49 and #133.

Copy link
Owner

@wez wez left a comment

Choose a reason for hiding this comment

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

Thanks!

Would you mind adding a basic test for this?

I think you could get away with doing something like this:
https://github.com/wez/wezterm/blob/main/term/src/test/mod.rs#L632-L642
but printing the relevant sequences to get into the alt charset and print out the chars, and then assert that they show up as the intended unicode chars.

You can run the term tests directly via cargo test -p wezterm-term

term/src/terminalstate.rs Outdated Show resolved Hide resolved
"`" => "◆",
"a" => "▒",

// AZL: I do not know why the next four Unicode glyphs are
Copy link
Owner

Choose a reason for hiding this comment

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

Is that a local font problem? If I copy and paste the characters from your PR description into the terminal I see those glyphs in my terminal on this mac. If you do the same, do you see those glyphs on your system, or is the issue that the don't show up when used in the appropriate mode when rendered via the terminal?

wezterm will log information about glyphs that it failed to resolve. It can take several seconds to do that if you're in a debug build, because it has to parse and compute glyph coverage maps for a potentially large list of fallback fonts, and that's really slow in a debug build.

Copy link
Author

Choose a reason for hiding this comment

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

I saw nothing logged to stderr on the glyphs. wezterm ls-fonts yields:

 Primary font:
wezterm.font_with_fallback({
  -- <built-in>, BuiltIn
  "JetBrains Mono",

  -- /usr/share/fonts/truetype/noto/NotoColorEmoji.ttf, FontConfig
  "Noto Color Emoji",

  -- <built-in>, BuiltIn
  "Last Resort High-Efficiency",

})

The other fonts are all the same: JetBrains Mono, Noto, Last Resort.

When I paste " ␉ ␌ ␍ ␊ ␋" into cat I see these instead:

glyphs

I've got the format fixed, and will add the test. (This is fun! :) )

Copy link
Owner

Choose a reason for hiding this comment

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

I think the glyphs you're seeing are coming from a fallback font; they seem like alternative pictorial versions of ␉ ␌ ␍ ␊ ␋. I think I should add a thing to ls-fonts to have it explain which fonts are used for a given text input to help understand cases like this!

I've got the format fixed, and will add the test. (This is fun! :) )

Great!

Copy link
Owner

Choose a reason for hiding this comment

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

wezterm ls-fonts --text "␉ ␌ ␍ ␊ ␋" will explain which fonts were used now in main

@ghost
Copy link
Author

ghost commented Jun 20, 2021

OK, this update handles VT100 ASCII, UK, and DEC character sets. It passes the vttest VT100 character set test, and can support ncurses ACS chars.

What do you think?

Copy link
Owner

@wez wez left a comment

Choose a reason for hiding this comment

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

This is looking great thanks! I think the only thing outstanding is that there are couple of tests in the termwiz crate that need adjusting to the EscCode changes, but this otherwise looks ready to land.

Thanks!

/// Designate Character Set – DEC Line Drawing
DecLineDrawing = esc!('(', '0'),
/// Designate Character Set – US ASCII
AsciiCharacterSet = esc!('(', 'B'),
Copy link
Owner

Choose a reason for hiding this comment

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

There are a couple of tests in the termwiz crate that need to be adjusted for this change:
https://github.com/wez/wezterm/blob/main/termwiz/src/render/terminfo.rs#L891

You can run those tests using cargo test -p termwiz (or just run all of the tests: cargo test)

@ghost
Copy link
Author

ghost commented Jun 20, 2021

When I use Terminus font, the scan line glyphs (⎺⎻─⎼⎽) look good for one moment, and are then overwritten with wrongly-placed lines that look like (⎺⎻──⎼). I thought I saw some logic for wezterm to draw its own glyphs; is there a way to turn that feature off, so that I can ensure I am testing my font and not that drawing code?

@wez wez merged commit 39535bb into wez:main Jun 20, 2021
wez pushed a commit that referenced this pull request Jun 20, 2021
wez pushed a commit that referenced this pull request Jun 20, 2021
@wez
Copy link
Owner

wez commented Jun 20, 2021

When I use Terminus font, the scan line glyphs (⎺⎻─⎼⎽) look good for one moment, and are then overwritten with wrongly-placed lines that look like (⎺⎻──⎼). I thought I saw some logic for wezterm to draw its own glyphs; is there a way to turn that feature off, so that I can ensure I am testing my font and not that drawing code?

Hmm, that sounds like a fallback font is being loaded asynchronously.
If you run: WEZTERM_LOGG=wezterm_font=trace,info wezterm it should show you what's happening in the font crate and perhaps give some insight

wez added a commit that referenced this pull request Jun 20, 2021
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.

1 participant