Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AtlasEngine: Implement remaining grid lines #13587

Merged
5 commits merged into from
Jul 28, 2022
Merged

AtlasEngine: Implement remaining grid lines #13587

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dec1c11
AtlasEngine: Implement remaining grid lines
lhecker Jul 26, 2022
336b275
Add additional comments
lhecker Jul 26, 2022
92a0518
Wording
lhecker Jul 26, 2022
060c9bd
Address feedback
lhecker Jul 26, 2022
8450182
Address feedback
lhecker Jul 28, 2022
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
93 changes: 64 additions & 29 deletions src/renderer/atlas/AtlasEngine.api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ CATCH_RETURN()
DWRITE_TEXT_METRICS metrics;
RETURN_IF_FAILED(textLayout->GetMetrics(&metrics));

*pResult = static_cast<unsigned int>(std::ceil(metrics.width)) > _api.fontMetrics.cellSize.x;
*pResult = static_cast<unsigned int>(std::ceilf(metrics.width)) > _api.fontMetrics.cellSize.x;
return S_OK;
}

Expand Down Expand Up @@ -605,18 +605,51 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo
// Point sizes are commonly treated at a 72 DPI scale
// (including by OpenType), whereas DirectWrite uses 96 DPI.
// Since we want the height in px we multiply by the display's DPI.
const auto fontSizeInPx = std::ceil(requestedSize.Y / 72.0 * _api.dpi);

const auto designUnitsPerPx = fontSizeInPx / static_cast<double>(metrics.designUnitsPerEm);
const auto ascentInPx = static_cast<double>(metrics.ascent) * designUnitsPerPx;
const auto descentInPx = static_cast<double>(metrics.descent) * designUnitsPerPx;
const auto lineGapInPx = static_cast<double>(metrics.lineGap) * designUnitsPerPx;
const auto advanceWidthInPx = static_cast<double>(glyphMetrics.advanceWidth) * designUnitsPerPx;

const auto halfGapInPx = lineGapInPx / 2.0;
const auto baseline = std::ceil(ascentInPx + halfGapInPx);
const auto cellWidth = gsl::narrow<u16>(std::ceil(advanceWidthInPx));
const auto cellHeight = gsl::narrow<u16>(std::ceil(baseline + descentInPx + halfGapInPx));
const auto fontSize = std::ceilf(requestedSize.Y / 72.0f * _api.dpi);

const auto designUnitsPerPx = fontSize / static_cast<float>(metrics.designUnitsPerEm);
const auto ascent = static_cast<float>(metrics.ascent) * designUnitsPerPx;
const auto descent = static_cast<float>(metrics.descent) * designUnitsPerPx;
const auto lineGap = static_cast<float>(metrics.lineGap) * designUnitsPerPx;
const auto advanceWidth = static_cast<float>(glyphMetrics.advanceWidth) * designUnitsPerPx;
const auto underlinePosition = static_cast<float>(-metrics.underlinePosition) * designUnitsPerPx;
const auto underlineThickness = static_cast<float>(metrics.underlineThickness) * designUnitsPerPx;
const auto strikethroughPosition = static_cast<float>(-metrics.strikethroughPosition) * designUnitsPerPx;
const auto strikethroughThickness = static_cast<float>(metrics.strikethroughThickness) * designUnitsPerPx;

const auto halfGap = lineGap / 2.0f;
const auto baseline = std::ceilf(ascent + halfGap);
const auto lineHeight = std::ceilf(baseline + descent + halfGap);
const auto underlinePos = std::roundf(baseline + underlinePosition);
const auto underlineWidth = std::max(1.0f, std::roundf(underlineThickness));
const auto strikethroughPos = std::roundf(baseline + strikethroughPosition);
const auto strikethroughWidth = std::max(1.0f, std::roundf(strikethroughThickness));
const auto thinLineWidth = std::max(1.0f, std::roundf(underlineThickness / 2.0f));

// For double underlines we loosely follow what Word does:
// 1. The lines are half the width of an underline (= thinLineWidth)
// 2. Ideally the bottom line is aligned with the bottom of the underline
// 3. The top underline is vertically in the middle between baseline and ideal bottom underline
// 4. If the top line gets too close to the baseline the underlines are shifted downwards
// 5. The minimum gap between the two lines appears to be similar to Tex (1.2pt)
// (Additional notes below.)

