Repository: microsoft/terminal (language: cpp)

Issue:
Selection Color should invert selected text color
# Environment

```none
Windows build number: 10.0.18363.0
Windows Terminal version (if applicable): All
```

# Steps to reproduce

1. Open the terminal.
1. Select some text.

# Expected behavior

Selected text should invert the text colors:


# Actual behavior

Text maintains it's original color which leads to contrast issues. 
![image](https://user-images.githubusercontent.com/2334756/68795006-ed5aad00-0604-11ea-822f-51f3ae4d9c9c.png)

# Notes

Powershell and CMD both invert the text colors when selected, which would fix the contrast ratio accessibility issue present in the terminal:
![image](https://user-images.githubusercontent.com/2334756/68793524-3b21e600-0602-11ea-820f-c1885cb48aa6.png)




Discussion / hints:
_oh no_

This is hard to _very hard_ on the scale of tasks, unfortunately. 

GDI has a very handy function for inverting the colors of a region of the window. unfortunately, DX has so such equivalent function. I believe @miniksa has said something in that past about rendering the frame, then specially inverting the selected area in another rendering pass? He can elaborate more.
How about being able to choose the selected text foreground color? This could potentially solve the contrast ratio issue on selected text and not force us to vet all the colors against two background colors for every theme.
Didn't #3471 committed today literally just do this?
> Didn't #3471 committed today literally just do this?

It only includes a Selection Background colour.  I did make the suggestion there should also be a Selection Foreground colour.
We're open to contributions to our DX renderer. Thanks!

Fix patch:
diff --git a/src/cascadia/TerminalControl/HwndTerminal.cpp b/src/cascadia/TerminalControl/HwndTerminal.cpp
index 5086515e975..4b8c345baac 100644
--- a/src/cascadia/TerminalControl/HwndTerminal.cpp
+++ b/src/cascadia/TerminalControl/HwndTerminal.cpp
@@ -882,7 +882,7 @@ void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR font
         renderSettings.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, theme.DefaultForeground);
         renderSettings.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, theme.DefaultBackground);
 
-        publicTerminal->_renderEngine->SetSelectionBackground(theme.DefaultSelectionBackground, theme.SelectionBackgroundAlpha);
+        publicTerminal->_renderEngine->SetSelectionBackground(theme.DefaultSelectionBackground);
 
         // Set the font colors
         for (size_t tableIndex = 0; tableIndex < 16; tableIndex++)
diff --git a/src/cascadia/TerminalControl/HwndTerminal.hpp b/src/cascadia/TerminalControl/HwndTerminal.hpp
index e561f1793fc..9a19b6090db 100644
--- a/src/cascadia/TerminalControl/HwndTerminal.hpp
+++ b/src/cascadia/TerminalControl/HwndTerminal.hpp
@@ -35,7 +35,6 @@ typedef struct _TerminalTheme
     COLORREF DefaultBackground;
     COLORREF DefaultForeground;
     COLORREF DefaultSelectionBackground;
-    float SelectionBackgroundAlpha;
     uint32_t CursorStyle; // This will be converted to DispatchTypes::CursorStyle (size_t), but C# cannot marshal an enum type and have it fit in a size_t.
     COLORREF ColorTable[16];
 } TerminalTheme, *LPTerminalTheme;
diff --git a/src/cascadia/WpfTerminalControl/TerminalTheme.cs b/src/cascadia/WpfTerminalControl/TerminalTheme.cs
index 89e2ada53a4..699d1665609 100644
--- a/src/cascadia/WpfTerminalControl/TerminalTheme.cs
+++ b/src/cascadia/WpfTerminalControl/TerminalTheme.cs
@@ -70,11 +70,6 @@ public struct TerminalTheme
         /// </summary>
         public uint DefaultSelectionBackground;
 
-        /// <summary>
-        /// The opacity alpha for the selection color of the terminal, must be between 1.0 and 0.0.
-        /// </summary>
-        public float SelectionBackgroundAlpha;
-
         /// <summary>
         /// The style of cursor to use in the terminal.
         /// </summary>
