Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2849,13 +2849,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())
Expand Down
26 changes: 13 additions & 13 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,12 +414,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)
Expand All @@ -430,10 +430,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
}

// Override the default width and height to match the size of the swapChainPanel
const til::size viewportSize{ Utils::ClampToShortMax(width, 1),
Utils::ClampToShortMax(height, 1) };

// TODO:MSFT:20642297 - Support infinite scrollback here, if HistorySize is -1
_terminal->Create(viewportSize, Utils::ClampToShortMax(_settings.HistorySize(), 0), *_renderer);
_terminal->UpdateSettings(_settings);
Expand Down Expand Up @@ -1228,10 +1224,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 });
Expand All @@ -1247,15 +1245,17 @@ 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;
}

if (_connection)
{
_connection.Resize(vp.Height(), vp.Width());
_connection.Resize(rows, cols);
}

// TermControl will call Search() once the OutputIdle even fires after 100ms.
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalControl/HwndTerminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 13 additions & 8 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "pch.h"
#include "TermControl.h"

#include <DefaultSettings.h>
#include <inputpaneinterop.h>

#include "TermControlAutomationPeer.h"
Expand Down Expand Up @@ -2741,16 +2742,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 };

Expand Down Expand Up @@ -2842,8 +2845,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();
Expand Down Expand Up @@ -2884,20 +2887,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)
{
Expand Down
13 changes: 10 additions & 3 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -289,9 +291,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)
{
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalSettingsEditor/Launch.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -219,7 +219,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"
Expand All @@ -232,7 +232,7 @@
Grid.Column="1"
VerticalAlignment="Center"
Maximum="999"
Minimum="1"
Minimum="2"
Style="{StaticResource LaunchSizeNumberBoxStyle}"
Value="{x:Bind ViewModel.InitialRows, Mode=TwoWay}" />
</Grid>
Expand Down
5 changes: 3 additions & 2 deletions src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "pch.h"
#include "GlobalAppSettings.h"
#include <DefaultSettings.h>
#include "../../types/inc/Utils.hpp"
#include "JsonUtils.h"
#include "KeyChordSerialization.h"
Expand Down Expand Up @@ -191,11 +192,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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,7 @@ namespace SettingsModelUnitTests
const auto settings = createSettings(inputSettings);

VERIFY_ARE_EQUAL(999, settings->WindowSettingsDefaults().InitialCols());
VERIFY_ARE_EQUAL(1, settings->WindowSettingsDefaults().InitialRows());
VERIFY_ARE_EQUAL(MINIMUM_VISIBLE_CELLS, settings->WindowSettingsDefaults().InitialRows());
}

void DeserializationTests::TestTrailingCommas()
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
4 changes: 4 additions & 0 deletions src/inc/DefaultSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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%" };

Expand Down