starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-03 21:25:34 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

DBCS characters rendered incorrectly when scrolling horizontally in conhost #11570

New Issue
Closed
opened 2026-01-31 02:51:26 +00:00 by claunia · 4 comments
claunia commented 2026-01-31 02:51:26 +00:00
Owner
Copy Link

Originally created by @j4james on GitHub (Nov 24, 2020).

Environment

Windows build number: Version 10.0.18363.1198
Windows Terminal version (if applicable): Commit 1fbcf34ba8

Steps to reproduce

  1. Build OpenConsole.
  2. Run Host.EXE with a bash shell.
  3. Open the console properties and turn off Wrap text output on resize.
  4. Also make sure the Screen Buffer Size is wider than the Window Size so there's a horizontal scrollbar.
  5. Enter some DBCS characters at the prompt (the example below uses 子小尒尢).
  6. Wiggle the horizontal scroll bar backwards and forwards a few times, so those characters are being scrolled in and out of view.

I should add that I just picked these characters at random from charmap, so my apologies if they mean anything offensive.

Expected behavior

The characters should always be rendered correctly.

image

Actual behavior

Sometimes the characters get chopped in half, or rendered in the wrong place so they're overlapping other characters.

image

I can't reproduce this in the conhost that comes with my current version of Windows, so it looks like it might be a regression. But I also checked out the initial commit of OpenConsole and it seems to have already been broken then. Is it possible my version of Windows is so old that it's prior to the first open source release?

Anyway, I think I know what the problem is - a couple of problems actually. I'll try and write up an explanation later with my proposed fix.

Originally created by @j4james on GitHub (Nov 24, 2020). # Environment Windows build number: Version 10.0.18363.1198 Windows Terminal version (if applicable): Commit 1fbcf34ba8a5edfe2d78ffcf69496f10b9cc3d25 # Steps to reproduce 1. Build _OpenConsole_. 2. Run _Host.EXE_ with a bash shell. 3. Open the console properties and turn off _Wrap text output on resize_. 4. Also make sure the _Screen Buffer Size_ is wider than the _Window Size_ so there's a horizontal scrollbar. 5. Enter some DBCS characters at the prompt (the example below uses `子小尒尢`). 6. Wiggle the horizontal scroll bar backwards and forwards a few times, so those characters are being scrolled in and out of view. I should add that I just picked these characters at random from charmap, so my apologies if they mean anything offensive. # Expected behavior The characters should always be rendered correctly. ![image](https://user-images.githubusercontent.com/4181424/100147658-946a5980-2e93-11eb-874a-78f6e3a365e2.png) # Actual behavior Sometimes the characters get chopped in half, or rendered in the wrong place so they're overlapping other characters. ![image](https://user-images.githubusercontent.com/4181424/100147691-9af8d100-2e93-11eb-94ed-d9d5870ba02f.png) I can't reproduce this in the conhost that comes with my current version of Windows, so it looks like it might be a regression. But I also checked out the initial commit of _OpenConsole_ and it seems to have already been broken then. Is it possible my version of Windows is so old that it's prior to the first open source release? Anyway, I think I know what the problem is - a couple of problems actually. I'll try and write up an explanation later with my proposed fix.
claunia commented 2026-01-31 02:51:27 +00:00
Author
Owner
Copy Link

@j4james commented on GitHub (Nov 24, 2020):

As far as I can tell, the problem is in the Renderer::_PaintBufferOutputHelper method. When the content starts with the trailing half of a DBCS character, it tries to move the screenPoint back a position, so it can safely render the full character. See below:

26ca73b823/src/renderer/base/renderer.cpp (L767-L783)

But as you can see, if the X position is already in column 0, it instead moves forward one position and "skips" that character. At least that's the intention, but the code doesn't actually increment the iterator, so it just ends up rendering the character in the wrong place. And even if it worked as intended, at best you're going to end up with half a character not rendered.

My suggested fix for this is just to drop that check. If the X position is already in column 0, let it go negative. It should then paint the character exactly as expected, with half of it off screen. At least that seems to work for the GDI engine.

But that brings us to the next problem. When we're dealing with the second half of a DBCS char, the it->Columns() value is 1, so we set the columnCount to it->Columns() + 1 to allow for the fact that it's actually going to occupy two cells. However, that columnCount variable is also used to increment the iterator, so it ends up incrementing too much, and any subsequent characters will be rendered in the wrong place, or possibly skipped altogether. See below:

26ca73b823/src/renderer/base/renderer.cpp (L797-L799)

