Skip to content

Commit

Permalink
Add noexcept to all FontInfo structs (#11640)
Browse files Browse the repository at this point in the history
FontInfoBase and it's descendents are missing noexcept annotations, which
virally forces other code to not be noexcept as well during AuditMode checks.
Apart from adding noexcept, this commit also
* Passes std::wstring_view by reference.
* Pass the FillLegacyNameBuffer argument as a simple pointer-to-array,
  allowing us to fill the buffer with a single memcpy.
  (gsl::span's iterators inhibit any internal STL optimizations.)
* Move operator== declarations inside the class to reduce code size.

All other changes are an effect of the virality of noexcept.

This is an offshoot from #11623.

## Validation Steps Performed
* It still compiles ✔️
  • Loading branch information
lhecker authored Oct 29, 2021
1 parent 1cedac6 commit 95cc7d9
Show file tree
Hide file tree
Showing 19 changed files with 117 additions and 165 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/PublicTerminalCore/HwndTerminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ void HwndTerminal::_StringPaste(const wchar_t* const pData) noexcept
CATCH_LOG();
}

COORD HwndTerminal::GetFontSize() const
COORD HwndTerminal::GetFontSize() const noexcept
{
return _actualFont.GetSize();
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/PublicTerminalCore/HwndTerminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ struct HwndTerminal : ::Microsoft::Console::Types::IControlAccessibilityInfo
void _SendCharEvent(wchar_t ch, WORD scanCode, WORD flags) noexcept;

// Inherited via IControlAccessibilityInfo
COORD GetFontSize() const override;
COORD GetFontSize() const noexcept override;
RECT GetBounds() const noexcept override;
double GetScaleFactor() const noexcept override;
void ChangeViewport(const SMALL_RECT NewWindow) override;
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
#pragma endregion

#pragma region IControlAccessibilityInfo
COORD InteractivityAutomationPeer::GetFontSize() const
COORD InteractivityAutomationPeer::GetFontSize() const noexcept
{
return til::size{ til::math::rounding, _interactivity->Core().FontSize() };
}

RECT InteractivityAutomationPeer::GetBounds() const
RECT InteractivityAutomationPeer::GetBounds() const noexcept
{
return _controlBounds;
}
Expand All @@ -159,12 +159,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return S_OK;
}

RECT InteractivityAutomationPeer::GetPadding() const
RECT InteractivityAutomationPeer::GetPadding() const noexcept
{
return _controlPadding;
}

double InteractivityAutomationPeer::GetScaleFactor() const
double InteractivityAutomationPeer::GetScaleFactor() const noexcept
{
return DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel();
}
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalControl/InteractivityAutomationPeer.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation

#pragma region IControlAccessibilityInfo Pattern
// Inherited via IControlAccessibilityInfo
virtual COORD GetFontSize() const override;
virtual RECT GetBounds() const override;
virtual RECT GetPadding() const override;
virtual double GetScaleFactor() const override;
virtual COORD GetFontSize() const noexcept override;
virtual RECT GetBounds() const noexcept override;
virtual RECT GetPadding() const noexcept override;
virtual double GetScaleFactor() const noexcept override;
virtual void ChangeViewport(SMALL_RECT NewWindow) override;
virtual HRESULT GetHostUiaProvider(IRawElementProviderSimple** provider) override;
#pragma endregion
Expand Down
3 changes: 1 addition & 2 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,7 @@ void ApiRoutines::GetNumberOfConsoleMouseButtonsImpl(ULONG& buttons) noexcept
const FontInfo& fontInfo = activeScreenInfo.GetCurrentFont();
consoleFontInfoEx.FontFamily = fontInfo.GetFamily();
consoleFontInfoEx.FontWeight = fontInfo.GetWeight();

RETURN_IF_FAILED(fontInfo.FillLegacyNameBuffer(gsl::make_span(consoleFontInfoEx.FaceName)));
fontInfo.FillLegacyNameBuffer(consoleFontInfoEx.FaceName);

return S_OK;
}
Expand Down
62 changes: 23 additions & 39 deletions src/renderer/base/FontInfoBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,11 @@

#include "../inc/FontInfoBase.hpp"

bool operator==(const FontInfoBase& a, const FontInfoBase& b)
{
return a._faceName == b._faceName &&
a._weight == b._weight &&
a._family == b._family &&
a._codePage == b._codePage &&
a._fDefaultRasterSetFromEngine == b._fDefaultRasterSetFromEngine;
}

FontInfoBase::FontInfoBase(const std::wstring_view faceName,
FontInfoBase::FontInfoBase(const std::wstring_view& faceName,
const unsigned char family,
const unsigned int weight,
const bool fSetDefaultRasterFont,
const unsigned int codePage) :
const unsigned int codePage) noexcept :
_faceName(faceName),
_family(family),
_weight(weight),
Expand All @@ -30,43 +21,39 @@ FontInfoBase::FontInfoBase(const std::wstring_view faceName,
ValidateFont();
}

FontInfoBase::FontInfoBase(const FontInfoBase& fibFont) :
FontInfoBase(fibFont.GetFaceName(),
fibFont.GetFamily(),
fibFont.GetWeight(),
fibFont.WasDefaultRasterSetFromEngine(),
fibFont.GetCodePage())
{
}

FontInfoBase::~FontInfoBase()
bool FontInfoBase::operator==(const FontInfoBase& other) noexcept
{
return _faceName == other._faceName &&
_weight == other._weight &&
_family == other._family &&
_codePage == other._codePage &&
_fDefaultRasterSetFromEngine == other._fDefaultRasterSetFromEngine;
}

unsigned char FontInfoBase::GetFamily() const
unsigned char FontInfoBase::GetFamily() const noexcept
{
return _family;
}

// When the default raster font is forced set from the engine, this is how we differentiate it from a simple apply.
// Default raster font is internally represented as a blank face name and zeros for weight, family, and size. This is
// the hint for the engine to use whatever comes back from GetStockObject(OEM_FIXED_FONT) (at least in the GDI world).
bool FontInfoBase::WasDefaultRasterSetFromEngine() const
bool FontInfoBase::WasDefaultRasterSetFromEngine() const noexcept
{
return _fDefaultRasterSetFromEngine;
}

unsigned int FontInfoBase::GetWeight() const
unsigned int FontInfoBase::GetWeight() const noexcept
{
return _weight;
}

const std::wstring_view FontInfoBase::GetFaceName() const noexcept
const std::wstring& FontInfoBase::GetFaceName() const noexcept
{
return _faceName;
}

unsigned int FontInfoBase::GetCodePage() const
unsigned int FontInfoBase::GetCodePage() const noexcept
{
return _codePage;
}
Expand All @@ -77,21 +64,18 @@ unsigned int FontInfoBase::GetCodePage() const
// Arguments:
// - buffer: the buffer into which to copy characters
// - size: the size of buffer
HRESULT FontInfoBase::FillLegacyNameBuffer(gsl::span<wchar_t> buffer) const
try
void FontInfoBase::FillLegacyNameBuffer(wchar_t (&buffer)[LF_FACESIZE]) const noexcept
{
auto toCopy = std::min<size_t>(buffer.size() - 1, _faceName.size());
auto last = std::copy(_faceName.cbegin(), _faceName.cbegin() + toCopy, buffer.begin());
std::fill(last, buffer.end(), L'\0');
return S_OK;
const auto toCopy = std::min(std::size(buffer) - 1, _faceName.size());
const auto last = std::copy_n(_faceName.data(), toCopy, &buffer[0]);
*last = L'\0';
}
CATCH_RETURN();

// NOTE: this method is intended to only be used from the engine itself to respond what font it has chosen.
void FontInfoBase::SetFromEngine(const std::wstring_view faceName,
void FontInfoBase::SetFromEngine(const std::wstring_view& faceName,
const unsigned char family,
const unsigned int weight,
const bool fSetDefaultRasterFont)
const bool fSetDefaultRasterFont) noexcept
{
_faceName = faceName;
_family = family;
Expand All @@ -101,12 +85,12 @@ void FontInfoBase::SetFromEngine(const std::wstring_view faceName,

// Internally, default raster font is represented by empty facename, and zeros for weight, family, and size. Since
// FontInfoBase doesn't have sizing information, this helper checks everything else.
bool FontInfoBase::IsDefaultRasterFontNoSize() const
bool FontInfoBase::IsDefaultRasterFontNoSize() const noexcept
{
return (_weight == 0 && _family == 0 && _faceName.empty());
}

void FontInfoBase::ValidateFont()
void FontInfoBase::ValidateFont() noexcept
{
// If we were given a blank name, it meant raster fonts, which to us is always Terminal.
if (!IsDefaultRasterFontNoSize() && s_pFontDefaultList != nullptr)
Expand All @@ -128,14 +112,14 @@ void FontInfoBase::ValidateFont()
}
}

bool FontInfoBase::IsTrueTypeFont() const
bool FontInfoBase::IsTrueTypeFont() const noexcept
{
return WI_IsFlagSet(_family, TMPF_TRUETYPE);
}

Microsoft::Console::Render::IFontDefaultList* FontInfoBase::s_pFontDefaultList;

void FontInfoBase::s_SetFontDefaultList(_In_ Microsoft::Console::Render::IFontDefaultList* const pFontDefaultList)
void FontInfoBase::s_SetFontDefaultList(_In_ Microsoft::Console::Render::IFontDefaultList* const pFontDefaultList) noexcept
{
s_pFontDefaultList = pFontDefaultList;
}
46 changes: 23 additions & 23 deletions src/renderer/base/FontInfoDesired.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,47 @@

#include "../inc/FontInfoDesired.hpp"

bool operator==(const FontInfoDesired& a, const FontInfoDesired& b)
{
return (static_cast<FontInfoBase>(a) == static_cast<FontInfoBase>(b) &&
a._coordSizeDesired == b._coordSizeDesired);
}

COORD FontInfoDesired::GetEngineSize() const
{
COORD coordSize = _coordSizeDesired;
if (IsTrueTypeFont())
{
coordSize.X = 0; // Don't tell the engine about the width for a TrueType font. It makes a mess.
}

return coordSize;
}

FontInfoDesired::FontInfoDesired(const std::wstring_view faceName,
FontInfoDesired::FontInfoDesired(const std::wstring_view& faceName,
const unsigned char family,
const unsigned int weight,
const COORD coordSizeDesired,
const unsigned int codePage) :
const unsigned int codePage) noexcept :
FontInfoBase(faceName, family, weight, false, codePage),
_coordSizeDesired(coordSizeDesired)
{
}

FontInfoDesired::FontInfoDesired(const FontInfo& fiFont) :
FontInfoDesired::FontInfoDesired(const FontInfo& fiFont) noexcept :
FontInfoBase(fiFont),
_coordSizeDesired(fiFont.GetUnscaledSize())
{
}

bool FontInfoDesired::operator==(const FontInfoDesired& other) noexcept
{
return FontInfoBase::operator==(other) &&
_coordSizeDesired == other._coordSizeDesired;
}

COORD FontInfoDesired::GetEngineSize() const noexcept
{
COORD coordSize = _coordSizeDesired;
if (IsTrueTypeFont())
{
coordSize.X = 0; // Don't tell the engine about the width for a TrueType font. It makes a mess.
}

return coordSize;
}

// This helper determines if this object represents the default raster font. This can either be because internally we're
// using the empty facename and zeros for size, weight, and family, or it can be because we were given explicit
// dimensions from the engine that were the result of loading the default raster font. See GdiEngine::_GetProposedFont().
bool FontInfoDesired::IsDefaultRasterFont() const
bool FontInfoDesired::IsDefaultRasterFont() const noexcept
{
// Either the raster was set from the engine...
// OR the face name is empty with a size of 0x0 or 8x12.
return WasDefaultRasterSetFromEngine() || (GetFaceName().empty() &&
((_coordSizeDesired.X == 0 && _coordSizeDesired.Y == 0) ||
(_coordSizeDesired.X == 8 && _coordSizeDesired.Y == 12)));
(_coordSizeDesired == COORD{ 0, 0 } ||
_coordSizeDesired == COORD{ 8, 12 }));
}
33 changes: 12 additions & 21 deletions src/renderer/base/fontinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,12 @@

#include "../inc/FontInfo.hpp"

bool operator==(const FontInfo& a, const FontInfo& b)
{
return (static_cast<FontInfoBase>(a) == static_cast<FontInfoBase>(b) &&
a._coordSize == b._coordSize &&
a._coordSizeUnscaled == b._coordSizeUnscaled);
}

FontInfo::FontInfo(const std::wstring_view faceName,
FontInfo::FontInfo(const std::wstring_view& faceName,
const unsigned char family,
const unsigned int weight,
const COORD coordSize,
const unsigned int codePage,
const bool fSetDefaultRasterFont /* = false */) :
const bool fSetDefaultRasterFont /* = false */) noexcept :
FontInfoBase(faceName, family, weight, fSetDefaultRasterFont, codePage),
_coordSize(coordSize),
_coordSizeUnscaled(coordSize),
Expand All @@ -26,38 +19,36 @@ FontInfo::FontInfo(const std::wstring_view faceName,
ValidateFont();
}

FontInfo::FontInfo(const FontInfo& fiFont) :
FontInfoBase(fiFont),
_coordSize(fiFont.GetSize()),
_coordSizeUnscaled(fiFont.GetUnscaledSize())
bool FontInfo::operator==(const FontInfo& other) noexcept
{
return FontInfoBase::operator==(other) &&
_coordSize == other._coordSize &&
_coordSizeUnscaled == other._coordSizeUnscaled;
}

COORD FontInfo::GetUnscaledSize() const
COORD FontInfo::GetUnscaledSize() const noexcept
{
return _coordSizeUnscaled;
}

COORD FontInfo::GetSize() const
COORD FontInfo::GetSize() const noexcept
{
return _coordSize;
}

void FontInfo::SetFromEngine(const std::wstring_view faceName,
void FontInfo::SetFromEngine(const std::wstring_view& faceName,
const unsigned char family,
const unsigned int weight,
const bool fSetDefaultRasterFont,
const COORD coordSize,
const COORD coordSizeUnscaled)
const COORD coordSizeUnscaled) noexcept
{
FontInfoBase::SetFromEngine(faceName,
family,
weight,
fSetDefaultRasterFont);

_coordSize = coordSize;
_coordSizeUnscaled = coordSizeUnscaled;

_ValidateCoordSize();
}

Expand All @@ -71,12 +62,12 @@ void FontInfo::SetFallback(const bool didFallback) noexcept
_didFallback = didFallback;
}

void FontInfo::ValidateFont()
void FontInfo::ValidateFont() noexcept
{
_ValidateCoordSize();
}

void FontInfo::_ValidateCoordSize()
void FontInfo::_ValidateCoordSize() noexcept
{
// a (0,0) font is okay for the default raster font, as we will eventually set the dimensions based on the font GDI
// passes back to us.
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/gdi/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ GdiEngine::~GdiEngine()
// NOTE: not using what GDI gave us because some fonts don't quite roundtrip (e.g. MS Gothic and VL Gothic)
lf.lfPitchAndFamily = (FIXED_PITCH | FF_MODERN);

RETURN_IF_FAILED(FontDesired.FillLegacyNameBuffer(gsl::make_span(lf.lfFaceName)));
FontDesired.FillLegacyNameBuffer(lf.lfFaceName);

// Create font.
hFont.reset(CreateFontIndirectW(&lf));
Expand Down
Loading

0 comments on commit 95cc7d9

Please sign in to comment.