Skip to content

Commit

Permalink
Allow FontInfo{,Base,Desired} to store a font name > 32 wch (#3107)
Browse files Browse the repository at this point in the history
We now truncate the font name as it goes out to GDI APIs, in console API
servicing, and in the propsheet.

I attempted to defer truncating the font to as far up the stack as
possible, so as to make FontInfo usable for the broadest set of cases.

There were a couple questions that came up: I know that `Settings` gets
memset (memsat?) by the registry deserializer, and perhaps that's
another place for us to tackle. Right now, this pull request enables
fonts whose names are >= 32 characters _in Windows Terminal only_, but
the underpinnings are there for conhost as well. We'd need to explicitly
break at the API, or perhaps return a failure or log something to
telemetry.

* Should we log truncation at the API boundary to telemetry?
-> Later; followup filed (#3123)

* Should we fix Settings here, or later?
-> Later; followup filed (#3123)

* `TrueTypeFontList` is built out of things in winconp, the private
console header. Concern about interop structures.
-> Not used for interop, followup filed to clean it up (#3123)

* Is `unsigned int` right for codepage? For width?
-> Yes: codepage became UINT (from WORD) when we moved from Win16 to
Win32

This commit also includes a workaround for #3170. Growing
CONSOLE_INFORMATION made us lose the struct layout lottery during
release builds, and this was an expedient fix.

Closes #602.
Related to #3123.
  • Loading branch information
DHowett committed Oct 15, 2019
1 parent b9233c0 commit b664761
Show file tree
Hide file tree
Showing 24 changed files with 154 additions and 158 deletions.
8 changes: 2 additions & 6 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1065,14 +1065,10 @@ const TextBuffer::TextAndColor TextBuffer::GetTextForClipboard(const bool lineSe
// - htmlTitle - value used in title tag of html header. Used to name the application
// Return Value:
// - string containing the generated HTML
std::string TextBuffer::GenHTML(const TextAndColor& rows, const int fontHeightPoints, const PCWCHAR fontFaceName, const COLORREF backgroundColor, const std::string& htmlTitle)
std::string TextBuffer::GenHTML(const TextAndColor& rows, const int fontHeightPoints, const std::wstring_view fontFaceName, const COLORREF backgroundColor, const std::string& htmlTitle)
{
try
{
// TODO: GH 602 the font name needs to be passed and stored around as an actual bounded type, not an implicit bounds on LF_FACESIZE
const auto faceLength = wcsnlen_s(fontFaceName, LF_FACESIZE);
const std::wstring_view faceNameView{ fontFaceName, faceLength };

std::ostringstream htmlBuilder;

// First we have to add some standard
Expand All @@ -1096,7 +1092,7 @@ std::string TextBuffer::GenHTML(const TextAndColor& rows, const int fontHeightPo

htmlBuilder << "font-family:";
htmlBuilder << "'";
htmlBuilder << ConvertToA(CP_UTF8, faceNameView);
htmlBuilder << ConvertToA(CP_UTF8, fontFaceName);
htmlBuilder << "',";
// even with different font, add monospace as fallback
htmlBuilder << "monospace;";
Expand Down
2 changes: 1 addition & 1 deletion src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class TextBuffer final

static std::string GenHTML(const TextAndColor& rows,
const int fontHeightPoints,
const PCWCHAR fontFaceName,
const std::wstring_view fontFaceName,
const COLORREF backgroundColor,
const std::string& htmlTitle);

Expand Down
4 changes: 2 additions & 2 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ void ApiRoutines::GetNumberOfConsoleMouseButtonsImpl(ULONG& buttons) noexcept
consoleFontInfoEx.FontFamily = fontInfo.GetFamily();
consoleFontInfoEx.FontWeight = fontInfo.GetWeight();

RETURN_IF_FAILED(StringCchCopyW(consoleFontInfoEx.FaceName, ARRAYSIZE(consoleFontInfoEx.FaceName), fontInfo.GetFaceName()));
RETURN_IF_FAILED(fontInfo.FillLegacyNameBuffer(gsl::make_span(consoleFontInfoEx.FaceName)));

return S_OK;
}
Expand Down Expand Up @@ -300,7 +300,7 @@ void ApiRoutines::GetNumberOfConsoleMouseButtonsImpl(ULONG& buttons) noexcept
RETURN_IF_FAILED(StringCchCopyW(FaceName, ARRAYSIZE(FaceName), consoleFontInfoEx.FaceName));

FontInfo fi(FaceName,
static_cast<BYTE>(consoleFontInfoEx.FontFamily),
gsl::narrow_cast<unsigned char>(consoleFontInfoEx.FontFamily),
consoleFontInfoEx.FontWeight,
consoleFontInfoEx.dwFontSize,
gci.OutputCP);
Expand Down
2 changes: 1 addition & 1 deletion src/host/output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ using namespace Microsoft::Console::Interactivity;
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();

FontInfo fiFont(gci.GetFaceName(),
static_cast<BYTE>(gci.GetFontFamily()),
gsl::narrow_cast<unsigned char>(gci.GetFontFamily()),
gci.GetFontWeight(),
gci.GetFontSize(),
gci.GetCodePage());
Expand Down
12 changes: 8 additions & 4 deletions src/host/renderFontDefaults.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ RenderFontDefaults::~RenderFontDefaults()
LOG_IF_FAILED(TrueTypeFontList::s_Destroy());
}

[[nodiscard]] HRESULT RenderFontDefaults::RetrieveDefaultFontNameForCodepage(const UINT uiCodePage,
_Out_writes_(cchFaceName) PWSTR pwszFaceName,
const size_t cchFaceName)
[[nodiscard]] HRESULT RenderFontDefaults::RetrieveDefaultFontNameForCodepage(const unsigned int codePage,
std::wstring& outFaceName)
try
{
NTSTATUS status = TrueTypeFontList::s_SearchByCodePage(uiCodePage, pwszFaceName, cchFaceName);
// GH#3123: Propagate font length changes up through Settings and propsheet
wchar_t faceName[LF_FACESIZE]{ 0 };
NTSTATUS status = TrueTypeFontList::s_SearchByCodePage(codePage, faceName, ARRAYSIZE(faceName));
outFaceName.assign(faceName);
return HRESULT_FROM_NT(status);
}
CATCH_RETURN();
5 changes: 2 additions & 3 deletions src/host/renderFontDefaults.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class RenderFontDefaults sealed : public Microsoft::Console::Render::IFontDefaul
RenderFontDefaults();
~RenderFontDefaults();

[[nodiscard]] HRESULT RetrieveDefaultFontNameForCodepage(const UINT uiCodePage,
_Out_writes_(cchFaceName) PWSTR pwszFaceName,
const size_t cchFaceName);
[[nodiscard]] HRESULT RetrieveDefaultFontNameForCodepage(const unsigned int codePage,
std::wstring& outFaceName);
};
7 changes: 1 addition & 6 deletions src/host/selection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
using namespace Microsoft::Console::Interactivity;
using namespace Microsoft::Console::Types;

std::unique_ptr<Selection> Selection::_instance;

Selection::Selection() :
_fSelectionVisible(false),
_ulSavedCursorSize(0),
Expand All @@ -32,10 +30,7 @@ Selection::Selection() :

Selection& Selection::Instance()
{
if (!_instance)
{
_instance.reset(new Selection());
}
static std::unique_ptr<Selection> _instance{ new Selection() };
return *_instance;
}

Expand Down
2 changes: 0 additions & 2 deletions src/host/selection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ class Selection
void _CancelMarkSelection();
void _CancelMouseSelection();

static std::unique_ptr<Selection> _instance;

// -------------------------------------------------------------------------------------------------------
// input handling (selectionInput.cpp)
public:
Expand Down
14 changes: 7 additions & 7 deletions src/host/settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Settings::Settings() :
_fCtrlKeyShortcutsDisabled(false),
_bWindowAlpha(BYTE_MAX), // 255 alpha = opaque. 0 = transparent.
_fFilterOnPaste(false),
_LaunchFaceName{},
_fTrimLeadingZeros(FALSE),
_fEnableColorSelection(FALSE),
_fAllowAltF4Close(true),
Expand Down Expand Up @@ -76,8 +77,6 @@ Settings::Settings() :
ZeroMemory((void*)&_FaceName, sizeof(_FaceName));
wcscpy_s(_FaceName, DEFAULT_TT_FONT_FACENAME);

ZeroMemory((void*)&_LaunchFaceName, sizeof(_LaunchFaceName));

_CursorColor = Cursor::s_InvertCursorColor;
_CursorType = CursorType::Legacy;

Expand Down Expand Up @@ -404,13 +403,13 @@ void Settings::SetFilterOnPaste(const bool fFilterOnPaste)
_fFilterOnPaste = fFilterOnPaste;
}

const WCHAR* const Settings::GetLaunchFaceName() const
const std::wstring_view Settings::GetLaunchFaceName() const
{
return _LaunchFaceName;
}
void Settings::SetLaunchFaceName(_In_ PCWSTR const LaunchFaceName, const size_t cchLength)
void Settings::SetLaunchFaceName(const std::wstring_view launchFaceName)
{
StringCchCopyW(_LaunchFaceName, cchLength, LaunchFaceName);
_LaunchFaceName = launchFaceName;
}

UINT Settings::GetCodePage() const
Expand Down Expand Up @@ -630,9 +629,10 @@ const WCHAR* const Settings::GetFaceName() const
{
return _FaceName;
}
void Settings::SetFaceName(_In_ PCWSTR const pcszFaceName, const size_t cchLength)
void Settings::SetFaceName(const std::wstring_view faceName)
{
StringCchCopyW(_FaceName, cchLength, pcszFaceName);
auto extent = std::min<size_t>(faceName.size(), ARRAYSIZE(_FaceName));
StringCchCopyW(_FaceName, extent, faceName.data());
}

UINT Settings::GetCursorSize() const
Expand Down
8 changes: 4 additions & 4 deletions src/host/settings.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class Settings
bool GetFilterOnPaste() const;
void SetFilterOnPaste(const bool fFilterOnPaste);

const WCHAR* const GetLaunchFaceName() const;
void SetLaunchFaceName(_In_ PCWSTR const LaunchFaceName, const size_t cchLength);
const std::wstring_view GetLaunchFaceName() const;
void SetLaunchFaceName(const std::wstring_view launchFaceName);

UINT GetCodePage() const;
void SetCodePage(const UINT uCodePage);
Expand Down Expand Up @@ -130,7 +130,7 @@ class Settings

const WCHAR* const GetFaceName() const;
bool IsFaceNameSet() const;
void SetFaceName(_In_ PCWSTR const pcszFaceName, const size_t cchLength);
void SetFaceName(const std::wstring_view faceName);

UINT GetCursorSize() const;
void SetCursorSize(const UINT uCursorSize);
Expand Down Expand Up @@ -226,7 +226,7 @@ class Settings
BYTE _bWindowAlpha; // describes the opacity of the window

bool _fFilterOnPaste; // should we filter text when the user pastes? (e.g. remove <tab>)
WCHAR _LaunchFaceName[LF_FACESIZE];
std::wstring _LaunchFaceName;
bool _fAllowAltF4Close;
DWORD _dwVirtTermLevel;
bool _fAutoReturnOnNewline;
Expand Down
4 changes: 2 additions & 2 deletions src/host/srvinit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const UINT CONSOLE_LPC_PORT_FAILURE_ID = 21791;

Globals.pFontDefaultList = new RenderFontDefaults();

FontInfo::s_SetFontDefaultList(Globals.pFontDefaultList);
FontInfoBase::s_SetFontDefaultList(Globals.pFontDefaultList);
}
CATCH_RETURN();

Expand Down Expand Up @@ -187,7 +187,7 @@ static bool s_IsOnDesktop()
//Save initial font name for comparison on exit. We want telemetry when the font has changed
if (settings.IsFaceNameSet())
{
settings.SetLaunchFaceName(settings.GetFaceName(), LF_FACESIZE);
settings.SetLaunchFaceName(settings.GetFaceName());
}

// Allocate console will read the global ServiceLocator::LocateGlobals().getConsoleInformation
Expand Down
2 changes: 1 addition & 1 deletion src/host/telemetry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ void Telemetry::WriteFinalTraceLog()
TraceLoggingBool(gci.GetEnableColorSelection(), "EnabledColorSelection"),
TraceLoggingBool(gci.GetFilterOnPaste(), "FilterOnPaste"),
TraceLoggingBool(gci.GetTrimLeadingZeros(), "TrimLeadingZeros"),
TraceLoggingValue(gci.GetLaunchFaceName(), "LaunchFontName"),
TraceLoggingValue(gci.GetLaunchFaceName().data(), "LaunchFontName"),
TraceLoggingValue(CommandHistory::s_CountOfHistories(), "CommandHistoriesNumber"),
TraceLoggingValue(gci.GetCodePage(), "CodePage"),
TraceLoggingValue(gci.GetCursorSize(), "CursorSize"),
Expand Down
4 changes: 3 additions & 1 deletion src/interactivity/onecore/SystemConfigurationProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

#include "SystemConfigurationProvider.hpp"

static constexpr wchar_t DEFAULT_TT_FONT_FACENAME[]{ L"__DefaultTTFont__" };

using namespace Microsoft::Console::Interactivity::OneCore;

UINT SystemConfigurationProvider::GetCaretBlinkTime()
Expand Down Expand Up @@ -60,7 +62,7 @@ void SystemConfigurationProvider::GetSettingsFromLink(
// Hence, we make it seem like the console is in fact configred to use a
// TrueType font by the user.

pLinkSettings->SetFaceName(DEFAULT_TT_FONT_FACENAME, ARRAYSIZE(DEFAULT_TT_FONT_FACENAME));
pLinkSettings->SetFaceName(DEFAULT_TT_FONT_FACENAME);
pLinkSettings->SetFontFamily(TMPF_TRUETYPE);

return;
Expand Down
4 changes: 0 additions & 4 deletions src/interactivity/onecore/SystemConfigurationProvider.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ Author(s):

#pragma hdrstop

#ifndef DEFAULT_TT_FONT_FACENAME
#define DEFAULT_TT_FONT_FACENAME L"__DefaultTTFont__"
#endif

class InputTests;

namespace Microsoft::Console::Interactivity::OneCore
Expand Down
6 changes: 3 additions & 3 deletions src/interactivity/win32/menu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ void Menu::s_ShowPropertiesDialog(HWND const hwnd, BOOL const Defaults)
pStateInfo->FontFamily = currentFont.GetFamily();
pStateInfo->FontSize = currentFont.GetUnscaledSize();
pStateInfo->FontWeight = currentFont.GetWeight();
StringCchCopyW(pStateInfo->FaceName, ARRAYSIZE(pStateInfo->FaceName), currentFont.GetFaceName());
LOG_IF_FAILED(StringCchCopyW(pStateInfo->FaceName, ARRAYSIZE(pStateInfo->FaceName), currentFont.GetFaceName().data()));

const Cursor& cursor = ScreenInfo.GetTextBuffer().GetCursor();
pStateInfo->CursorSize = cursor.GetSize();
Expand Down Expand Up @@ -447,7 +447,7 @@ void Menu::s_PropertiesUpdate(PCONSOLE_STATE_INFO pStateInfo)
// end V2 console properties

// Apply font information (must come before all character calculations for window/buffer size).
FontInfo fiNewFont(pStateInfo->FaceName, static_cast<BYTE>(pStateInfo->FontFamily), pStateInfo->FontWeight, pStateInfo->FontSize, pStateInfo->CodePage);
FontInfo fiNewFont(pStateInfo->FaceName, gsl::narrow_cast<unsigned char>(pStateInfo->FontFamily), pStateInfo->FontWeight, pStateInfo->FontSize, pStateInfo->CodePage);

ScreenInfo.UpdateFont(&fiNewFont);

Expand All @@ -457,7 +457,7 @@ void Menu::s_PropertiesUpdate(PCONSOLE_STATE_INFO pStateInfo)
gci.SetFontFamily(fontApplied.GetFamily());
gci.SetFontSize(fontApplied.GetUnscaledSize());
gci.SetFontWeight(fontApplied.GetWeight());
gci.SetFaceName(fontApplied.GetFaceName(), LF_FACESIZE);
gci.SetFaceName(fontApplied.GetFaceName());

// Set the cursor properties in the Settings
const auto cursorType = static_cast<CursorType>(pStateInfo->CursorType);
Expand Down
Loading

0 comments on commit b664761

Please sign in to comment.