starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-04-06 06:09:50 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Fix line drawing during IME operations. (#6223)

Browse Source 
## Summary of the Pull Request
Restores proper line drawing during IME operations in `conhost`

## PR Checklist
* [x] Closes #803
* [x] I work here.
* [x] Tested manually.
* [x] Check the performance of this and see if it's worse-enough to merit a more confusing algorithm. It was worse for the majority case so I scoped it.
* [x] No doc, it should have worked this way.
* [x] Am core contributor.

## Detailed Description of the Pull Request / Additional comments
- Changed `ConsoleImeInfo::s_ConvertToCells` to be less confusing. It's doing about the same thing, but it's way easier to read now and the compiler/linker/optimizer should just be the same.
- Edited `Renderer::_PaintBufferOutputHelper` to check each attribute for line drawing characters as the right half of a two-col character might have different line drawing characters than the left-half.

## Validation Steps Performed
- [x] Manual operation of IME in conhost with Japanese IME.
- [x] Manual operation of IME in conhost with Chinese IME.
- [x] Manual operation of IME in conhost with Chinese (Traditional) IME.
- [x] Manual operation of IME in conhost with and Korean IME. - @leonMSFT says Korean doesn't work this way. But Korean is broken worse in that it's not showing suggestions at all. Filing new bug. #6227 
- [x] Validated against API-filling calls through `SetConsoleTextAttribute` per @j4james's sample code
This commit is contained in:
Michael Niksa
2020-06-03 15:21:04 -07:00
committed by GitHub
parent 413b658805
commit 9b075e985a
2 changed files with 50 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/host/conimeinfo.cpp
View File

@@ -258,6 +258,7 @@ std::vector<OutputCell> ConsoleImeInfo::s_ConvertToCells(const std::wstring_view
if (IsGlyphFullWidth(glyph))
{
auto leftHalfAttr = drawingAttr;
auto rightHalfAttr = drawingAttr;
// Don't draw lines in the middle of full width glyphs.
// If we need a right vertical, don't apply it to the left side of the character
@@ -271,12 +272,16 @@ std::vector<OutputCell> ConsoleImeInfo::s_ConvertToCells(const std::wstring_view
dbcsAttr.SetTrailing();
// If we need a left vertical, don't apply it to the right side of the character
if (drawingAttr.IsLeftVerticalDisplayed())
if (rightHalfAttr.IsLeftVerticalDisplayed())
{
drawingAttr.SetLeftVerticalDisplayed(false);
rightHalfAttr.SetLeftVerticalDisplayed(false);
}
cells.emplace_back(glyph, dbcsAttr, rightHalfAttr);
}
else
{
cells.emplace_back(glyph, dbcsAttr, drawingAttr);
}
cells.emplace_back(glyph, dbcsAttr, drawingAttr);
}
return cells;

44
src/renderer/base/renderer.cpp
View File

@@ -705,6 +705,11 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine,
screenPoint.X += gsl::narrow<SHORT>(cols);
cols = 0;
// Hold onto the start of this run iterator and the target location where we started
// in case we need to do some special work to paint the line drawing characters.
const auto currentRunItStart = it;
const auto currentRunTargetStart = screenPoint;
// Ensure that our cluster vector is clear.
clusters.clear();
@@ -713,6 +718,9 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine,
// as the first item in our run.
bool trimLeft = false;
// Run contains wide character (>1 columns)
bool containsWideCharacter = false;
// This inner loop will accumulate clusters until the color changes.
// When the color changes, it will save the new color off and break.
do
@@ -762,6 +770,11 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine,
clusters.emplace_back(it->Chars(), columnCount);
}
if (columnCount > 1)
{
containsWideCharacter = true;
}
// Advance the cluster and column counts.
it += columnCount > 0 ? columnCount : 1; // prevent infinite loop for no visible columns
cols += columnCount;
@@ -772,10 +785,37 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine,
THROW_IF_FAILED(pEngine->PaintBufferLine({ clusters.data(), clusters.size() }, screenPoint, trimLeft, lineWrapped));
// If we're allowed to do grid drawing, draw that now too (since it will be coupled with the color data)
// We're only allowed to draw the grid lines under certain circumstances.
if (_pData->IsGridLineDrawingAllowed())
{
// We're only allowed to draw the grid lines under certain circumstances.
_PaintBufferOutputGridLineHelper(pEngine, currentRunColor, cols, screenPoint);
// See GH: 803
// If we found a wide character while we looped above, it's possible we skipped over the right half
// attribute that could have contained different line information than the left half.
if (containsWideCharacter)
{
// Start from the original position in this run.
auto lineIt = currentRunItStart;
// Start from the original target in this run.
auto lineTarget = currentRunTargetStart;
// We need to go through the iterators again to ensure we get the lines associated with each
// exact column. The code above will condense two-column characters into one, but it is possible
// (like with the IME) that the line drawing characters will vary from the left to right half
// of a wider character.
// We could theoretically pre-pass for this in the loop above to be more efficient about walking
// the iterator, but I fear it would make the code even more confusing than it already is.
// Do that in the future if some WPR trace points you to this spot as super bad.
for (auto colsPainted = 0; colsPainted < cols; ++colsPainted, ++lineIt, ++lineTarget.X)
{
auto lines = lineIt->TextAttr();
_PaintBufferOutputGridLineHelper(pEngine, lines, 1, lineTarget);
}
}
else
{
// If nothing exciting is going on, draw the lines in bulk.
_PaintBufferOutputGridLineHelper(pEngine, currentRunColor, cols, screenPoint);
}
}
}
}