diff --git a/src/cascadia/WpfTerminalTestNetCore/MainWindow.xaml.cs b/src/cascadia/WpfTerminalTestNetCore/MainWindow.xaml.cs
index 1389e542913..e67b1280319 100644
--- a/src/cascadia/WpfTerminalTestNetCore/MainWindow.xaml.cs
+++ b/src/cascadia/WpfTerminalTestNetCore/MainWindow.xaml.cs
@@ -97,7 +97,6 @@ private void Terminal_Loaded(object sender, RoutedEventArgs e)
                 DefaultBackground = 0x0c0c0c,
                 DefaultForeground = 0xcccccc,
                 DefaultSelectionBackground = 0xcccccc,
-                SelectionBackgroundAlpha = 0.5f,
                 CursorStyle = CursorStyle.BlinkingBar,
                 // This is Campbell.
                 ColorTable = new uint[] { 0x0C0C0C, 0x1F0FC5, 0x0EA113, 0x009CC1, 0xDA3700, 0x981788, 0xDD963A, 0xCCCCCC, 0x767676, 0x5648E7, 0x0CC616, 0xA5F1F9, 0xFF783B, 0x9E00B4, 0xD6D661, 0xF2F2F2 },
diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp
index 1850cf0cab1..f5ddc5a000d 100644
--- a/src/renderer/atlas/AtlasEngine.api.cpp
+++ b/src/renderer/atlas/AtlasEngine.api.cpp
@@ -7,6 +7,7 @@
 #include "Backend.h"
 #include "../../buffer/out/textBuffer.hpp"
 #include "../base/FontCache.h"
+#include "../../types/inc/ColorFix.hpp"
 
 // #### NOTE ####
 // If you see any code in here that contains "_r." you might be seeing a race condition.
@@ -424,12 +425,15 @@ void AtlasEngine::SetRetroTerminalEffect(bool enable) noexcept
     }
 }
 
-void AtlasEngine::SetSelectionBackground(const COLORREF color, const float alpha) noexcept
+void AtlasEngine::SetSelectionBackground(const COLORREF color) noexcept
 {
-    const u32 selectionColor = (color & 0xffffff) | gsl::narrow_cast<u32>(lrintf(alpha * 255.0f)) << 24;
+    const u32 selectionColor = (color & 0xffffff) | 0xff000000;
     if (_api.s->misc->selectionColor != selectionColor)
     {
-        _api.s.write()->misc.write()->selectionColor = selectionColor;
+        auto misc = _api.s.write()->misc.write();
+        misc->selectionColor = selectionColor;
+        // Selection Foreground is based on the default foreground; it is also updated in UpdateDrawingBrushes
+        misc->selectionForeground = 0xff000000 | ColorFix::GetPerceivableColor(misc->foregroundColor, color, 0.5f * 0.5f);
     }
 }
 
diff --git a/src/renderer/atlas/AtlasEngine.cpp b/src/renderer/atlas/AtlasEngine.cpp
index 0f80bceb787..6aa8bff265a 100644
--- a/src/renderer/atlas/AtlasEngine.cpp
+++ b/src/renderer/atlas/AtlasEngine.cpp
@@ -11,6 +11,8 @@
 #include "DWriteTextAnalysis.h"
 #include "../../interactivity/win32/CustomWindowMessages.h"
 
+#include "../types/inc/ColorFix.hpp"
+
 // #### NOTE ####
 // This file should only contain methods that are only accessed by the caller of Present() (the "Renderer" class).
 // Basically this file poses the "synchronization" point between the concurrently running
@@ -311,6 +313,8 @@ CATCH_RETURN()
                 _api.searchHighlightFocused = { info.searchHighlightFocused, 1 };
             }
         }
+
+        _api.selectionSpans = til::point_span_subspan_within_rect(info.selectionSpans, dr);
     }
 
     return S_OK;
@@ -411,9 +415,10 @@ try
         const auto end = isFinalRow ? std::min(hiEnd.x + 1, x2) : x2;
         _fillColorBitmap(row, x1, end, fgColor, bgColor);
 