// 2.
auto doubleUnderlinePosBottom = underlinePos + underlineWidth - thinLineWidth;
// 3. Since we don't align the center of our two lines, but rather the top borders
// we need to subtract half a line width from our center point.
auto doubleUnderlinePosTop = std::roundf((baseline + doubleUnderlinePosBottom - thinLineWidth) / 2.0f);
// 4.
doubleUnderlinePosTop = std::max(doubleUnderlinePosTop, baseline + thinLineWidth);
// 5. The gap is only the distance _between_ the lines, but we need the distance from the
// top border of the top and bottom lines, which includes an additional line width.
const auto doubleUnderlineGap = std::max(1.0f, std::roundf(1.2f / 72.0f * _api.dpi));
doubleUnderlinePosBottom = std::max(doubleUnderlinePosBottom, doubleUnderlinePosTop + doubleUnderlineGap + thinLineWidth);
// Our cells can't overlap each other so we additionally clamp the bottom line to be inside the cell boundaries.
doubleUnderlinePosBottom = std::min(doubleUnderlinePosBottom, lineHeight - thinLineWidth);

const auto cellWidth = gsl::narrow<u16>(std::ceilf(advanceWidth));
const auto cellHeight = gsl::narrow<u16>(lineHeight);

{
til::size coordSize;
Expand All @@ -637,28 +670,30 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo

if (fontMetrics)
{
const auto underlineOffsetInPx = static_cast<double>(-metrics.underlinePosition) * designUnitsPerPx;
const auto underlineThicknessInPx = static_cast<double>(metrics.underlineThickness) * designUnitsPerPx;
const auto strikethroughOffsetInPx = static_cast<double>(-metrics.strikethroughPosition) * designUnitsPerPx;
const auto strikethroughThicknessInPx = static_cast<double>(metrics.strikethroughThickness) * designUnitsPerPx;
const auto lineThickness = gsl::narrow<u16>(std::round(std::min(underlineThicknessInPx, strikethroughThicknessInPx)));
const auto underlinePos = gsl::narrow<u16>(std::ceil(baseline + underlineOffsetInPx - lineThickness / 2.0));
const auto strikethroughPos = gsl::narrow<u16>(std::round(baseline + strikethroughOffsetInPx - lineThickness / 2.0));

auto fontName = wil::make_process_heap_string(requestedFaceName);
const auto fontWeight = gsl::narrow<u16>(requestedWeight);
std::wstring fontName{ requestedFaceName };
const auto fontWeightU16 = gsl::narrow<u16>(requestedWeight);
const auto underlinePosU16 = gsl::narrow<u16>(underlinePos);
const auto underlineWidthU16 = gsl::narrow<u16>(underlineWidth);
const auto strikethroughPosU16 = gsl::narrow<u16>(strikethroughPos);
const auto strikethroughWidthU16 = gsl::narrow<u16>(strikethroughWidth);
const auto doubleUnderlinePosTopU16 = gsl::narrow<u16>(doubleUnderlinePosTop);
const auto doubleUnderlinePosBottomU16 = gsl::narrow<u16>(doubleUnderlinePosBottom);
const auto thinLineWidthU16 = gsl::narrow<u16>(thinLineWidth);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all throwing -- OK?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically I only want to throw if the result is out of bounds for an uint16_t. This code should work because floats can represent 16-bit integers exactly (i.e. converting back and forth yields the same result). I could convert them to int32_t first and then do the narrowing, e.g. with lround. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever you think strikes the best clarity vs work balance. This is fine with me :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this perhaps being unnecessarily cautious? I means is there any scenario here where a simple narrow_cast would actually produce a worse outcome than a throw?

Copy link
Member Author

@lhecker lhecker Jul 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I tend to prefer gsl::narrow over gsl::narrow_cast unless I need the latter for performance or exception safety (or I just want narrowing). I don't think the cost of the narrowing check is particularly high in most cases, so I'm fine with it.

In this case I'm a bit afraid that these values could be negative or larger than UINT16_MAX (since they come from the font file which might specify anything). I'll probably modify this PR to use gsl::narrow<u16>(lroundf(...)) which is even more excessively cautious, but it avoids any potential rounding errors, while catching any weird behavior (e.g. font size of >9000 leads to cell size >65536) and is fast enough (this code is only called once and each lroundf is <10ns).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was getting at is say you have some kind of overflow, and your underline position is now so large that it's off screen, is that a big deal? Is it likely to crash? If not, and the alternative is that the font just doesn't work at all, it doesn't seem like the narrow_cast is really an improvement. But it's not important. It was just a passing thought.


// NOTE: From this point onward no early returns or throwing code should exist,
// as we might cause _api to be in an inconsistent state otherwise.

fontMetrics->fontCollection = std::move(fontCollection);
fontMetrics->fontName = std::move(fontName);
fontMetrics->fontSizeInDIP = static_cast<float>(fontSizeInPx / static_cast<double>(_api.dpi) * 96.0);
fontMetrics->baselineInDIP = static_cast<float>(baseline / static_cast<double>(_api.dpi) * 96.0);
fontMetrics->fontSizeInDIP = fontSize / static_cast<float>(_api.dpi) * 96.0f;
fontMetrics->baselineInDIP = baseline / static_cast<float>(_api.dpi) * 96.0f;
fontMetrics->cellSize = { cellWidth, cellHeight };
fontMetrics->fontWeight = fontWeight;
fontMetrics->underlinePos = underlinePos;
fontMetrics->strikethroughPos = strikethroughPos;
fontMetrics->lineThickness = lineThickness;
fontMetrics->fontWeight = fontWeightU16;
fontMetrics->underlinePos = underlinePosU16;
fontMetrics->underlineWidth = underlineWidthU16;
fontMetrics->strikethroughPos = strikethroughPosU16;
fontMetrics->strikethroughWidth = strikethroughWidthU16;
fontMetrics->doubleUnderlinePos = { doubleUnderlinePosTopU16, doubleUnderlinePosBottomU16 };
fontMetrics->thinLineWidth = thinLineWidthU16;
}
}
17 changes: 6 additions & 11 deletions src/renderer/atlas/AtlasEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -976,10 +976,11 @@ void AtlasEngine::_recreateFontDependentResources()