My suggested fix is to use the it->Columns() value itself for the increment, since that should always be correct. So something like this:

it += std::max<size_t>(it->Columns(), 1);

I'm not sure if we still need to make sure the increment is non-zero, but it probably wouldn't harm.

@j4james commented on GitHub (Nov 24, 2020): As far as I can tell, the problem is in the `Renderer::_PaintBufferOutputHelper` method. When the content starts with the trailing half of a DBCS character, it tries to move the `screenPoint` back a position, so it can safely render the full character. See below: https://github.com/microsoft/terminal/blob/26ca73b823cde97cd2425baec62961947a8f85ee/src/renderer/base/renderer.cpp#L767-L783 But as you can see, if the X position is already in column 0, it instead moves forward one position and "skips" that character. At least that's the intention, but the code doesn't actually increment the iterator, so it just ends up rendering the character in the wrong place. And even if it worked as intended, at best you're going to end up with half a character not rendered. My suggested fix for this is just to drop that check. If the X position is already in column 0, let it go negative. It should then paint the character exactly as expected, with half of it off screen. At least that seems to work for the GDI engine. But that brings us to the next problem. When we're dealing with the second half of a DBCS char, the `it->Columns()` value is 1, so we set the `columnCount` to `it->Columns() + 1` to allow for the fact that it's actually going to occupy two cells. However, that `columnCount` variable is also used to increment the iterator, so it ends up incrementing too much, and any subsequent characters will be rendered in the wrong place, or possibly skipped altogether. See below: https://github.com/microsoft/terminal/blob/26ca73b823cde97cd2425baec62961947a8f85ee/src/renderer/base/renderer.cpp#L797-L799 My suggested fix is to use the `it->Columns()` value itself for the increment, since that should always be correct. So something like this: ```cpp it += std::max<size_t>(it->Columns(), 1); ``` I'm not sure if we still need to make sure the increment is non-zero, but it probably wouldn't harm.
claunia commented 2026-01-31 02:51:28 +00:00
Author
Owner
Copy Link

@DHowett commented on GitHub (Nov 25, 2020):

I believe @miniksa changed something in here recently (after 1836x, as you've observed) that fixed an issue with ConPTY; blame might be enlightening here. 😄

Otherwise, this looks like the right fix. I hope we have test coverage for the ConPTY thing!

@DHowett commented on GitHub (Nov 25, 2020): I believe @miniksa changed something in here recently (after 1836x, as you've observed) that fixed an issue with ConPTY; blame might be enlightening here. :smile: Otherwise, this looks like the right fix. I hope we have test coverage for the ConPTY thing!
claunia commented 2026-01-31 02:51:28 +00:00
Author
Owner
Copy Link

@j4james commented on GitHub (Nov 29, 2020):

@DHowett I think the PR you're thinking of is #4668, but that didn't introduce this bug. It was an attempt to fix a related issue with fullwidth glyphs (#2191), but didn't correctly handle this particular case. I've now created a PR with my proposed changes, and confirmed that the test cases described in #2191 are still working correctly with those changes.

However, I should note that the #2191 issue is no longer reproducible in Terminal even with this whole section commented out (i.e. the fix introduced in #4668). I suspect that might be because the DX engine now always invalidates a full line at a time. I could reproduce the problem with the GDI engine in conhost, though, so could confirm that the fix does still work there.

@j4james commented on GitHub (Nov 29, 2020): @DHowett I think the PR you're thinking of is #4668, but that didn't introduce this bug. It was an attempt to fix a related issue with fullwidth glyphs (#2191), but didn't correctly handle this particular case. I've now created a PR with my proposed changes, and confirmed that the test cases described in #2191 are still working correctly with those changes. However, I should note that the #2191 issue is no longer reproducible in Terminal even with this whole section commented out (i.e. the fix introduced in #4668). I suspect that might be because the DX engine now always invalidates a full line at a time. I could reproduce the problem with the GDI engine in conhost, though, so could confirm that the fix does still work there.
claunia commented 2026-01-31 02:51:28 +00:00
Author
Owner
Copy Link

@ghost commented on GitHub (Jan 28, 2021):

:tada:This issue was addressed in #8438, which has now been successfully released as Windows Terminal Preview v1.6.10272.0.🎉

Handy links:

@ghost commented on GitHub (Jan 28, 2021): :tada:This issue was addressed in #8438, which has now been successfully released as `Windows Terminal Preview v1.6.10272.0`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.6.10272.0) * [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-Rendering Impact-Compatibility Impact-Correctness Issue-Bug Product-Conhost Resolution-Fix-Committed
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#11570
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?