mirror of
https://github.com/aphrody-labs/terminal.git
synced 2026-10-14 14:19:16 +00:00
Prevent panes from shrinking below 2x2 cells (#20564)
Raise the visible terminal minimum from 1×1 cells to 2×2 cells so a pane
or window can no longer reach a 1-column viewport (keyboard, mouse,
`split-pane`, `wt --size`, `initialCols`/`initialRows`, or VT resize).
This mitigates an issue where we hang when we display a 2-cell char
in one cell.
Refs:
- j4james: 2×2 is the VT theoretical minimum (`DECSTBM` height ≥ 2,
`DECSLRM` width ≥ 2).
Verification:
- Confirmed the hang on unpatched Dev (`WindowsTerminalDev`) and Store
1.24.11911 with `echo 音楽🎵测试` in view, then shrink the left pane to the
stop.
Closes #19996
(cherry picked from commit f88a1cdafd)
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
Service-Version: 1.24
This commit is contained in:
1 parent
835d60f116
commit
416ff03988
11 files changed
+63
-43
No files matched your search
@@ -2844,13 +2844,12 @@ void Pane::_AdvanceSnappedDimension(const bool widthOrHeight, LayoutSizeNode& si
|
||||
|
||||
// Method Description:
|
||||
// - Get the absolute minimum size that this pane can be resized to and still
|
||||
// have 1x1 character visible, in each of its children. If we're a leaf, we'll
|
||||
// include the space needed for borders _within_ us.
|
||||
// satisfy each child's MinimumSize. If we're a leaf, we'll include the
|
||||
// space needed for borders _within_ us.
|
||||
// Arguments:
|
||||
// - <none>
|
||||
// Return Value:
|
||||
// - The minimum size that this pane can be resized to and still have a visible
|
||||
// character.
|
||||
// - The minimum size that this pane can be resized to and still fit its content.
|
||||
Size Pane::_GetMinSize() const
|
||||
{
|
||||
if (_IsLeaf())
|
||||
|
||||
@@ -405,12 +405,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
LOG_IF_FAILED(_renderEngine->SetWindowSize({ viewInPixels.Width(), viewInPixels.Height() }));
|
||||
|
||||
const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels);
|
||||
const auto width = vp.Width();
|
||||
const auto height = vp.Height();
|
||||
const til::size viewportSize{ Utils::ClampToShortMax(vp.Width(), MINIMUM_VISIBLE_CELLS),
|
||||
Utils::ClampToShortMax(vp.Height(), MINIMUM_VISIBLE_CELLS) };
|
||||
|
||||
if (_connection)
|
||||
{
|
||||
_connection.Resize(height, width);
|
||||
_connection.Resize(viewportSize.height, viewportSize.width);
|
||||
}
|
||||
|
||||
if (_owningHwnd != 0)
|
||||
@@ -422,8 +422,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
}
|
||||
|
||||
// Override the default width and height to match the size of the swapChainPanel
|
||||
_settings->InitialCols(width);
|
||||
_settings->InitialRows(height);
|
||||
_settings->InitialCols(viewportSize.width);
|
||||
_settings->InitialRows(viewportSize.height);
|
||||
|
||||
_terminal->CreateFromSettings(*_settings, *_renderer);
|
||||
|
||||
@@ -1167,10 +1167,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
auto cx = gsl::narrow_cast<til::CoordType>(lrint(_panelWidth * _compositionScale));
|
||||
auto cy = gsl::narrow_cast<til::CoordType>(lrint(_panelHeight * _compositionScale));
|
||||
|
||||
// Don't actually resize so small that a single character wouldn't fit
|
||||
// in either dimension. The buffer really doesn't like being size 0.
|
||||
cx = std::max(cx, _actualFont.GetSize().width);
|
||||
cy = std::max(cy, _actualFont.GetSize().height);
|
||||
// Don't resize below the visible minimum. A 1-cell viewport can hang
|
||||
// TextBuffer::Reflow on a wide glyph (GH#19996). The buffer also
|
||||
// doesn't like being size 0.
|
||||
const auto cell = _actualFont.GetSize();
|
||||
cx = std::max(cx, cell.width * MINIMUM_VISIBLE_CELLS);
|
||||
cy = std::max(cy, cell.height * MINIMUM_VISIBLE_CELLS);
|
||||
|
||||
// Convert our new dimensions to characters
|
||||
const auto viewInPixels = Viewport::FromDimensions({ 0, 0 }, { cx, cy });
|
||||
@@ -1186,7 +1188,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
// If this function succeeds with S_FALSE, then the terminal didn't
|
||||
// actually change size. No need to notify the connection of this no-op.
|
||||
const auto hr = _terminal->UserResize({ vp.Width(), vp.Height() });
|
||||
const auto cols = std::max(vp.Width(), MINIMUM_VISIBLE_CELLS);
|
||||
const auto rows = std::max(vp.Height(), MINIMUM_VISIBLE_CELLS);
|
||||
const auto hr = _terminal->UserResize({ cols, rows });
|
||||
if (FAILED(hr) || hr == S_FALSE)
|
||||
{
|
||||
return;
|
||||
@@ -1194,7 +1198,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
if (_connection)
|
||||
{
|
||||
_connection.Resize(vp.Height(), vp.Width());
|
||||
_connection.Resize(rows, cols);
|
||||
}
|
||||
|
||||
// TermControl will call Search() once the OutputIdle even fires after 100ms.
|
||||
|
||||
@@ -434,10 +434,10 @@ HRESULT HwndTerminal::Refresh(const til::size windowSize, _Out_ til::size* dimen
|
||||
const auto viewInPixels = Viewport::FromDimensions({}, windowSize);
|
||||
const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels);
|
||||
|
||||
// Guard against resizing the window to 0 columns/rows, which the text buffer classes don't really support.
|
||||
// Guard against resizing below the visible minimum (GH#19996).
|
||||
auto size = vp.Dimensions();
|
||||
size.width = std::max(size.width, 1);
|
||||
size.height = std::max(size.height, 1);
|
||||
size.width = std::max(size.width, MINIMUM_VISIBLE_CELLS);
|
||||
size.height = std::max(size.height, MINIMUM_VISIBLE_CELLS);
|
||||
|
||||
// If this function succeeds with S_FALSE, then the terminal didn't
|
||||
// actually change size. No need to notify the connection of this
|
||||
@@ -575,8 +575,8 @@ try
|
||||
const auto lock = publicTerminal->_terminal->LockForReading();
|
||||
const auto viewInCharacters = publicTerminal->_renderEngine->GetViewportInCharacters(viewInPixels);
|
||||
|
||||
dimensions->width = viewInCharacters.Width();
|
||||
dimensions->height = viewInCharacters.Height();
|
||||
dimensions->width = std::max(viewInCharacters.Width(), MINIMUM_VISIBLE_CELLS);
|
||||
dimensions->height = std::max(viewInCharacters.Height(), MINIMUM_VISIBLE_CELLS);
|
||||
|
||||
return S_OK;
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
#include "TermControl.h"
|
||||
|
||||
#include <LibraryResources.h>
|
||||
#include <DefaultSettings.h>
|
||||
#include <inputpaneinterop.h>
|
||||
|
||||
#include "TermControlAutomationPeer.h"
|
||||
@@ -2840,16 +2841,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
int32_t commandlineRows)
|
||||
{
|
||||
// If the settings have negative or zero row or column counts, ignore those counts.
|
||||
// Floor at MINIMUM_VISIBLE_CELLS so wt --size 1,1 / initialCols:1 cannot
|
||||
// open a 1-cell viewport (GH#19996).
|
||||
// (The lower TerminalCore layer also has upper bounds as well, but at this layer
|
||||
// we may eventually impose different ones depending on how many pixels we can address.)
|
||||
const auto cols = static_cast<float>(std::max(commandlineCols > 0 ?
|
||||
commandlineCols :
|
||||
settings.InitialCols(),
|
||||
1));
|
||||
MINIMUM_VISIBLE_CELLS));
|
||||
const auto rows = static_cast<float>(std::max(commandlineRows > 0 ?
|
||||
commandlineRows :
|
||||
settings.InitialRows(),
|
||||
1));
|
||||
MINIMUM_VISIBLE_CELLS));
|
||||
|
||||
const winrt::Windows::Foundation::Size initialSize{ cols, rows };
|
||||
|
||||
@@ -2941,8 +2944,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
// - a size containing the requested dimensions in pixels.
|
||||
winrt::Windows::Foundation::Size TermControl::GetNewDimensions(const winrt::Windows::Foundation::Size& sizeInChars)
|
||||
{
|
||||
const auto cols = ::base::saturated_cast<int32_t>(sizeInChars.Width);
|
||||
const auto rows = ::base::saturated_cast<int32_t>(sizeInChars.Height);
|
||||
const auto cols = std::max(::base::saturated_cast<int32_t>(sizeInChars.Width), MINIMUM_VISIBLE_CELLS);
|
||||
const auto rows = std::max(::base::saturated_cast<int32_t>(sizeInChars.Height), MINIMUM_VISIBLE_CELLS);
|
||||
const auto fontSize = _core.FontSize();
|
||||
const auto scrollState = _core.Settings().ScrollState();
|
||||
const auto padding = _core.Settings().Padding();
|
||||
@@ -2983,20 +2986,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation
|
||||
|
||||
// Method Description:
|
||||
// - Get the absolute minimum size that this control can be resized to and
|
||||
// still have 1x1 character visible. This includes the space needed for
|
||||
// still have 2x2 characters visible. This includes the space needed for
|
||||
// the scrollbar and the padding.
|
||||
// 2x2 is the VT theoretical minimum (DECSTBM / DECSLRM). A 1-cell
|
||||
// viewport can hang TextBuffer::Reflow on a wide glyph (GH#19996).
|
||||
// Arguments:
|
||||
// - <none>
|
||||
// Return Value:
|
||||
// - The minimum size that this terminal control can be resized to and still
|
||||
// have a visible character.
|
||||
// have a usable character grid.
|
||||
winrt::Windows::Foundation::Size TermControl::MinimumSize()
|
||||
{
|
||||
if (_initializedTerminal)
|
||||
{
|
||||
const auto fontSize = _core.FontSizeInDips();
|
||||
auto width = fontSize.Width;
|
||||
auto height = fontSize.Height;
|
||||
auto width = fontSize.Width * MINIMUM_VISIBLE_CELLS;
|
||||
auto height = fontSize.Height * MINIMUM_VISIBLE_CELLS;
|
||||
// Reserve additional space if scrollbar is intended to be visible
|
||||
if (_core.Settings().ScrollState() != ScrollbarState::Hidden)
|
||||
{
|
||||
|
||||
@@ -42,6 +42,8 @@ Terminal::Terminal(TestDummyMarker) :
|
||||
|
||||
void Terminal::Create(til::size viewportSize, til::CoordType scrollbackLines, Renderer& renderer)
|
||||
{
|
||||
viewportSize.width = std::max(viewportSize.width, MINIMUM_VISIBLE_CELLS);
|
||||
viewportSize.height = std::max(viewportSize.height, MINIMUM_VISIBLE_CELLS);
|
||||
_mutableViewport = Viewport::FromDimensions({ 0, 0 }, viewportSize);
|
||||
_scrollbackLines = scrollbackLines;
|
||||
const til::size bufferSize{ viewportSize.width,
|
||||
@@ -75,8 +77,8 @@ void Terminal::HardResetWithoutErase()
|
||||
void Terminal::CreateFromSettings(ICoreSettings settings,
|
||||
Renderer& renderer)
|
||||
{
|
||||
const til::size viewportSize{ Utils::ClampToShortMax(settings.InitialCols(), 1),
|
||||
Utils::ClampToShortMax(settings.InitialRows(), 1) };
|
||||
const til::size viewportSize{ Utils::ClampToShortMax(settings.InitialCols(), MINIMUM_VISIBLE_CELLS),
|
||||
Utils::ClampToShortMax(settings.InitialRows(), MINIMUM_VISIBLE_CELLS) };
|
||||
|
||||
// TODO:MSFT:20642297 - Support infinite scrollback here, if HistorySize is -1
|
||||
Create(viewportSize, Utils::ClampToShortMax(settings.HistorySize(), 0), renderer);
|
||||
@@ -275,9 +277,14 @@ std::wstring_view Terminal::GetWorkingDirectory() noexcept
|
||||
// - S_OK if we successfully resized the terminal, S_FALSE if there was
|
||||
// nothing to do (the viewportSize is the same as our current size), or an
|
||||
// appropriate HRESULT for failing to resize.
|
||||
[[nodiscard]] HRESULT Terminal::UserResize(const til::size viewportSize) noexcept
|
||||
[[nodiscard]] HRESULT Terminal::UserResize(const til::size requestedSize) noexcept
|
||||
try
|
||||
{
|
||||
const til::size viewportSize{
|
||||
std::max(requestedSize.width, MINIMUM_VISIBLE_CELLS),
|
||||
std::max(requestedSize.height, MINIMUM_VISIBLE_CELLS)
|
||||
};
|
||||
|
||||
const auto oldDimensions = _GetMutableViewport().Dimensions();
|
||||
if (viewportSize == oldDimensions)
|
||||
{
|
||||
|
||||
@@ -28,7 +28,7 @@
|
||||
TargetType="muxc:NumberBox">
|
||||
<Setter Property="SmallChange" Value="1" />
|
||||
<Setter Property="LargeChange" Value="10" />
|
||||
<Setter Property="Minimum" Value="1" />
|
||||
<Setter Property="Minimum" Value="2" />
|
||||
</Style>
|
||||
<Style x:Key="LaunchPositionNumberBoxStyle"
|
||||
BasedOn="{StaticResource NumberBoxSettingStyle}"
|
||||
@@ -213,7 +213,7 @@
|
||||
Grid.Column="1"
|
||||
VerticalAlignment="Center"
|
||||
Maximum="999"
|
||||
Minimum="1"
|
||||
Minimum="2"
|
||||
Style="{StaticResource LaunchSizeNumberBoxStyle}"
|
||||
Value="{x:Bind ViewModel.InitialCols, Mode=TwoWay}" />
|
||||
<TextBlock x:Uid="Globals_InitialRows"
|
||||
@@ -226,7 +226,7 @@
|
||||
Grid.Column="1"
|
||||
VerticalAlignment="Center"
|
||||
Maximum="999"
|
||||
Minimum="1"
|
||||
Minimum="2"
|
||||
Style="{StaticResource LaunchSizeNumberBoxStyle}"
|
||||
Value="{x:Bind ViewModel.InitialRows, Mode=TwoWay}" />
|
||||
</Grid>
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
|
||||
#include "pch.h"
|
||||
#include "GlobalAppSettings.h"
|
||||
#include <DefaultSettings.h>
|
||||
#include "../../types/inc/Utils.hpp"
|
||||
#include "JsonUtils.h"
|
||||
#include "KeyChordSerialization.h"
|
||||
@@ -182,11 +183,11 @@ void GlobalAppSettings::LayerJson(const Json::Value& json, const OriginTag origi
|
||||
// otherwise we could end up setting defaults that get persisted
|
||||
if (this->HasInitialCols())
|
||||
{
|
||||
this->InitialCols(std::clamp(this->InitialCols(), 1, 999));
|
||||
this->InitialCols(std::clamp(this->InitialCols(), MINIMUM_VISIBLE_CELLS, 999));
|
||||
}
|
||||
if (this->HasInitialRows())
|
||||
{
|
||||
this->InitialRows(std::clamp(this->InitialRows(), 1, 999));
|
||||
this->InitialRows(std::clamp(this->InitialRows(), MINIMUM_VISIBLE_CELLS, 999));
|
||||
}
|
||||
LayerActionsFrom(json, origin, true);
|
||||
|
||||
|
||||
@@ -1274,7 +1274,7 @@ namespace SettingsModelUnitTests
|
||||
const auto settings = createSettings(inputSettings);
|
||||
|
||||
VERIFY_ARE_EQUAL(999, settings->GlobalSettings().InitialCols());
|
||||
VERIFY_ARE_EQUAL(1, settings->GlobalSettings().InitialRows());
|
||||
VERIFY_ARE_EQUAL(MINIMUM_VISIBLE_CELLS, settings->GlobalSettings().InitialRows());
|
||||
}
|
||||
|
||||
void DeserializationTests::TestTrailingCommas()
|
||||
|
||||
@@ -36,21 +36,21 @@ using namespace TerminalCoreUnitTests;
|
||||
void ScreenSizeLimitsTest::ScreenWidthAndHeightAreClampedToBounds()
|
||||
{
|
||||
// Negative values for initial visible row count or column count
|
||||
// are clamped to 1. Too-large positive values are clamped to SHRT_MAX.
|
||||
// are clamped to MINIMUM_VISIBLE_CELLS. Too-large positive values are clamped to SHRT_MAX.
|
||||
auto negativeColumnsSettings = winrt::make<MockTermSettings>(10000, 9999999, -1234);
|
||||
Terminal negativeColumnsTerminal{ Terminal::TestDummyMarker{} };
|
||||
DummyRenderer renderer{ &negativeColumnsTerminal };
|
||||
negativeColumnsTerminal.CreateFromSettings(negativeColumnsSettings, renderer);
|
||||
auto actualDimensions = negativeColumnsTerminal.GetViewport().Dimensions();
|
||||
VERIFY_ARE_EQUAL(actualDimensions.height, SHRT_MAX, L"Row count clamped to SHRT_MAX == " WCS(SHRT_MAX));
|
||||
VERIFY_ARE_EQUAL(actualDimensions.width, 1, L"Column count clamped to 1");
|
||||
VERIFY_ARE_EQUAL(actualDimensions.width, MINIMUM_VISIBLE_CELLS, L"Column count clamped to MINIMUM_VISIBLE_CELLS");
|
||||
|
||||
// Zero values are clamped to 1 as well.
|
||||
// Zero values are clamped to MINIMUM_VISIBLE_CELLS as well.
|
||||
auto zeroRowsSettings = winrt::make<MockTermSettings>(10000, 0, 9999999);
|
||||
Terminal zeroRowsTerminal{ Terminal::TestDummyMarker{} };
|
||||
zeroRowsTerminal.CreateFromSettings(zeroRowsSettings, renderer);
|
||||
actualDimensions = zeroRowsTerminal.GetViewport().Dimensions();
|
||||
VERIFY_ARE_EQUAL(actualDimensions.height, 1, L"Row count clamped to 1");
|
||||
VERIFY_ARE_EQUAL(actualDimensions.height, MINIMUM_VISIBLE_CELLS, L"Row count clamped to MINIMUM_VISIBLE_CELLS");
|
||||
VERIFY_ARE_EQUAL(actualDimensions.width, SHRT_MAX, L"Column count clamped to SHRT_MAX == " WCS(SHRT_MAX));
|
||||
}
|
||||
|
||||
|
||||
@@ -225,8 +225,8 @@ namespace Microsoft.Terminal.Wpf
|
||||
|
||||
NativeMethods.TilSize dimensions = new NativeMethods.TilSize
|
||||
{
|
||||
X = (int)columns,
|
||||
Y = (int)rows,
|
||||
X = Math.Max((int)columns, 2),
|
||||
Y = Math.Max((int)rows, 2),
|
||||
};
|
||||
|
||||
NativeMethods.TerminalTriggerResizeWithDimension(this.terminal, dimensions, out var dimensionsInPixels);
|
||||
|
||||
@@ -33,6 +33,10 @@ constexpr uint16_t DEFAULT_FONT_WEIGHT = 400; // normal
|
||||
constexpr int DEFAULT_ROWS = 30;
|
||||
constexpr int DEFAULT_COLS = 120;
|
||||
|
||||
// VT theoretical minimum (DECSTBM / DECSLRM). A 1-cell viewport can hang
|
||||
// TextBuffer::Reflow on a wide glyph (GH#19996).
|
||||
constexpr int MINIMUM_VISIBLE_CELLS = 2;
|
||||
|
||||
constexpr std::wstring_view DEFAULT_PADDING{ L"8, 8, 8, 8" };
|
||||
constexpr std::wstring_view DEFAULT_STARTING_DIRECTORY{ L"%USERPROFILE%" };
|
||||
|
||||
|
||||
Reference in new issue
Block a user