-        // Return early if we couldn't paint the whole region. We will resume
-        // from here in the next call.
-        if (!isFinalRow || end == x2)
+        // Return early if we couldn't paint the whole region (either this was not the last row, or
+        // it was the last row but the highlight ends outside of our x range.)
+        // We will resume from here in the next call.
+        if (!isFinalRow || hiEnd.x /*inclusive*/ >= x2 /*exclusive*/)
         {
             return S_OK;
         }
@@ -497,6 +502,7 @@ try
     // Apply the highlighting colors to the highlighted cells
     RETURN_IF_FAILED(_drawHighlighted(_api.searchHighlights, y, x, columnEnd, highlightFg, highlightBg));
     RETURN_IF_FAILED(_drawHighlighted(_api.searchHighlightFocused, y, x, columnEnd, highlightFocusFg, highlightFocusBg));
+    RETURN_IF_FAILED(_drawHighlighted(_api.selectionSpans, y, x, columnEnd, _api.s->misc->selectionForeground, _api.s->misc->selectionColor));
 
     _api.lastPaintBufferLineCoord = { x, y };
     return S_OK;
@@ -563,28 +569,9 @@ try
 CATCH_RETURN()
 
 [[nodiscard]] HRESULT AtlasEngine::PaintSelection(const til::rect& rect) noexcept
-try
 {
-    // Unfortunately there's no step after Renderer::_PaintBufferOutput that
-    // would inform us that it's done with the last AtlasEngine::PaintBufferLine.
-    // As such we got to call _flushBufferLine() here just to be sure.
-    _flushBufferLine();
-
-    const auto y = gsl::narrow_cast<u16>(clamp<til::CoordType>(rect.top, 0, _p.s->viewportCellCount.y - 1));
-    const auto from = gsl::narrow_cast<u16>(clamp<til::CoordType>(rect.left, 0, _p.s->viewportCellCount.x - 1));
-    const auto to = gsl::narrow_cast<u16>(clamp<til::CoordType>(rect.right, from, _p.s->viewportCellCount.x));
-
-    auto& row = *_p.rows[y];
-    row.selectionFrom = from;
-    row.selectionTo = to;
-
-    _p.dirtyRectInPx.left = std::min(_p.dirtyRectInPx.left, from * _p.s->font->cellSize.x);
-    _p.dirtyRectInPx.top = std::min(_p.dirtyRectInPx.top, y * _p.s->font->cellSize.y);
-    _p.dirtyRectInPx.right = std::max(_p.dirtyRectInPx.right, to * _p.s->font->cellSize.x);
-    _p.dirtyRectInPx.bottom = std::max(_p.dirtyRectInPx.bottom, _p.dirtyRectInPx.top + _p.s->font->cellSize.y);
     return S_OK;
 }
-CATCH_RETURN()
 
 [[nodiscard]] HRESULT AtlasEngine::PaintCursor(const CursorOptions& options) noexcept
 try
@@ -661,10 +648,21 @@ try
         _api.currentForeground = gsl::narrow_cast<u32>(fg);
         _api.attributes = attributes;
     }
-    else if (textAttributes.BackgroundIsDefault() && bg != _api.s->misc->backgroundColor)
+    else
     {
-        _api.s.write()->misc.write()->backgroundColor = bg;
-        _p.s.write()->misc.write()->backgroundColor = bg;
+        if (textAttributes.BackgroundIsDefault() && bg != _api.s->misc->backgroundColor)
+        {
+            _api.s.write()->misc.write()->backgroundColor = bg;
+            _p.s.write()->misc.write()->backgroundColor = bg;
+        }
+
+        if (textAttributes.GetForeground().IsDefault() && fg != _api.s->misc->foregroundColor)
+        {
+            auto misc = _api.s.write()->misc.write();
+            misc->foregroundColor = fg;
+            // Selection Foreground is based on the default foreground; it is also updated in SetSelectionColor
+            misc->selectionForeground = 0xff000000 | ColorFix::GetPerceivableColor(fg, misc->selectionColor, 0.5f * 0.5f);
+        }
     }
 
     return S_OK;
diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h
index 84ac6f601c6..b20bd04f4c2 100644
--- a/src/renderer/atlas/AtlasEngine.h
+++ b/src/renderer/atlas/AtlasEngine.h
@@ -71,7 +71,7 @@ namespace Microsoft::Console::Render::Atlas
         void SetPixelShaderPath(std::wstring_view value) noexcept;
         void SetPixelShaderImagePath(std::wstring_view value) noexcept;
         void SetRetroTerminalEffect(bool enable) noexcept;
-        void SetSelectionBackground(COLORREF color, float alpha = 0.5f) noexcept;
+        void SetSelectionBackground(COLORREF color) noexcept;
         void SetSoftwareRendering(bool enable) noexcept;
         void SetDisablePartialInvalidation(bool enable) noexcept;
         void SetGraphicsAPI(GraphicsAPI graphicsAPI) noexcept;
@@ -171,6 +171,7 @@ namespace Microsoft::Console::Render::Atlas
             // These tracks the highlighted regions on the screen that are yet to be painted.
             std::span<const til::point_span> searchHighlights;
             std::span<const til::point_span> searchHighlightFocused;
+            std::span<const til::point_span> selectionSpans;
 
             // dirtyRect is a computed value based on invalidatedRows.
             til::rect dirtyRect;
diff --git a/src/renderer/atlas/BackendD2D.cpp b/src/renderer/atlas/BackendD2D.cpp
index 8a7861ecd9b..c05dd1f21e3 100644
--- a/src/renderer/atlas/BackendD2D.cpp
+++ b/src/renderer/atlas/BackendD2D.cpp
@@ -51,7 +51,6 @@ void BackendD2D::Render(RenderingPayload& p)
         _drawCursorPart1(p);
         _drawText(p);
         _drawCursorPart2(p);
-        _drawSelection(p);
 #if ATLAS_DEBUG_SHOW_DIRTY
         _debugShowDirty(p);
 #endif
@@ -938,26 +937,6 @@ void BackendD2D::_drawCursor(const RenderingPayload& p, ID2D1RenderTarget* rende
     }
 }
 
