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

Make sure the inverted cursor is always readable (#3647) #13748

Merged
2 commits merged into from
Aug 16, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions .github/actions/spelling/allow/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ lround
Lsa
lsass
LSHIFT
LTGRAY
MAINWINDOW
memchr
memicmp
Expand Down Expand Up @@ -146,6 +147,7 @@ OUTLINETEXTMETRICW
overridable
PACL
PAGESCROLL
PATINVERT
PEXPLICIT
PICKFOLDERS
pmr
Expand Down
3 changes: 3 additions & 0 deletions src/renderer/atlas/shader_ps.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ float4 main(float4 pos: SV_Position): SV_Target
[flatten] if (cursorColor == INVALID_COLOR && glyphs[cellPos].a != 0)
{
color = float4(1 - color.rgb, 1);

// Make sure the cursor is always readable (see gh-3647)
color = float4((((uint3(color.rgb * 255) ^ 0x3f) & 0xff) / 255.0).rgb, 1);
Copy link
Member

@lhecker lhecker Aug 16, 2022

Choose a reason for hiding this comment

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

I've been thinking about whether this can be expressed in floating point since GPUs prefer floating point over integer operations and because it would allow us to retain the full "color accuracy" up to the end of the shader. Here's what I came up with:

Suggested change
color = float4(1 - color.rgb, 1);
// Make sure the cursor is always readable (see gh-3647)
color = float4((((uint3(color.rgb * 255) ^ 0x3f) & 0xff) / 255.0).rgb, 1);
// Make sure the cursor is always readable (see gh-3647)
// If we imagine color to be in 0-255 instead of 0-1,
// this effectively XORs color with 63.
float3 ip; // integral part
float3 frac = modf(color.rgb * (255.0f / 64.0f), ip);
color = float4((3.0f - ip + frac) * (64.0f / 255.0f), 1);

If you're curious, this is how it changes the assembly to be more compact:
image

}
}

Expand Down
4 changes: 2 additions & 2 deletions src/renderer/dx/CustomTextRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,8 @@ try
if (firstPass)
{
// Draw a backplate behind the cursor in the *background* color so that we can invert it later.
// We're going to draw the exact same color as the background behind the cursor
const til::color color{ drawingContext.backgroundBrush->GetColor() };
// Make sure the cursor is always readable (see gh-3647)
const til::color color{ til::color{ drawingContext.backgroundBrush->GetColor() } ^ RGB(63, 63, 63) };
RETURN_IF_FAILED(d2dContext->CreateSolidColorBrush(color.with_alpha(255),
&brush));
}
Expand Down
6 changes: 5 additions & 1 deletion src/renderer/gdi/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,11 @@ bool GdiEngine::FontHasWesternScript(HDC hdc)

for (auto r : cursorInvertRects)
{
RETURN_HR_IF(E_FAIL, !(InvertRect(_hdcMemoryContext, &r)));
// Make sure the cursor is always readable (see gh-3647)
const auto PrevObject = SelectObject(_hdcMemoryContext, GetStockObject(LTGRAY_BRUSH));
const auto Result = PatBlt(_hdcMemoryContext, r.left, r.top, r.right - r.left, r.bottom - r.top, PATINVERT);
SelectObject(_hdcMemoryContext, PrevObject);
RETURN_HR_IF(E_FAIL, !Result);
}
}

Expand Down