From 1b6e6bd6dd03b72e2918a076eea8f701a71455ab Mon Sep 17 00:00:00 2001 From: PankajBhojwani Date: Tue, 24 Aug 2021 18:19:40 -0700 Subject: [PATCH] Fix setting `wght` axis font bugs (#10863) - 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 --- src/renderer/dx/CustomTextLayout.cpp | 11 +++- src/renderer/dx/DxFontRenderData.cpp | 89 +++++++++++++++++++++++++--- src/renderer/dx/DxFontRenderData.h | 7 +++ 3 files changed, 96 insertions(+), 11 deletions(-) diff --git a/src/renderer/dx/CustomTextLayout.cpp b/src/renderer/dx/CustomTextLayout.cpp index bf60bce33..d0b789131 100644 --- a/src/renderer/dx/CustomTextLayout.cpp +++ b/src/renderer/dx/CustomTextLayout.cpp @@ -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; } @@ -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)); diff --git a/src/renderer/dx/DxFontRenderData.cpp b/src/renderer/dx/DxFontRenderData.cpp index fb3e75138..be1047885 100644 --- a/src/renderer/dx/DxFontRenderData.cpp +++ b/src/renderer/dx/DxFontRenderData.cpp @@ -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 @@ -506,17 +537,51 @@ void DxFontRenderData::_SetFeatures(const std::unordered_map& axes) { - _axesVector.clear(); + // Clear out the old vector and booleans in case this is a hot reload + _axesVector = std::vector{}; + _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 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) { - 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 }); + 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) + { + _axesVector.emplace_back(weightAxis.value()); + } + _didUserSetAxes = true; } } @@ -843,10 +908,16 @@ Microsoft::WRL::ComPtr DxFontRenderData::_BuildTextFormat(con // If the OS supports IDWriteTextFormat3, set the font axes ::Microsoft::WRL::ComPtr 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(_axesVector.size())); + if (_inhibitUserWeight && !_axesVectorWithoutWeight.empty()) + { + format3->SetFontAxisValues(_axesVectorWithoutWeight.data(), gsl::narrow(_axesVectorWithoutWeight.size())); + } + else if (!_inhibitUserWeight && !_axesVector.empty()) + { + format3->SetFontAxisValues(_axesVector.data(), gsl::narrow(_axesVector.size())); + } } return format; diff --git a/src/renderer/dx/DxFontRenderData.h b/src/renderer/dx/DxFontRenderData.h index 379fd6036..556fe58f7 100644 --- a/src/renderer/dx/DxFontRenderData.h +++ b/src/renderer/dx/DxFontRenderData.h @@ -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 GetAxisVector(const DWRITE_FONT_WEIGHT fontWeight, const DWRITE_FONT_STRETCH fontStretch, @@ -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 _featureVector; // The font axes to apply to the text std::vector _axesVector; + gsl::span _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