-
Notifications
You must be signed in to change notification settings - Fork 838
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
Console rendering bug when displaying VT on HDPI #1907
Comments
Just adding some of my own investigation. This appears to vary based on a combination of font AND size, not just the font alone. The first console in this image has (Consolas, 16), the second is (Consolas, 20), and the third shows what happens when you output [(Courier New 20), (Consolas, 20), (Lucidia Console, 20)]. Consolas 16 doesn't have this problem, but Consolas 20 does. @miniksa for more investigation. I opened MSFT:11641556 to track this. |
The problem here is that \u2592 is an ambiguous width character as defined in Unicode: http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt That means I have no idea if it's going to be a single column or two columns wide. So the heuristic we use is how many pixels is it going to consume versus a '0' in the currently selected font. Apparently in Consolas 16, that shaded box character consumes less than a '0' and in Consolas 20, it consumes more pixels than a '0' which is what is causing this rendering snafu. Adding High DPI to the mix makes this worse because now there's a MulDiv calculation with rounding potential to make the shaded box have more or less pixels than a '0'. If you have any ideas on how to more reliably detect whether it will take 1 or 2 columns in GDI, I'm all ears. This is just a very sensitive code path because drawing to the screen takes about 80% of our CPU time (because we still use GDI and it's done on the CPU) so I'm hesitant to make some wildly complicated mechanism here that might significantly impact the CPU usage/time. I think the correct solution is to just capture this problem in the scope of the work required to move to a modern renderer like DirectWrite on DirectX. Unfortunately that means no quick fix. |
@miniksa I had good results with the following code to determine the columns used by a string: ansi.getDisplayWidth uses GetStringTypeW(CT_CTYPE3, bstrText, cchLine, rgType), skips escape sequences, then simply check each character width as follow: You can completely ignore the non-spacing diacritics (C3_NONSPACING) as the console shows them as individual spacing characters anyway, so this test will correctly count them as 1 column. |
@miniksa If you really want to check the actual GDI width of the character in the specific font, asian glyphs are very consistent with double-width always taking a full square. So I would change the test from (chrPixelWidth > pixelWidth('0')) to (charPixelWidth > cfix.dwFontSize.X*1.5), which should still catch all full-width without false-positives on half-width that bleeds a bit outside a single column. |
Good ideas indeed. I still have to think it through a bit further though because things like the selection highlight are still going to be messed up if something takes 9 pixels when a '0' (or whatever metric we use like perhaps your suggestion above instead) takes 8 pixels. The characters will jump on redraw or the selection rectangle won't be drawn on top of the right spot. The only way I can think to fix that is to draw each character individually and chop it down to force it to fit inside its rectangle (or to leave enough space before drawing the next one) instead of drawing each row as a run like we do right now. The problem is, we draw runs because the performance is already terrible to call draw 25 times to draw 25 lines on the screen. I can't imagine how bad it would be to call to draw 80x25 individual characters on the screen (or more). Maybe some trade-off where it only draws individually when detecting an ambiguous character? Or maybe there's some GDI trickery I haven't figured out yet... |
@zadjii-msft Moving here from #1936 Hmm. If I have a choice between prioritizing support for 3rd-party consoles (which should also cover things like embedded terminals, e.g. VS code, right?) and making the default console awesome, I'll pick the first, since a third-party console doesn't have to care about compatibility and it takes the pressure off you guys to support everything perfectly now without breaking anything 😉 That said, this looks like a curly one. It sounds like you're currently using |
@miniksa You wrote:
So do you folks actually have plans to make a new Terminal? There are so many excellent terminal's included with Linux, seems like porting one of those to Windows would be the easiest path... And how great to have forks of the terminal code to add things like image support, higher range of colors... ConEmu seems to be the least painful option for now, but still a long ways from the native Linux terminals... |
@ryanerwin #111 seems to be, despite the obsolete title, the closest thing to an issue tracking "make the console api public", but basically all the label:console issues are "make a new terminal", in the sense of "fix everything in the current terminal: conhost.exe" |
@ryanerwin Considering the console performance is directly linked to its text rendering calls, making a cross-platform console with some abstraction in the text rendering isn't the right way to go. Abstraction always comes at a cost. A modern console using DirectWrite is the best way to go for a console on Windows and since the performance would impact all text-based apps, the effort to write a good console taking advantage of the DirectX platform is justified. |
@miniksa Taking another approach here. The problem is basically linked to how some fonts render at different sizes and mostly to how block elements and line (box) drawing characters render imperfectly at the pixel level, ending up a bit too narrow or too wide (and exhibit the same problem with their height, btw). So how about you push this problem to the Typography team and get them to fix the fonts that are now used for the console, mainly Consolas, to be better hinted and generate pixel-perfect blocks and lines glyphs at all DPI and font sizes supported by conhost? This would mitigate the current issue by making the characters that currently exhibit these issues fit their cells perfectly. And it wouldn't be wasted work on a temporary fix as it would make the rendering with the standard fonts more precise and better looking even on a future DirectWrite-based console. |
@miniksa I didn't notice this bug before the release of 1709 because it only happens in some rare cases, and I must admit I'm still unable to build a simple project that reproduces the issue. This is very erratic, it seems to be linked to font size again, but even at the same font size, a call to SetConsoleMode, or showing and OK'ing the console properties dialog (which I assume calls the same API) usually clears the error forever in that console window. Any idea ? |
@zadjii-msft have you seen anything with the 7m and 27m states recently that might explain this? |
Nope, this looks new to me. Might be related to the reflowing thing, but I don't think that code path would be hit here. I've filed a bug to look into this later this cycle. |
@zadjii-msft I couldn't replicate it because I was focusing my sample code on the \x1B[7m and \x1B[27m codes, but it only happens if you've set 24-bit VT colors earlier.
|
@zadjii-msft |
@PhMajerus Awesome! thanks for the super simple repro steps. I'd say that it's unlikely I fixed the reverse sequence to work with RGB color. That shouldn't be too hard to fix though. Thanks for investigating! I'd say it's best to keep the threads separate, so if you could file a new issue that'd be great. |
@zadjii-msft So it's not just in your \x1B[27m handler code path that handles if current fore or back is 24-bit, as in this sample it isn't anymore. There is something else that makes it stop working even if the color is back to a base console color. |
Closing this issue as characters width dependent on font-size issue is fixed. |
In RS2 / Creators Update (15063.138)
The console sometimes displays extra spaces in strings containing ANSI/VT colors when displayed at another DPI scaling than 100%. It happens a lot at 125% and 175% and a bit less but still common at 150%.
Testing the following in a console with the default Consolas font at 20 pixels high (default settings in latin script regions), on a system set to 150% (default on a Surface Pro device). ClearType is enabled, as that can change the rendered text width.
echo -e "\u2592##"
Displays a medium shade block followed by two # characters, 3 columns total.
echo -e "\E[41m\u2592#\E[49m#"
Should display the same characters, but showing the shade block and the first # over a red background.
Instead, it inserts an extra space of exactly 1 column width between the two # characters.
This is a bug in conhost and affects any console app, not just WSL. Confirmed in Win32 as well.
The console seems to output text in chunks of characters using identical attributes. If a color is changed, it computes the x location for the next chunk and starts displaying from that column. It seems to work fine when the display DPI is set to a 100% scaling, but it is regularly off by one when another system scaling ratio is used. This inserts spaces in colored text, breaking alignment.
It depends on previously displayed characters, but the shading blocks aren't the only ones affected. It seems the console computes the starting column of the next chunk based on the pixels width of the previous chunks instead of simply by counting the displayable characters, and probably rounds up some width that overflows a bit into the next column in some fonts and smoothing modes when rendered at some DPI scaling zoom.
This breaks many ANSI-art effects and complex text-based UI layouts.
Related: Parsing bug in 24bit VT colors (#1681)
These should probably be handled together, as they both exhibit a problem with how the console stores and render colored text.
The text was updated successfully, but these errors were encountered: