Skip to content

Commit

Permalink
Fix setting wght axis font bugs (#10863)
Browse files Browse the repository at this point in the history
- When deciding whether to call `_AnalyzeFontFallback`, also check if the user set any font axes
- Do not use the user set weight if we are setting the weight due to the bold attribute
- When calling `FontFaceWithAttribute`, check if the user set the italic axis as well as the text attribute

* [x] Closes #10852
* [x] Closes #10853
  • Loading branch information
PankajBhojwani authored Aug 25, 2021
1 parent 7b6df26 commit 1b6e6bd
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 11 deletions.
11 changes: 9 additions & 2 deletions src/renderer/dx/CustomTextLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,16 @@ try
{
// TODO: "relative" bold?
weight = DWRITE_FONT_WEIGHT_BOLD;
// Since we are setting the font weight according to the text attribute,
// make sure to tell the text format to ignore the user set font weight
_fontRenderData->InhibitUserWeight(true);
}
else
{
_fontRenderData->InhibitUserWeight(false);
}

if (drawingContext->useItalicFont)
if (drawingContext->useItalicFont || _fontRenderData->DidUserSetItalic())
{
style = DWRITE_FONT_STYLE_ITALIC;
}
Expand Down Expand Up @@ -236,7 +243,7 @@ CATCH_RETURN()
// Allocate enough room to have one breakpoint per code unit.
_breakpoints.resize(_text.size());

if (!_isEntireTextSimple)
if (!_isEntireTextSimple || _fontRenderData->DidUserSetAxes())
{
// Call each of the analyzers in sequence, recording their results.
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeLineBreakpoints(this, 0, textLength, this));
Expand Down
89 changes: 80 additions & 9 deletions src/renderer/dx/DxFontRenderData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,37 @@ bool DxFontRenderData::DidUserSetFeatures() const noexcept
return _didUserSetFeatures;
}

// Routine Description:
// - Returns whether the user set or updated any of the font axes to be applied
bool DxFontRenderData::DidUserSetAxes() const noexcept
{
return _didUserSetAxes;
}

// Routine Description:
// - Function called to inform us whether to use the user set weight
// in the font axes
// - Called by CustomTextLayout, when the text attribute is bold we should
// ignore the user set weight, otherwise setting the bold font axis
// breaks the bold font attribute
// Arguments:
// - inhibitUserWeight: boolean that tells us if we should use the user set weight
// in the font axes
void DxFontRenderData::InhibitUserWeight(bool inhibitUserWeight) noexcept
{
_inhibitUserWeight = inhibitUserWeight;
}

// Routine Description:
// - Returns whether the set italic in the font axes
// Return Value:
// - True if the user set the italic axis to 1,
// false if the italic axis is not present or the italic axis is set to 0
bool DxFontRenderData::DidUserSetItalic() const noexcept
{
return _didUserSetItalic;
}

