Skip to content

Commit

Permalink
Make sure background color is opaque when attrs are reversed (#5509)
Browse files Browse the repository at this point in the history
In order to support a transparent background for the acrylic effect, the
renderer sets the alpha value to zero for the default background color.
However, when the _reversed video_ attribute is set, the background is
actually filled with the foreground color, and will not be displayed
correctly if it is made transparent. This PR addresses that issue by
making sure the rendered background color is opaque if the reversed
video attribute is set.

## References

* This is not a major issue at the moment, since the _reverse video_
  attribute is not typically forwarded though conpty, but that will
  change once #2661 is fixed.

## PR Checklist
* [x] Closes #5498
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not
  checked, I'm ready to accept this work might be rejected in favor of a
  different grand plan. Issue number where discussion took place: #5498

## Detailed Description of the Pull Request / Additional comments

This simply adds an additional check in `Terminal::GetBackgroundColor`
to make sure the returned color is opaque if the _reverse video_
attribute is set. At some point in the future this check may need to be
extended to support the `DECSCNM` reverse screen mode, but for now
that's not an issue.

## Validation Steps Performed

I've run the test case from issue #5498, and confirmed that it now works
as expected. I've also got an experimental fix for #2661 that I've
tested with this patch, and that now displays _reverse video_ attributes
correctly too.

Closes #5498
  • Loading branch information
j4james authored and DHowett committed Apr 27, 2020
1 parent b05f4e4 commit 4b4a006
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/cascadia/TerminalCore/terminalrenderdata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ const COLORREF Terminal::GetBackgroundColor(const TextAttribute& attr) const noe
{
const auto bgColor = attr.CalculateRgbBackground({ _colorTable.data(), _colorTable.size() }, _defaultFg, _defaultBg);
// We only care about alpha for the default BG (which enables acrylic)
// If the bg isn't the default bg color, then make it fully opaque.
if (!attr.BackgroundIsDefault())
// If the bg isn't the default bg color, or reverse video is enabled, make it fully opaque.
if (!attr.BackgroundIsDefault() || WI_IsFlagSet(attr.GetMetaAttributes(), COMMON_LVB_REVERSE_VIDEO))
{
return 0xff000000 | bgColor;
}
Expand Down

0 comments on commit 4b4a006

Please sign in to comment.