forked from microsoft/terminal
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Eliminate the IRenderTarget interface (microsoft#12568)
## Summary of the Pull Request The purpose of the `IRenderTarget` interface was to support the concept of multiple buffers in conhost. When a text buffer needed to trigger a redraw, the render target implementation would be responsible for deciding whether to forward that redraw to the renderer, depending on whether the buffer was active or not. This PR instead introduces a flag in the `TextBuffer` class to track whether it is active or not, and it can then simply check the flag to decide whether it needs to trigger a redraw or not. That way it can work with the `Renderer` directly, and we can avoid a bunch of virtual calls for the various redraw methods. ## PR Checklist * [x] Closes microsoft#12551 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [x] I've discussed this with core contributors already. Issue number where discussion took place: microsoft#12551 ## Detailed Description of the Pull Request / Additional comments Anywhere that had previously been getting an `IRenderTarget` from the `TextBuffer` class in order to trigger a redraw, will now just call the `TriggerRedraw` method on the `TextBuffer` class itself, since that will handle the active check which used to be the responsibility of the render target. All the redraw methods that were in `IRenderTarget` are now exposed in `TextBuffer` instead. For this to work, though, the `Renderer` needed to be available before the `TextBuffer` could be constructed, which required a change in the conhost initialization order. In the `ConsoleInitializeConnectInfo` function, I had to move the `Renderer` initialization up so it could be constructed before the call to `SetUpConsole` (which initializes the buffer). Once both are ready, the `Renderer::EnablePainting` method is called to start the render thread. The other catch is that the renderer used to setup its initial viewport in the constructor, but with the new initialization order, the viewport size would not be known at that point. I've addressed that problem by moving the viewport initialization into the `EnablePainting` method, since that will only be called after the buffer is setup. ## Validation Steps Performed The changes in architecture required a number of tweaks to the unit tests. Some were using a dummy `IRenderTarget` implementation which now needed to be replaced with a full `Renderer` (albeit with mostly null parameters). In the case of the scroll test, a mock `IRenderTarget` was used to track the scroll delta, which now had to be replaced with a mock `RenderEngine` instead. Some tests previously relied on having just a `TextBuffer` but without a `Renderer`, and now they require both. And tests that were constructing the `TextBuffer` and `Renderer` themselves had to be updated to use the new initialization order, i.e. with the renderer constructed first. Semantically, though, the tests still essentially work the same way as they did before, and they all still pass.
- Loading branch information
Showing
45 changed files
with
435 additions
and
535 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.