SGR 8 doesn't work when acrylic is enabled

[j4james]

Windows Terminal version

1.12.2931.0

Windows build number

10.0.19041.1348

Other Software

No response

Steps to reproduce

1. In the Setting UI, go to the Profiles Defaults page and select the Appearance tab.
2. Make sure the option Automaticaly adjust lightness of indistinguishable text is disabled (see SGR 1 and SGR 8 don't work when adjusting lightness of indistinguishable text #11917).
3. Make sure the Enable acrylic option is on.
4. Set the Background opacity slider to 50%.
5. Open a bash shell.
6. Execute the following command: printf "\e[8m CONCEALED \e[m\n"
7. Move the terminal window over something that is white/bright in color.

Expected Behavior

I'd expect the text to be completely invisible.

Actual Behavior

The text is quite clearly visible.

An easy fix for this might be to include an additional attr.IsInvisible() test in the condition here:

terminal/src/cascadia/TerminalCore/terminalrenderdata.cpp

Lines 94 to 99 in bb71179

	// 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() ^ _screenReversed))
	{
	colors.second |= 0xff000000;
	}

The concealed text wouldn't show as transparent, but would instead appear as an opaque version of the background color, as if it had been blacked out by a censor. That seems like a reasonable rendition I think.

[zadjii-msft]

1. lol
2. that seems like a reasonable fix to me

[DHowett]

1. LOL!
2. I think the renderer (graphical renderer; the VT one can't do this...) should skip Concealed text entirely. After all... why try our hardest to put down matching pixels when we could just put down nothing at all?

[DHowett]

(I say this in part because renderer support for transparent foregrounds is going to be spotty; we have never formally tested it! Atlas engine made an assumption about it as well.)

[j4james]

I think the renderer (graphical renderer; the VT one can't do this...) should skip Concealed text entirely.

I'm not opposed to that, but it does seem likely to be a lot more work for an attribute that's probably not used very often. If someone else is keen to take that on, though, I wouldn't object.

[j4james]

This was actually fixed in PR #12127. As proposed above, we now render the concealed text with an opaque version of the background color. For anyone that would prefer it to be completely transparent, that's tracked in issue #16271.