-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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 a setting to control selection foreground color #3580
Comments
I suggested this in the Selection Background colour issue, so I am all for this. |
Note that both XTerm and VTE support OSC 17 and 19 for settings these colors, so that's something we may also want to support in the future. Not essential, but I thought it worth mentioning now in case it's something you need to take into account in the design. |
@j4james I did not know that. That's definitely good info to have, since I definitely said during the #3471 PR "this doesn't need to be in the terminal core, it's not something that's set by VT" :egg_on_face: It's not a terrible pain to refactor the existing code to support that. We'll make sure to keep that in mind when coming back to this |
My team is very interested in this as well! Being able to set the background selection text color fixes half of the problem, and setting the foreground as well would clear up our contrast ration issues. |
@zadjii-msft, has this issue been slated for a release? |
@javierdlg no. when it is slated for a release, the Milestone on the righthand side will change from "Backlog". 😄 |
@DHowett-MSFT not having this setting is impacting our accessibility story (e.g. high contrast themes) and thus our ability to release the VS terminal in the release channel. Do you have any estimates on when you'll be able to expose this setting? |
This is a significant amount of engineering work (cf. the original post) that I can't commit to before Terminal v1.0. I have, however, marked it Help-Wanted. 😄 |
Any update on this? It's been a few years, for accessibility reasons would love to be able to change the selection foreground colour. |
…a overlay (#17725) 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 is highly related to #3326/#3471 and #3561.
We've added a setting to control what color we use to highlight text with when it's selected in #3471.
However, that doesn't necessarily fix our contrast ratio problems w.r.t. selection, since the configured color might still not have enough difference from the foreground of the highlighted text.
Conhost had an inverting selection, which didn't have this problem. However, we think that might be Hard to implement in the dx renderer. Adding that to the Terminal is being tracked in #3561.
A selection foreground color
A: might be more feasible to implement
B: is a good configurable setting to add regardless, to enable further personalization of the terminal.
Implementation might still be tricky though, since the Renderer is only ever told about the selection rects after it has drawn the text. Re-drawing the text seems like a bad idea. I'm not sure we'll be able to re-order things in the Renderer safely without also impacting the GDI and other rendering heads. This is hard, but easier than inverting selection.
The text was updated successfully, but these errors were encountered: