maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fix rendering of DBCS characters when partially off screen (#8438)

Browse source 
When the renderer is called on to render part of a line starting halfway
through a DBCS character (as can occur in conhost when the viewport is
offset horizontally), it could result in the character not being
displayed, and/or with following the characters drawn in the wrong
place. This PR is an attempt to fix those problems. 

The original code for handling the trailing half of fullwidth glyphs was
introduced in PR #4668 to fix issue #2191.

When the content being rendered starts with the trailing half of a DBCS
character, the renderer tries to move the `screenPoint` back a position,
so it can instead render the full character, but instructing the render
engine to trim off the left half of it.

If the X position was already in column 0, though, it would instead move
forward one position, intending to skip that character. At best this
would mean the half character wouldn't be rendered, but since the
iterator wasn't incremented correctly, it actually just ended up
rendering the character in the wrong place.

The fix for this was simply to drop the check for the X position being
in column 0, and allow it go negative. The rendering engine would then
just start rendering the character partially off screen, and only the
second half of it would be displayed, which is exactly what is needed.

The second problem was that the code incrementing the iterator was using
the `columnCount` variable rather than the `it->Columns()` value for the
current position. When dealing with the trailing half of a DBCS
character, the `columnCount` is 2, but the `Columns()` value is 1. If
you use the former rather than the later, then the renderer may skip the
following character.

## Validation Steps Performed
I've developed a more easily reproducible version of the test case
described in #8390, and confirmed that the problem no longer occurs when
this PR is applied.

I've also manually confirmed that the problem described in #2191 that
was fixed by PR #4668 is still working correctly now.

Closes #8390
This commit is contained in:
James Holderness 2020-12-04 00:39:23 +00:00 committed by GitHub
parent 2a2f6b32a2
commit 3ccd831a3b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 8 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
src/renderer/base/renderer.cpp
View file

@ -764,23 +764,13 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine,
// (a.k.a. the right half of a two column character), then we need some special handling.
if (_clusterBuffer.empty() && it->DbcsAttr().IsTrailing())
{
// If we have room to move to the left to start drawing...
if (screenPoint.X > 0)
{
// Move left to the one so the whole character can be struck correctly.
--screenPoint.X;
// And tell the next function to trim off the left half of it.
trimLeft = true;
// And add one to the number of columns we expect it to take as we insert it.
columnCount = it->Columns() + 1;
_clusterBuffer.emplace_back(it->Chars(), columnCount);
}
else
{
// If we didn't have room, move to the right one and just skip this one.
screenPoint.X++;
continue;
}
// Move left to the one so the whole character can be struck correctly.
--screenPoint.X;
// And tell the next function to trim off the left half of it.
trimLeft = true;
// And add one to the number of columns we expect it to take as we insert it.
columnCount = it->Columns() + 1;
_clusterBuffer.emplace_back(it->Chars(), columnCount);
}
// Otherwise if it's not a special case, just insert it as is.
else
@ -795,7 +785,7 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine,
}
// Advance the cluster and column counts.
it += columnCount > 0 ? columnCount : 1; // prevent infinite loop for no visible columns
it += std::max<size_t>(it->Columns(), 1); // prevent infinite loop for no visible columns
cols += columnCount;
} while (it);