-void BackendD2D::_drawSelection(const RenderingPayload& p)
-{
-    u16 y = 0;
-    for (const auto& row : p.rows)
-    {
-        if (row->selectionTo > row->selectionFrom)
-        {
-            const D2D1_RECT_F rect{
-                static_cast<f32>(p.s->font->cellSize.x * row->selectionFrom),
-                static_cast<f32>(p.s->font->cellSize.y * y),
-                static_cast<f32>(p.s->font->cellSize.x * row->selectionTo),
-                static_cast<f32>(p.s->font->cellSize.y * (y + 1)),
-            };
-            _fillRectangle(rect, p.s->misc->selectionColor);
-        }
-
-        y++;
-    }
-}
-
 #if ATLAS_DEBUG_SHOW_DIRTY
 void BackendD2D::_debugShowDirty(const RenderingPayload& p)
 {
diff --git a/src/renderer/atlas/BackendD3D.cpp b/src/renderer/atlas/BackendD3D.cpp
index 2e02e342863..6d832ae0d75 100644
--- a/src/renderer/atlas/BackendD3D.cpp
+++ b/src/renderer/atlas/BackendD3D.cpp
@@ -229,7 +229,6 @@ void BackendD3D::Render(RenderingPayload& p)
     _drawBackground(p);
     _drawCursorBackground(p);
     _drawText(p);
-    _drawSelection(p);
     _debugShowDirty(p);
     _flushQuads(p);
 
@@ -2248,45 +2247,6 @@ size_t BackendD3D::_drawCursorForegroundSlowPath(const CursorRect& c, size_t off
     return addedInstances;
 }
 
-void BackendD3D::_drawSelection(const RenderingPayload& p)
-{
-    u16 y = 0;
-    u16 lastFrom = 0;
-    u16 lastTo = 0;
-
-    for (const auto& row : p.rows)
-    {
-        if (row->selectionTo > row->selectionFrom)
-        {
-            // If the current selection line matches the previous one, we can just extend the previous quad downwards.
-            // The way this is implemented isn't very smart, but we also don't have very many rows to iterate through.
-            if (row->selectionFrom == lastFrom && row->selectionTo == lastTo)
-            {
-                _getLastQuad().size.y += p.s->font->cellSize.y;
-            }
-            else
-            {
-                _appendQuad() = {
-                    .shadingType = static_cast<u16>(ShadingType::Selection),
-                    .position = {
-                        static_cast<i16>(p.s->font->cellSize.x * row->selectionFrom),
-                        static_cast<i16>(p.s->font->cellSize.y * y),
-                    },
-                    .size = {
-                        static_cast<u16>(p.s->font->cellSize.x * (row->selectionTo - row->selectionFrom)),
-                        static_cast<u16>(p.s->font->cellSize.y),
-                    },
-                    .color = p.s->misc->selectionColor,
-                };
-                lastFrom = row->selectionFrom;
-                lastTo = row->selectionTo;
-            }
-        }
-
-        y++;
-    }
-}
-
 void BackendD3D::_debugShowDirty(const RenderingPayload& p)
 {
 #if ATLAS_DEBUG_SHOW_DIRTY
@@ -2302,7 +2262,7 @@ void BackendD3D::_debugShowDirty(const RenderingPayload& p)
     {
         const auto& rect = _presentRects[(_presentRectsPos + i) % std::size(_presentRects)];
         _appendQuad() = {
-            .shadingType = static_cast<u16>(ShadingType::Selection),
+            .shadingType = static_cast<u16>(ShadingType::FilledRect),
             .position = {
                 static_cast<i16>(rect.left),
                 static_cast<i16>(rect.top),
diff --git a/src/renderer/atlas/BackendD3D.h b/src/renderer/atlas/BackendD3D.h
index cb1bfcccea4..962d691b01d 100644
--- a/src/renderer/atlas/BackendD3D.h
+++ b/src/renderer/atlas/BackendD3D.h
@@ -76,7 +76,7 @@ namespace Microsoft::Console::Render::Atlas
             SolidLine,
 
             Cursor,
-            Selection,
+            FilledRect,
 
             TextDrawingFirst = TextGrayscale,
             TextDrawingLast = SolidLine,
diff --git a/src/renderer/atlas/common.h b/src/renderer/atlas/common.h
index 56cc1fce139..3f70ae6748e 100644
--- a/src/renderer/atlas/common.h
+++ b/src/renderer/atlas/common.h
@@ -393,7 +393,9 @@ namespace Microsoft::Console::Render::Atlas
     struct MiscellaneousSettings
     {
         u32 backgroundColor = 0;
-        u32 selectionColor = 0x7fffffff;
+        u32 foregroundColor = 0;
+        u32 selectionColor = 0xffffffff;
+        u32 selectionForeground = 0xff000000;
         std::wstring customPixelShaderPath;
         std::wstring customPixelShaderImagePath;
         bool useRetroTerminalEffect = false;
@@ -475,8 +477,6 @@ namespace Microsoft::Console::Render::Atlas
             bitmap.active = false;
             gridLineRanges.clear();
             lineRendition = LineRendition::SingleWidth;
-            selectionFrom = 0;
-            selectionTo = 0;
             dirtyTop = y * cellHeight;
             dirtyBottom = dirtyTop + cellHeight;
         }
@@ -496,8 +496,6 @@ namespace Microsoft::Console::Render::Atlas
         Bitmap bitmap;
         std::vector<GridLineRange> gridLineRanges;
         LineRendition lineRendition = LineRendition::SingleWidth;
-        u16 selectionFrom = 0;
-        u16 selectionTo = 0;
         til::CoordType dirtyTop = 0;
         til::CoordType dirtyBottom = 0;
     };
diff --git a/src/renderer/atlas/shader_common.hlsl b/src/renderer/atlas/shader_common.hlsl
index 51eb524b86b..18268407f26 100644
--- a/src/renderer/atlas/shader_common.hlsl
+++ b/src/renderer/atlas/shader_common.hlsl
@@ -16,7 +16,7 @@
 #define SHADING_TYPE_CURLY_LINE         7
 #define SHADING_TYPE_SOLID_LINE         8
 #define SHADING_TYPE_CURSOR             9
-#define SHADING_TYPE_SELECTION          10
+#define SHADING_TYPE_FILLED_RECT       10
 
 struct VSData
 {


Test patch: