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.

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

Add doubleUnderscore cursor style #7827

Merged
9 commits merged into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
3 changes: 2 additions & 1 deletion doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -737,9 +737,10 @@
},
"cursorShape": {
"default": "bar",
"description": "Sets the shape of the cursor. Possible values:\n -\"bar\" ( ┃, default )\n -\"emptyBox\" ( ▯ )\n -\"filledBox\" ( █ )\n -\"underscore\" ( ▁ )\n -\"vintage\" ( ▃ )",
"description": "Sets the shape of the cursor. Possible values:\n -\"bar\" ( ┃, default )\n -\"doubleUnderscore\" ( ‗ )\n -\"emptyBox\" ( ▯ )\n -\"filledBox\" ( █ )\n -\"underscore\" ( ▁ )\n -\"vintage\" ( ▃ )",
"enum": [
"bar",
"doubleUnderscore",
"emptyBox",
"filledBox",
"underscore",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ Module Name:

JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::TerminalControl::CursorStyle)
{
static constexpr std::array<pair_type, 5> mappings = {
static constexpr std::array<pair_type, 6> mappings = {
pair_type{ "bar", ValueType::Bar },
pair_type{ "vintage", ValueType::Vintage },
pair_type{ "underscore", ValueType::Underscore },
pair_type{ "filledBox", ValueType::FilledBox },
pair_type{ "emptyBox", ValueType::EmptyBox }
pair_type{ "emptyBox", ValueType::EmptyBox },
pair_type{ "doubleUnderscore", ValueType::DoubleUnderscore }
};
};

Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalCore/ICoreSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ namespace Microsoft.Terminal.TerminalControl
Bar,
Underscore,
FilledBox,
EmptyBox
EmptyBox,
DoubleUnderscore
};

interface ICoreSettings
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ void Terminal::UpdateSettings(ICoreSettings settings)
case CursorStyle::Vintage:
cursorShape = CursorType::Legacy;
break;
case CursorStyle::DoubleUnderscore:
cursorShape = CursorType::DoubleUnderscore;
break;
default:
case CursorStyle::Bar:
cursorShape = CursorType::VerticalBar;
Expand Down
3 changes: 2 additions & 1 deletion src/inc/conattrs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ enum class CursorType : unsigned int
VerticalBar = 0x1, // A single vertical line, '|'
Underscore = 0x2, // a single horizontal underscore, smaller that the min height legacy cursor.
EmptyBox = 0x3, // Just the outline of a full box
FullBox = 0x4 // a full box, similar to legacy with height=100%
FullBox = 0x4, // a full box, similar to legacy with height=100%
DoubleUnderscore = 0x5 // a double horizontal underscore
miniksa marked this conversation as resolved.
Show resolved Hide resolved
};

// Valid COLORREFs are of the pattern 0x00bbggrr. -1 works as an invalid color,
Expand Down
15 changes: 13 additions & 2 deletions src/renderer/dx/CustomTextRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ try
}

CursorPaintType paintType = CursorPaintType::Fill;
Microsoft::WRL::ComPtr<ID2D1SolidColorBrush> brush{ drawingContext.foregroundBrush };

switch (options.cursorType)
{
Expand All @@ -318,6 +319,18 @@ try
rect.top = rect.bottom - 1;
break;
}
case CursorType::DoubleUnderscore:
{
// Use rect for lower line.
rect.top = rect.bottom - 1;

// Draw upper line directly.
D2D1_RECT_F upperLine = rect;
upperLine.top -= 2;
upperLine.bottom -= 2;
Comment on lines +336 to +337
Copy link
Member

Choose a reason for hiding this comment

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

Does this look correct at all DPIs or does it need to be adjusted by a scale factor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched "Custom scaling" to 200% in the settings. Is this the correct/preferred procedure?

Single underline is IMHO also thin:
underscore

And double underscore for comparison:
doubleUnderscore

(Clicking the images helps.)

I'm open whether to scale or not to scale 😊

Copy link
Member

Choose a reason for hiding this comment

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

Oof, can you file a follow on issue (or search the repo for an existing one) to make sure these things get scaled? I bet its just 1px and it looks bad for all options.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, I couldn't find one on searching. Maybe my boolean-fu is bad. @j4james this might be a thing you know about? I don't know why I think that... but something rings a bell in my head that you might have noticed cursors aren't DPI scaled...

Copy link
Collaborator

Choose a reason for hiding this comment

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

No. I was looking at the grid line rendering (underlines, overlines, etc.). Those do now track the font style and also scale appropriately, but I don't think the cursors do (at least not all of them).

d2dContext->FillRectangle(upperLine, brush.Get());
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
break;
}
case CursorType::EmptyBox:
{
paintType = CursorPaintType::Outline;
Expand All @@ -331,8 +344,6 @@ try
return E_NOTIMPL;
}

Microsoft::WRL::ComPtr<ID2D1SolidColorBrush> brush{ drawingContext.foregroundBrush };

if (options.fUseColor)
{
// Make sure to make the cursor opaque
Expand Down
13 changes: 13 additions & 0 deletions src/renderer/gdi/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,19 @@ using namespace Microsoft::Console::Render;
cursorInvertRects.push_back(rcInvert);
break;

case CursorType::DoubleUnderscore:
{
RECT top, bottom;
top = bottom = rcBoundaries;
RETURN_IF_FAILED(LongAdd(bottom.bottom, -1, &bottom.top));
RETURN_IF_FAILED(LongAdd(top.bottom, -3, &top.top));
miniksa marked this conversation as resolved.
Show resolved Hide resolved
RETURN_IF_FAILED(LongAdd(top.top, 1, &top.bottom));

cursorInvertRects.push_back(top);
cursorInvertRects.push_back(bottom);
}
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
break;

case CursorType::EmptyBox:
{
RECT top, left, right, bottom;
Expand Down