// Routine Description:
// - Updates our internal map of font features with the given features
// - NOTE TO CALLER: Make sure to call _BuildFontRenderData after calling this for the feature changes
Expand Down Expand Up @@ -506,17 +537,51 @@ void DxFontRenderData::_SetFeatures(const std::unordered_map<std::wstring_view,
// - axes - the axes to update our map with
void DxFontRenderData::_SetAxes(const std::unordered_map<std::wstring_view, float>& axes)
{
_axesVector.clear();
// Clear out the old vector and booleans in case this is a hot reload
_axesVector = std::vector<DWRITE_FONT_AXIS_VALUE>{};
_didUserSetAxes = false;
_didUserSetItalic = false;

// Update our axis map with the provided axes
#pragma warning(suppress : 26445) // the analyzer doesn't like reference to string_view
for (const auto& [axis, value] : axes)
if (!axes.empty())
{
if (axis.length() == TAG_LENGTH)
// Store the weight aside: we will be creating a span of all the axes in the vector except the weight,
// and then we will add the weight to the vector
// We are doing this so that when the text attribute is bold, we can apply all the axes except the weight
std::optional<DWRITE_FONT_AXIS_VALUE> weightAxis;

// Since we are calling an 'emplace_back' after creating the span,
// there is a chance a reallocation happens (if the vector needs to grow), which would make the span point to
// deallocated memory. To avoid this, make sure to reserve enough memory in the vector.
_axesVector.reserve(axes.size());

#pragma warning(suppress : 26445) // the analyzer doesn't like reference to string_view
for (const auto& [axis, value] : axes)
{
if (axis.length() == TAG_LENGTH)
{
const auto dwriteFontAxis = DWRITE_FONT_AXIS_VALUE{ DWRITE_MAKE_FONT_AXIS_TAG(til::at(axis, 0), til::at(axis, 1), til::at(axis, 2), til::at(axis, 3)), value };
if (dwriteFontAxis.axisTag != DWRITE_FONT_AXIS_TAG_WEIGHT)
{
_axesVector.emplace_back(dwriteFontAxis);
}
else
{
weightAxis = dwriteFontAxis;
}
_didUserSetItalic |= dwriteFontAxis.axisTag == DWRITE_FONT_AXIS_TAG_ITALIC && value == 1;
}
}

// Make the span, which has all the axes except the weight
_axesVectorWithoutWeight = gsl::make_span(_axesVector);

// Add the weight axis to the vector if needed
if (weightAxis)
{
const auto dwriteTag = DWRITE_MAKE_FONT_AXIS_TAG(til::at(axis, 0), til::at(axis, 1), til::at(axis, 2), til::at(axis, 3));
_axesVector.emplace_back(DWRITE_FONT_AXIS_VALUE{ dwriteTag, value });
_axesVector.emplace_back(weightAxis.value());
}
_didUserSetAxes = true;
}
}

Expand Down Expand Up @@ -843,10 +908,16 @@ Microsoft::WRL::ComPtr<IDWriteTextFormat> DxFontRenderData::_BuildTextFormat(con

// If the OS supports IDWriteTextFormat3, set the font axes
::Microsoft::WRL::ComPtr<IDWriteTextFormat3> format3;
if (!_axesVector.empty() && !FAILED(format->QueryInterface(IID_PPV_ARGS(&format3))))
if (!FAILED(format->QueryInterface(IID_PPV_ARGS(&format3))))
{
DWRITE_FONT_AXIS_VALUE const* axesList = _axesVector.data();
format3->SetFontAxisValues(axesList, gsl::narrow<uint32_t>(_axesVector.size()));
if (_inhibitUserWeight && !_axesVectorWithoutWeight.empty())
{
format3->SetFontAxisValues(_axesVectorWithoutWeight.data(), gsl::narrow<uint32_t>(_axesVectorWithoutWeight.size()));
}
else if (!_inhibitUserWeight && !_axesVector.empty())
{
format3->SetFontAxisValues(_axesVector.data(), gsl::narrow<uint32_t>(_axesVector.size()));
}
}

return format;
Expand Down
7 changes: 7 additions & 0 deletions src/renderer/dx/DxFontRenderData.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ namespace Microsoft::Console::Render
[[nodiscard]] static HRESULT STDMETHODCALLTYPE s_CalculateBoxEffect(IDWriteTextFormat* format, size_t widthPixels, IDWriteFontFace1* face, float fontScale, IBoxDrawingEffect** effect) noexcept;

bool DidUserSetFeatures() const noexcept;
bool DidUserSetAxes() const noexcept;
void InhibitUserWeight(bool inhibitUserWeight) noexcept;
bool DidUserSetItalic() const noexcept;

std::vector<DWRITE_FONT_AXIS_VALUE> GetAxisVector(const DWRITE_FONT_WEIGHT fontWeight,
const DWRITE_FONT_STRETCH fontStretch,
Expand All @@ -97,12 +100,16 @@ namespace Microsoft::Console::Render
private:
using FontAttributeMapKey = uint32_t;

bool _inhibitUserWeight{ false };
bool _didUserSetItalic{ false };
bool _didUserSetFeatures{ false };
bool _didUserSetAxes{ false };
// The font features to apply to the text
std::vector<DWRITE_FONT_FEATURE> _featureVector;

// The font axes to apply to the text
std::vector<DWRITE_FONT_AXIS_VALUE> _axesVector;
gsl::span<DWRITE_FONT_AXIS_VALUE> _axesVectorWithoutWeight;

// We use this to identify font variants with different attributes.
static FontAttributeMapKey _ToMapKey(DWRITE_FONT_WEIGHT weight, DWRITE_FONT_STYLE style, DWRITE_FONT_STRETCH stretch) noexcept
Expand Down

0 comments on commit 1b6e6bd

Please sign in to comment.