Acrylic background doesn't work if foreground is invisible

[alabuzhev]

Windows Terminal version

Latest nightly

Windows build number

10.0.19045.3448

Other Software

No response

Steps to reproduce

1. Enable background transparency and acrylic material in settings.
2. 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

Actual Behavior

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

Looking at the code, it seems to be here:

terminal/src/renderer/base/RenderSettings.cpp

Lines 205 to 207 in 9e86c98

	// We only care about alpha for the default BG (which enables acrylic)
	// If the bg isn't the default bg color, or reverse video is enabled, make it fully opaque.
	if (!attr.BackgroundIsDefault() || (attr.IsReverseVideo() ^ GetRenderMode(Mode::ScreenReversed)) || attr.IsInvisible())

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]

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]

See also #7014.

[DHowett]

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]

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]

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

[alabuzhev]

@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]

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.