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

atlas: draw selection colors as background/foreground instead of alpha overlay #17725

Merged
merged 4 commits into from
Aug 19, 2024

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Aug 15, 2024

With the merge of #17638, selections are now accumulated early in the
rendering process. This allows Atlas, which currently makes decisions
about cell foreground/background at the time of text rendering,
awareness of the selection ranges before text rendering begins.

As a result, we can now paint the selection into the background and
foreground bitmaps. We no longer need to overlay a rectangle, or series
of rectangles, on top of the rendering surface and alpha blend the
selection color onto the final image.

As a reminder, "alpha selection" was always a stopgap because we didn't
have durable per-cell foreground and background customization in the
original DxEngine.

Selection foregrounds are not customizable, and will be chosen using the
same color distancing algorithm as the cursor. We can make them
customizable "easily" (once we figure out the schema for it) for #3580.

ATLAS_DEBUG_SHOW_DIRTY was using the Selection shading type to draw
colored regions. I didn't want to break that, so I elected to rename the
Selection shading type to FilledRect and keep its value. It helps
that the shader didn't have any special treatment for
SHADING_TYPE_SELECTION.

This fixes the entire category of issues created by selection being an
80%-opacity white rectangle. However, given that it changes the imputed
colors of the text it will reveal SGR 8 concealed/hidden characters.

Refs #17355
Refs #14859
Refs #11181
Refs #8716
Refs #4971
Closes #3561

This lets us get rid of SelectionFrom/SelectionTo and the selection
quad.
This required some changes to the WPF Control API surface
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-Fonts Related to the font Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Product-Terminal The new Windows Terminal. labels Aug 15, 2024
// Return early if we couldn't paint the whole region (either this was not the last row, or
// it was the last row but the highlight ends outside of our x range.)
// We will resume from here in the next call.
if (!isFinalRow || hiEnd.x /*inclusive*/ >= x2 /*exclusive*/)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the thing it took hours to fix lol

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, Ahh, hmm. Thanks for fixing this 🥲😃

@@ -16,7 +16,7 @@
#define SHADING_TYPE_CURLY_LINE 7
#define SHADING_TYPE_SOLID_LINE 8
#define SHADING_TYPE_CURSOR 9
#define SHADING_TYPE_SELECTION 10
#define SHADING_TYPE_FILLED_RECT 10
Copy link
Member Author

Choose a reason for hiding this comment

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

better name would be A-OK with me

@DHowett
Copy link
Member Author

DHowett commented Aug 15, 2024

Here's a quick demo video of selection colors, and how they change while I change color schemes.

WindowsTerminal_20240803T175408_562.mp4

_api.s.write()->misc.write()->selectionColor = selectionColor;
auto misc = _api.s.write()->misc.write();
misc->selectionColor = selectionColor;
misc->selectionForeground = 0xff000000 | ColorFix::GetPerceivableColor(color, color, 0.5f * 0.5f);
Copy link
Member

@lhecker lhecker Aug 15, 2024

Choose a reason for hiding this comment

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

Hold up, this has color twice. (1st param is fg, 2nd param is bg.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I see: I misread the code in BackendD3D for cursor colors. I thought it did pass the background twice, but it passes background and bg (lol)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, no, I didn't misread. I pulled this from BackendD3D _drawCursorForegroundSlowPath

    auto color = c.foreground == 0xffffffff ? it.color ^ 0xffffff : c.foreground;
    color = ColorFix::GetPerceivableColor(color, c.background, 0.5f * 0.5f);

It uses a foreground and a background, whereas we don't have a foreground color to speak of yet.

@DHowett
Copy link
Member Author

DHowett commented Aug 18, 2024

@tusharsnx let me tell you, I spent five hours with a whiteboard and rewrote the whole thing, and it was a seriously humbling experience. I fixed the issue I was seeing, and then I decided to just keep everything that was already there.

I couldn't--and still can't!--wrap my head around how to simplify it given that it detects overlaps of a coordinate span with a coordinate span. In my rewrite, I ended up linearizing every coordinate (y*w+x) instead.

I'm still mad at myself for it. 😁

@DHowett
Copy link
Member Author

DHowett commented Aug 19, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DHowett DHowett enabled auto-merge (squash) August 19, 2024 14:18
@DHowett DHowett merged commit faf21ac into main Aug 19, 2024
18 of 20 checks passed
@DHowett DHowett deleted the dev/duhowett/sel-3-atlas branch August 19, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Fonts Related to the font Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selection Color should invert selected text color
4 participants