_r.cellSizeDIP.x = static_cast<float>(_api.fontMetrics.cellSize.x) / scaling;
_r.cellSizeDIP.y = static_cast<float>(_api.fontMetrics.cellSize.y) / scaling;
_r.cellSize = _api.fontMetrics.cellSize;
_r.cellCount = _api.cellCount;
_r.dpi = _api.dpi;
_r.fontMetrics = _api.fontMetrics;
_r.atlasSizeInPixel = { 0, 0 };
_r.tileAllocator = TileAllocator{ _r.cellSize, _api.sizeInPixel };
_r.tileAllocator = TileAllocator{ _api.fontMetrics.cellSize, _api.sizeInPixel };

_r.glyphs = {};
_r.glyphQueue = {};
Expand All @@ -1000,12 +1001,6 @@ void AtlasEngine::_recreateFontDependentResources()
}

// D2D
{
_r.underlinePos = _api.fontMetrics.underlinePos;
_r.strikethroughPos = _api.fontMetrics.strikethroughPos;
_r.lineThickness = _api.fontMetrics.lineThickness;
_r.dpi = _api.dpi;
}
{
// See AtlasEngine::UpdateFont.
// It hardcodes indices 0/1/2 in fontAxisValues to the weight/italic/slant axes.
Expand Down Expand Up @@ -1036,7 +1031,7 @@ void AtlasEngine::_recreateFontDependentResources()
const auto fontStyle = italic ? DWRITE_FONT_STYLE_ITALIC : DWRITE_FONT_STYLE_NORMAL;
auto& textFormat = _r.textFormats[italic][bold];

THROW_IF_FAILED(_sr.dwriteFactory->CreateTextFormat(_api.fontMetrics.fontName.get(), _api.fontMetrics.fontCollection.get(), fontWeight, fontStyle, DWRITE_FONT_STRETCH_NORMAL, _api.fontMetrics.fontSizeInDIP, L"", textFormat.put()));
THROW_IF_FAILED(_sr.dwriteFactory->CreateTextFormat(_api.fontMetrics.fontName.c_str(), _api.fontMetrics.fontCollection.get(), fontWeight, fontStyle, DWRITE_FONT_STRETCH_NORMAL, _api.fontMetrics.fontSizeInDIP, L"", textFormat.put()));
textFormat->SetTextAlignment(DWRITE_TEXT_ALIGNMENT_CENTER);
textFormat->SetWordWrapping(DWRITE_WORD_WRAPPING_NO_WRAP);

Expand Down Expand Up @@ -1204,7 +1199,7 @@ void AtlasEngine::_flushBufferLine()
/* textPosition */ idx,
/* textLength */ gsl::narrow_cast<u32>(_api.bufferLine.size()) - idx,
/* baseFontCollection */ fontCollection.get(),
/* baseFamilyName */ _api.fontMetrics.fontName.get(),
/* baseFamilyName */ _api.fontMetrics.fontName.c_str(),
/* fontAxisValues */ textFormatAxis.data(),
/* fontAxisValueCount */ gsl::narrow_cast<u32>(textFormatAxis.size()),
/* mappedLength */ &mappedLength,
Expand All @@ -1223,7 +1218,7 @@ void AtlasEngine::_flushBufferLine()
/* textPosition */ idx,
/* textLength */ gsl::narrow_cast<u32>(_api.bufferLine.size()) - idx,
/* baseFontCollection */ fontCollection.get(),
/* baseFamilyName */ _api.fontMetrics.fontName.get(),
/* baseFamilyName */ _api.fontMetrics.fontName.c_str(),
/* baseWeight */ baseWeight,
/* baseStyle */ baseStyle,
/* baseStretch */ DWRITE_FONT_STRETCH_NORMAL,
Expand Down
20 changes: 12 additions & 8 deletions src/renderer/atlas/AtlasEngine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,14 +394,17 @@ namespace Microsoft::Console::Render
struct FontMetrics
{
wil::com_ptr<IDWriteFontCollection> fontCollection;
wil::unique_process_heap_string fontName;
std::wstring fontName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whhhyy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wil::unique_process_heap_string is not copyable and I didn't want to be overly complex. I mean it's 4x larger without benefits (wstring only stores 7 characters inline after all and even something trivial like "Consolas" is longer than that), but that doesn't really matter much here I think.

float baselineInDIP = 0.0f;
float fontSizeInDIP = 0.0f;
u16x2 cellSize;
u16 fontWeight = 0;
u16 underlinePos = 0;
u16 underlineWidth = 0;
u16 strikethroughPos = 0;
u16 lineThickness = 0;
u16 strikethroughWidth = 0;
u16x2 doubleUnderlinePos;
u16 thinLineWidth = 0;
};

// These flags are shared with shader_ps.hlsl.
Expand Down Expand Up @@ -823,8 +826,12 @@ namespace Microsoft::Console::Render
alignas(sizeof(f32)) f32 enhancedContrast = 0;
alignas(sizeof(u32)) u32 cellCountX = 0;
alignas(sizeof(u32x2)) u32x2 cellSize;
alignas(sizeof(u32x2)) u32x2 underlinePos;
alignas(sizeof(u32x2)) u32x2 strikethroughPos;
alignas(sizeof(u32)) u32 underlinePos = 0;
alignas(sizeof(u32)) u32 underlineWidth = 0;
alignas(sizeof(u32)) u32 strikethroughPos = 0;
alignas(sizeof(u32)) u32 strikethroughWidth = 0;
alignas(sizeof(u32x2)) u32x2 doubleUnderlinePos;
alignas(sizeof(u32)) u32 thinLineWidth = 0;
alignas(sizeof(u32)) u32 backgroundColor = 0;
alignas(sizeof(u32)) u32 cursorColor = 0;
alignas(sizeof(u32)) u32 selectionColor = 0;
Expand Down Expand Up @@ -943,12 +950,9 @@ namespace Microsoft::Console::Render
Buffer<Cell, 32> cells; // invalidated by ApiInvalidations::Size
Buffer<TileHashMap::iterator> cellGlyphMapping; // invalidated by ApiInvalidations::Size
f32x2 cellSizeDIP; // invalidated by ApiInvalidations::Font, caches _api.cellSize but in DIP
u16x2 cellSize; // invalidated by ApiInvalidations::Font, caches _api.cellSize
u16x2 cellCount; // invalidated by ApiInvalidations::Font|Size, caches _api.cellCount
u16 underlinePos = 0;
u16 strikethroughPos = 0;
u16 lineThickness = 0;
u16 dpi = USER_DEFAULT_SCREEN_DPI; // invalidated by ApiInvalidations::Font, caches _api.dpi
FontMetrics fontMetrics; // invalidated by ApiInvalidations::Font, cached _api.fontMetrics
u16x2 atlasSizeInPixel; // invalidated by ApiInvalidations::Font
TileHashMap glyphs;
TileAllocator tileAllocator;
Expand Down
20 changes: 12 additions & 8 deletions src/renderer/atlas/AtlasEngine.r.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,21 @@ void AtlasEngine::_updateConstantBuffer() const noexcept
ConstBuffer data;
data.viewport.x = 0;
data.viewport.y = 0;
data.viewport.z = static_cast<float>(_r.cellCount.x * _r.cellSize.x);
data.viewport.w = static_cast<float>(_r.cellCount.y * _r.cellSize.y);
data.viewport.z = static_cast<float>(_r.cellCount.x * _r.fontMetrics.cellSize.x);
data.viewport.w = static_cast<float>(_r.cellCount.y * _r.fontMetrics.cellSize.y);
DWrite_GetGammaRatios(_r.gamma, data.gammaRatios);
data.enhancedContrast = useClearType ? _r.cleartypeEnhancedContrast : _r.grayscaleEnhancedContrast;
data.cellCountX = _r.cellCount.x;
data.cellSize.x = _r.cellSize.x;
data.cellSize.y = _r.cellSize.y;
data.underlinePos.x = _r.underlinePos;
data.underlinePos.y = _r.underlinePos + _r.lineThickness;
data.strikethroughPos.x = _r.strikethroughPos;
data.strikethroughPos.y = _r.strikethroughPos + _r.lineThickness;
data.cellSize.x = _r.fontMetrics.cellSize.x;
data.cellSize.y = _r.fontMetrics.cellSize.y;

data.underlinePos = _r.fontMetrics.underlinePos;
data.underlineWidth = _r.fontMetrics.underlineWidth;
data.strikethroughPos = _r.fontMetrics.strikethroughPos;
data.strikethroughWidth = _r.fontMetrics.strikethroughWidth;
data.doubleUnderlinePos.x = _r.fontMetrics.doubleUnderlinePos.x;
data.doubleUnderlinePos.y = _r.fontMetrics.doubleUnderlinePos.y;
data.thinLineWidth = _r.fontMetrics.thinLineWidth;
data.backgroundColor = _r.backgroundColor;
data.cursorColor = _r.cursorOptions.cursorColor;
data.selectionColor = _r.selectionColor;
Expand Down
57 changes: 36 additions & 21 deletions src/renderer/atlas/shader_ps.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,12 @@ cbuffer ConstBuffer : register(b0)
float enhancedContrast;
uint cellCountX;
uint2 cellSize;
uint2 underlinePos;
uint2 strikethroughPos;
uint underlinePos;
uint underlineWidth;
uint strikethroughPos;
uint strikethroughWidth;
uint2 doubleUnderlinePos;
uint thinLineWidth;
uint backgroundColor;
uint cursorColor;
uint selectionColor;
Expand Down Expand Up @@ -107,22 +111,7 @@ float4 main(float4 pos: SV_Position): SV_Target
}

