mirror of
https://github.com/microsoft/terminal.git
synced 2026-02-07 05:39:46 +00:00
Fix resize crash in OpenConsole with AtlasEngine (#13015)
`_api.cellCount` caches the `TextBuffer` size in AtlasEngine. Calculating it based on the `_api.sizeInPixel` is incorrect as the `TextBuffer` size doesn't necessarily have to be the size of the window. This can occur when the window is resized, as the main thread is receiving its `WM_SIZE` message and resizing the `TextBuffer` concurrently with the render thread performing a render pass and AtlasEngine checking the `GetClientRect`. In order to inform `AtlasEngine` about the initial buffer size, `Renderer` was modified to also invoke `UpdateViewport()` on the first render cycle. The only other user of `UpdateViewport()` is `VtEngine` which used to call `InvalidateAll()` in these situations. In order to prevent the `InvalidateAll()` call, `VtEngine::UpdateViewport()` was modified to suppress this. ## Validation Steps Performed * Resizing wide characters doesn't crash the terminal anymore ✅ * The additional call to `UpdateViewport()` doesn't break VtEngine ✅
This commit is contained in:
@@ -174,6 +174,9 @@ constexpr HRESULT vec2_narrow(U x, U y, AtlasEngine::vec2<T>& out) noexcept
|
||||
|
||||
[[nodiscard]] HRESULT AtlasEngine::UpdateViewport(const SMALL_RECT srNewViewport) noexcept
|
||||
{
|
||||
_api.cellCount.x = gsl::narrow_cast<u16>(srNewViewport.Right - srNewViewport.Left + 1);
|
||||
_api.cellCount.y = gsl::narrow_cast<u16>(srNewViewport.Bottom - srNewViewport.Top + 1);
|
||||
WI_SetFlag(_api.invalidations, ApiInvalidations::Size);
|
||||
return S_OK;
|
||||
}
|
||||
|
||||
@@ -396,7 +399,6 @@ void AtlasEngine::SetWarningCallback(std::function<void(HRESULT)> pfn) noexcept
|
||||
if (_api.sizeInPixel != newSize && newSize != u16x2{})
|
||||
{
|
||||
_api.sizeInPixel = newSize;
|
||||
_api.cellCount = _api.sizeInPixel / _api.fontMetrics.cellSize;
|
||||
WI_SetFlag(_api.invalidations, ApiInvalidations::Size);
|
||||
}
|
||||
|
||||
@@ -536,7 +538,6 @@ void AtlasEngine::_updateFont(const wchar_t* faceName, const FontInfoDesired& fo
|
||||
|
||||
if (previousCellSize != _api.fontMetrics.cellSize)
|
||||
{
|
||||
_api.cellCount = _api.sizeInPixel / _api.fontMetrics.cellSize;
|
||||
WI_SetFlag(_api.invalidations, ApiInvalidations::Size);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -390,12 +390,13 @@ bool Renderer::_CheckViewportAndScroll()
|
||||
const auto srOldViewport = _viewport.ToInclusive();
|
||||
const auto srNewViewport = _pData->GetViewport().ToInclusive();
|
||||
|
||||
if (srOldViewport == srNewViewport)
|
||||
if (!_forceUpdateViewport && srOldViewport == srNewViewport)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
_viewport = Viewport::FromInclusive(srNewViewport);
|
||||
_forceUpdateViewport = false;
|
||||
|
||||
COORD coordDelta;
|
||||
coordDelta.X = srOldViewport.Left - srNewViewport.Left;
|
||||
|
||||
@@ -123,6 +123,7 @@ namespace Microsoft::Console::Render
|
||||
std::vector<SMALL_RECT> _previousSelection;
|
||||
std::function<void()> _pfnRendererEnteredErrorState;
|
||||
bool _destructing = false;
|
||||
bool _forceUpdateViewport = true;
|
||||
|
||||
#ifdef UNIT_TESTING
|
||||
friend class ConptyOutputTests;
|
||||
|
||||
@@ -246,18 +246,41 @@ CATCH_RETURN();
|
||||
[[nodiscard]] HRESULT VtEngine::UpdateViewport(const SMALL_RECT srNewViewport) noexcept
|
||||
{
|
||||
auto hr = S_OK;
|
||||
const auto oldView = _lastViewport;
|
||||
const auto newView = Viewport::FromInclusive(srNewViewport);
|
||||
const auto oldSize = _lastViewport.Dimensions();
|
||||
const auto newSize = newView.Dimensions();
|
||||
|
||||
_lastViewport = newView;
|
||||
|
||||
if ((oldView.Height() != newView.Height()) || (oldView.Width() != newView.Width()))
|
||||
if (oldSize != newSize)
|
||||
{
|
||||
// Don't emit a resize event if we've requested it be suppressed
|
||||
if (!_suppressResizeRepaint)
|
||||
{
|
||||
hr = _ResizeWindow(newView.Width(), newView.Height());
|
||||
hr = _ResizeWindow(newSize.X, newSize.Y);
|
||||
}
|
||||
|
||||
if (_resizeQuirk)
|
||||
{
|
||||
// GH#3490 - When the viewport width changed, don't do anything extra here.
|
||||
// If the buffer had areas that were invalid due to the resize, then the
|
||||
// buffer will have triggered it's own invalidations for what it knows is
|
||||
// invalid. Previously, we'd invalidate everything if the width changed,
|
||||
// because we couldn't be sure if lines were reflowed.
|
||||
_invalidMap.resize(til::size{ newSize });
|
||||
}
|
||||
else
|
||||
{
|
||||
if (SUCCEEDED(hr))
|
||||
{
|
||||
_invalidMap.resize(til::size{ newSize }, true); // resize while filling in new space with repaint requests.
|
||||
|
||||
// Viewport is smaller now - just update it all.
|
||||
if (oldSize.Y > newSize.Y || oldSize.X > newSize.X)
|
||||
{
|
||||
hr = InvalidateAll();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
_resized = true;
|
||||
}
|
||||
|
||||
@@ -269,29 +292,7 @@ CATCH_RETURN();
|
||||
// If we only clear the flag when the new viewport is different, this can
|
||||
// lead to the first _actual_ resize being suppressed.
|
||||
_suppressResizeRepaint = false;
|
||||
|
||||
if (_resizeQuirk)
|
||||
{
|
||||
// GH#3490 - When the viewport width changed, don't do anything extra here.
|
||||
// If the buffer had areas that were invalid due to the resize, then the
|
||||
// buffer will have triggered it's own invalidations for what it knows is
|
||||
// invalid. Previously, we'd invalidate everything if the width changed,
|
||||
// because we couldn't be sure if lines were reflowed.
|
||||
_invalidMap.resize(til::size{ newView.Dimensions() });
|
||||
}
|
||||
else
|
||||
{
|
||||
if (SUCCEEDED(hr))
|
||||
{
|
||||
_invalidMap.resize(til::size{ newView.Dimensions() }, true); // resize while filling in new space with repaint requests.
|
||||
|
||||
// Viewport is smaller now - just update it all.
|
||||
if (oldView.Height() > newView.Height() || oldView.Width() > newView.Width())
|
||||
{
|
||||
hr = InvalidateAll();
|
||||
}
|
||||
}
|
||||
}
|
||||
_lastViewport = newView;
|
||||
|
||||
return hr;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user