Acrylic background doesn't work if foreground is invisible - Githubissues

microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!

MIT License

95.36k stars 8.3k forks source link

Acrylic background doesn't work if foreground is invisible #16271

Open alabuzhev opened 11 months ago

alabuzhev commented 11 months ago

Windows Terminal version

Latest nightly

Windows build number

10.0.19045.3448

Other Software

No response

Steps to reproduce

Enable background transparency and acrylic material in settings.
Run the script below:
```
@echo off
```

echo [1m bold[m echo [2m faint[m echo [3m italic[m echo [4m underline[m echo [9m strikeout[m echo [53m overline[m echo [5m blink[m echo [7m inverse[m echo [8m invisible[m

pause


\- where  is \x1b

### Expected Behavior

![image](https://github.com/microsoft/terminal/assets/11453922/95df0270-308d-45b2-8e1e-105b0a8b7a65)

### Actual Behavior

![image](https://github.com/microsoft/terminal/assets/11453922/65a97640-eb16-45ec-ba4e-9d52ca30c065)

\- "Invisible" foreground causes the background to be fully opaque.

Looking at the code, it seems to be here:
https://github.com/microsoft/terminal/blob/9e86c9811f2cf0ad53c9c543b886274b1079dd00/src/renderer/base/RenderSettings.cpp#L205-L207

The ` || attr.IsInvisible()` bit was added here:
https://github.com/microsoft/terminal/pull/12127/files#diff-0f336e9fd3608b43380ed0ad16fc625c4f1e2a681c70fc627f9df7239e1b6d54R252-R253

Looks like it wasn't in the original code:
https://github.com/microsoft/terminal/pull/12127/files#diff-f9112caf8cb75e7a48a7b84987724d754181227385fbfcc2cc09a879b1f97c12L90-L91

The change wasn't reflected in the comment / mentioned elsewhere in the PR (or at least I can't find it), so, is it intentional / a bug?

j4james commented 11 months ago

It was an intentional fix for issue #11919, but I must have forgotten to reference it in the PR notes, and obviously that issue should also have been closed now.

The fact that it renders opaque with an acrylic background was a known compromise which I mentioned in #11919. Personally I kind of like the effect, but I also wouldn't care if someone wanted to make it fully transparent. That would likely be more complicated though.

j4james commented 11 months ago

See also #7014.

DHowett commented 11 months ago

I also wouldn't care if someone wanted to make it fully transparent.

I keep thinking about this - we could probably skip it in the renderer completely. The VtEngine would of course continue to produce it, but the actual graphical engines could ignore it and it would definitely appear to be concealed :laugh:

alabuzhev commented 11 months ago

Thanks James. Personally I don't really care how exactly it works, just reporting an inconsistency with other terminals:

Fully transparent could be beneficial at least for the sake of consistency and reducing the amount of WTFs/min (I thought there's a bug in my code somewhere).

zadjii-msft commented 10 months ago

Pretty sure this is just a dup of #7014? Or, a more specific subset of that?

alabuzhev commented 10 months ago

@zadjii-msft not really. Both are about transparency handling, but #7014 is about the text (when "reversed"), while this one is about the background (when "concealed").

j4james commented 10 months ago

They are kind of related, in that the concealed background only lost its transparency in order to fix #11919, but if we had support for transparent text (via #7014), then #11919 could probably have been fixed by rendering both the foreground and background as transparent.

That said, I'm inclined to agree with @DHowett that skipping the rendering of concealed text entirely might be the best approach to take.