// Layer 2:
// Step 1: Underlines
[branch] if (cell.flags & CellFlags_Underline)
{
[flatten] if (cellPos.y >= underlinePos.x && cellPos.y < underlinePos.y)
{
color = alphaBlendPremultiplied(color, fg);
}
}
[branch] if (cell.flags & CellFlags_UnderlineDotted)
{
[flatten] if (cellPos.y >= underlinePos.x && cellPos.y < underlinePos.y && (viewportPos.x / (underlinePos.y - underlinePos.x) & 3) == 0)
{
color = alphaBlendPremultiplied(color, fg);
}
}
// Step 2: The cell's glyph, potentially drawn in the foreground color
// Step 1: The cell's glyph, potentially drawn in the foreground color
{
float4 glyph = glyphs[decodeU16x2(cell.glyphPos) + cellPos];

Expand Down Expand Up @@ -152,10 +141,36 @@ float4 main(float4 pos: SV_Position): SV_Target
}
}
}
// Step 3: Lines, but not "under"lines
[branch] if (cell.flags & CellFlags_Strikethrough)
// Step 2: Lines
{
[flatten] if (cellPos.y >= strikethroughPos.x && cellPos.y < strikethroughPos.y)
// What a nice coincidence that we have exactly 8 flags to handle right now!
// `mask` will mask away any positive results from checks we don't want.
// (I.e. even if we're in an underline, it doesn't matter if we don't want an underline.)
bool4x2 mask = {
cell.flags & CellFlags_BorderLeft,
cell.flags & CellFlags_BorderTop,
cell.flags & CellFlags_BorderRight,
cell.flags & CellFlags_BorderBottom,
cell.flags & CellFlags_Underline,
cell.flags & CellFlags_UnderlineDotted,
cell.flags & CellFlags_UnderlineDouble,
cell.flags & CellFlags_Strikethrough,
};
// The following <lineWidth checks rely on underflow turning the
// uint into a way larger number than any reasonable lineWidth.
// That way we don't need to write `y >= lo && y < hi`.
bool4x2 checks = {
// These 2 expand to 4 bools, because cellPos is a
// uint2 vector which results in a bool2 result each.
cellPos < thinLineWidth,
(cellSize - cellPos) <= thinLineWidth,
// These 4 are 4 regular bools.
(cellPos.y - underlinePos) < underlineWidth,
(cellPos.y - underlinePos) < underlineWidth && (viewportPos.x / underlineWidth & 3) == 0,
any((cellPos.y - doubleUnderlinePos) < thinLineWidth),
(cellPos.y - strikethroughPos) < strikethroughWidth,
};
[flatten] if (any(mask && checks))
{
color = alphaBlendPremultiplied(color, fg);
}
Expand Down