starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

[PR #12127] [MERGED] Move the common render settings into a shared class #28857

New Issue
Open
opened 2026-01-31 09:31:14 +00:00 by claunia · 0 comments
claunia commented 2026-01-31 09:31:14 +00:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/12127
Author: @j4james
Created: 1/10/2022
Status: ✅ Merged
Merged: 1/13/2022
Merged by: @undefined

Base: main ← Head: render-settings

📝 Commits (10+)

  • 6d9c1d0 Introduce a class for managing runtime render settings.
  • 1006686 Use RenderSettings where possible in the renderer.
  • 474e35c Integrate the RenderSettings into conhost.
  • 98c01b3 Update conhost unit tests.
  • dc1c9a9 Incorporate the color adjustment algorithm into RenderSettings.
  • 8a53a7d Integrate the RenderSettings into Windows Terminal.
  • 72514c0 Remove unused IRenderData methods.
  • fbe4bae Remove unused BlinkingState class.
  • 44ed8ed Replace SetScreenMode in ConGetSet and ITerminalApi.
  • 2dbb6ee Convert the "intense is bold" option into a mode.

📊 Changes

79 files changed (+902 additions, -924 deletions)

View changed files

📝 src/buffer/out/TextAttribute.cpp (+0 -35)
📝 src/buffer/out/TextAttribute.hpp (+0 -7)
📝 src/buffer/out/TextColor.h (+7 -0)
📝 src/buffer/out/ut_textbuffer/TextAttributeTests.cpp (+70 -66)
📝 src/buffer/out/ut_textbuffer/TextBuffer.Unit.Tests.vcxproj (+3 -0)
📝 src/cascadia/PublicTerminalCore/HwndTerminal.cpp (+2 -1)
📝 src/cascadia/TerminalControl/ControlCore.cpp (+5 -6)
📝 src/cascadia/TerminalControl/IControlAppearance.idl (+1 -2)
📝 src/cascadia/TerminalCore/ICoreAppearance.idl (+1 -0)
📝 src/cascadia/TerminalCore/ITerminalApi.hpp (+3 -1)
📝 src/cascadia/TerminalCore/Terminal.cpp (+54 -85)
📝 src/cascadia/TerminalCore/Terminal.hpp (+10 -18)
📝 src/cascadia/TerminalCore/TerminalApi.cpp (+18 -5)
📝 src/cascadia/TerminalCore/TerminalDispatch.cpp (+4 -1)
📝 src/cascadia/TerminalCore/TerminalSelection.cpp (+3 -1)
📝 src/cascadia/TerminalCore/terminalcore-common.vcxitems (+0 -2)
📝 src/cascadia/TerminalCore/terminalrenderdata.cpp (+5 -102)
📝 src/cascadia/TerminalSettingsModel/TerminalSettings.h (+1 -1)
📝 src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp (+1 -1)
📝 src/cascadia/inc/ControlProperties.h (+1 -1)

...and 59 more files

📄 Description

Summary of the Pull Request

This PR moves the color table and related render settings, which are common to both conhost and Windows Terminal, into a shared class that can be accessed directly from the renderer. This avoids the overhead of having to look up these properties via the IRenderData interface, which relies on inefficient virtual function calls.

This also introduces the concept of color aliases, which determine the position in the color table that colors like the default foreground and background are stored. This allows the option of mapping them to one of the standard 16 colors, or to have their own separate table entries.

References

This is a continuation of the color table refactoring started in #11602 and #11784. The color alias functionality is a prerequisite for supporting a default bold color as proposed in #11939. The color aliases could also be a way for us to replace the PowerShell color quirk for #6807.

PR Checklist

  • Closes Add option to add Windows Terminal to File Explorer (#12002)
  • CLA signed.
  • Tests added/passed
  • Documentation updated.
  • Schema updated.
  • 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: #xxx

Detailed Description of the Pull Request / Additional comments

In addition to the color table, this new RenderSettings class manages the blinking state, the code for adjusting indistinguishable colors, and various boolean properties used in the color calculations. These boolean properties are now stored in a til::enumset so they can all be managed through a single SetRenderMode API, and easily extended with additional modes that we're likely to need in the future.

In Windows Terminal we have an instance of RenderSettings stored in the Terminal class, and in conhost it's stored in the Settings class. In both cases, a reference to this class is passed to the Renderer constructor, so it now has direct access to that data. The renderer can then pass that reference to the render engines where it's needed in the UpdateDrawingBrushes method.

This means the renderer no longer needs the IRenderData interface to access the GetAttributeColors, GetCursorColor, or IsScreenReversed methods, so those have now been removed. We still need access to GetAttributeColors in certain accessibility code, though, so I've kept that method in the IUIAData interface, but the implementation just forwards to the RenderSettings class.

The implementation of the RenderSettings::GetAttributeColors method is loosely based on the original Terminal code, only the CalculateRgbColors call has now been incorporated directly into the code. This let us deduplicate some bits that were previously repeated in the section for adjusting indistinguishable colors. The last steps, where we calculate the alpha components, have now been split in to a separate GetAttributeColorsWithAlpha method, since that's typically not needed.

Validation Steps Performed

There were quite a lot changes needed in the unit tests, but they're mostly straightforward replacements of one method call with another.

In the TextAttributeTests, where we were previously testing the CalculateRgbColors method, we're now running those tests though RenderSettings::GetAttributeColors, which incorporates the same functionality. The only complication is when testing the IntenseIsBright option, that needs to be set with an additional SetRenderMode call where previously it was just a parameter on CalculateRgbColors.

In the ScreenBufferTests and TextBufferTests, calls to LookupAttributeColors have again been replaced by the RenderSettings::GetAttributeColors method, which serves the same purpose, and calls to IsScreenReversed have been replaced with an appropriate GetRenderMode call. In the VtRendererTests, all the calls to UpdateDrawingBrushes now just need to be passed a reference to a RenderSettings instance.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/12127 **Author:** [@j4james](https://github.com/j4james) **Created:** 1/10/2022 **Status:** ✅ Merged **Merged:** 1/13/2022 **Merged by:** [@undefined](undefined) **Base:** `main` ← **Head:** `render-settings` --- ### 📝 Commits (10+) - [`6d9c1d0`](https://github.com/microsoft/terminal/commit/6d9c1d0e386ba076bf571aae37ac483e8f1d7c98) Introduce a class for managing runtime render settings. - [`1006686`](https://github.com/microsoft/terminal/commit/1006686ee9d7afcccf89e0083c288db8a7dfb44c) Use RenderSettings where possible in the renderer. - [`474e35c`](https://github.com/microsoft/terminal/commit/474e35cfbfc6dfa5ad6209ec02b0d28153b502f7) Integrate the RenderSettings into conhost. - [`98c01b3`](https://github.com/microsoft/terminal/commit/98c01b343a97bd5650d2ad41de8471a3a4cde735) Update conhost unit tests. - [`dc1c9a9`](https://github.com/microsoft/terminal/commit/dc1c9a975d5ec768c582b0bba9efed2c01722146) Incorporate the color adjustment algorithm into RenderSettings. - [`8a53a7d`](https://github.com/microsoft/terminal/commit/8a53a7dcbe2047320e0e0260ddb8580147445583) Integrate the RenderSettings into Windows Terminal. - [`72514c0`](https://github.com/microsoft/terminal/commit/72514c09b08a6a5824cf5fa59350df5b7b12285d) Remove unused IRenderData methods. - [`fbe4bae`](https://github.com/microsoft/terminal/commit/fbe4baeaa9f77a522fe67fb27f392e4381e7af27) Remove unused BlinkingState class. - [`44ed8ed`](https://github.com/microsoft/terminal/commit/44ed8ed78ce4503c874f7a3b6f42ccf84779b1a3) Replace SetScreenMode in ConGetSet and ITerminalApi. - [`2dbb6ee`](https://github.com/microsoft/terminal/commit/2dbb6ee2b89513cd99c522d3e0f62fc7f3c407d4) Convert the "intense is bold" option into a mode. ### 📊 Changes **79 files changed** (+902 additions, -924 deletions) <details> <summary>View changed files</summary> 📝 `src/buffer/out/TextAttribute.cpp` (+0 -35) 📝 `src/buffer/out/TextAttribute.hpp` (+0 -7) 📝 `src/buffer/out/TextColor.h` (+7 -0) 📝 `src/buffer/out/ut_textbuffer/TextAttributeTests.cpp` (+70 -66) 📝 `src/buffer/out/ut_textbuffer/TextBuffer.Unit.Tests.vcxproj` (+3 -0) 📝 `src/cascadia/PublicTerminalCore/HwndTerminal.cpp` (+2 -1) 📝 `src/cascadia/TerminalControl/ControlCore.cpp` (+5 -6) 📝 `src/cascadia/TerminalControl/IControlAppearance.idl` (+1 -2) 📝 `src/cascadia/TerminalCore/ICoreAppearance.idl` (+1 -0) 📝 `src/cascadia/TerminalCore/ITerminalApi.hpp` (+3 -1) 📝 `src/cascadia/TerminalCore/Terminal.cpp` (+54 -85) 📝 `src/cascadia/TerminalCore/Terminal.hpp` (+10 -18) 📝 `src/cascadia/TerminalCore/TerminalApi.cpp` (+18 -5) 📝 `src/cascadia/TerminalCore/TerminalDispatch.cpp` (+4 -1) 📝 `src/cascadia/TerminalCore/TerminalSelection.cpp` (+3 -1) 📝 `src/cascadia/TerminalCore/terminalcore-common.vcxitems` (+0 -2) 📝 `src/cascadia/TerminalCore/terminalrenderdata.cpp` (+5 -102) 📝 `src/cascadia/TerminalSettingsModel/TerminalSettings.h` (+1 -1) 📝 `src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp` (+1 -1) 📝 `src/cascadia/inc/ControlProperties.h` (+1 -1) _...and 59 more files_ </details> ### 📄 Description ## Summary of the Pull Request This PR moves the color table and related render settings, which are common to both conhost and Windows Terminal, into a shared class that can be accessed directly from the renderer. This avoids the overhead of having to look up these properties via the `IRenderData` interface, which relies on inefficient virtual function calls. This also introduces the concept of color aliases, which determine the position in the color table that colors like the default foreground and background are stored. This allows the option of mapping them to one of the standard 16 colors, or to have their own separate table entries. ## References This is a continuation of the color table refactoring started in #11602 and #11784. The color alias functionality is a prerequisite for supporting a default bold color as proposed in #11939. The color aliases could also be a way for us to replace the PowerShell color quirk for #6807. ## PR Checklist * [x] Closes #12002 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] 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: #xxx ## Detailed Description of the Pull Request / Additional comments In addition to the color table, this new `RenderSettings` class manages the blinking state, the code for adjusting indistinguishable colors, and various boolean properties used in the color calculations. These boolean properties are now stored in a `til::enumset` so they can all be managed through a single `SetRenderMode` API, and easily extended with additional modes that we're likely to need in the future. In Windows Terminal we have an instance of `RenderSettings` stored in the `Terminal` class, and in conhost it's stored in the `Settings` class. In both cases, a reference to this class is passed to the `Renderer` constructor, so it now has direct access to that data. The renderer can then pass that reference to the render engines where it's needed in the `UpdateDrawingBrushes` method. This means the renderer no longer needs the `IRenderData` interface to access the `GetAttributeColors`, `GetCursorColor`, or `IsScreenReversed` methods, so those have now been removed. We still need access to `GetAttributeColors` in certain accessibility code, though, so I've kept that method in the `IUIAData` interface, but the implementation just forwards to the `RenderSettings` class. The implementation of the `RenderSettings::GetAttributeColors` method is loosely based on the original `Terminal` code, only the `CalculateRgbColors` call has now been incorporated directly into the code. This let us deduplicate some bits that were previously repeated in the section for adjusting indistinguishable colors. The last steps, where we calculate the alpha components, have now been split in to a separate `GetAttributeColorsWithAlpha` method, since that's typically not needed. ## Validation Steps Performed There were quite a lot changes needed in the unit tests, but they're mostly straightforward replacements of one method call with another. In the `TextAttributeTests`, where we were previously testing the `CalculateRgbColors` method, we're now running those tests though `RenderSettings::GetAttributeColors`, which incorporates the same functionality. The only complication is when testing the `IntenseIsBright` option, that needs to be set with an additional `SetRenderMode` call where previously it was just a parameter on `CalculateRgbColors`. In the `ScreenBufferTests` and `TextBufferTests`, calls to `LookupAttributeColors` have again been replaced by the `RenderSettings::GetAttributeColors` method, which serves the same purpose, and calls to `IsScreenReversed` have been replaced with an appropriate `GetRenderMode` call. In the `VtRendererTests`, all the calls to `UpdateDrawingBrushes` now just need to be passed a reference to a `RenderSettings` instance. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:31:14 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#28857
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?