Remove getApp and getSettings calls from Message-Rendering
#4535
Conversation
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
originally in this pr before I removed that change :-)
pajlada
approved these changes
Jul 30, 2023
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation
I wanted to try and experiment if #1618 would be possible to implement. My idea was to have a hotkey similar to CTRL+N that brings up the current channel in a transparent window (⇒ this wouldn't support splits yet (?)). This window would be essentially a channel-view-wrapping widget. The wrapped channel-view would then be considered "transparent" and it wouldn't draw a background. But it turns out that a lot of places throughout the rendering pipeline draw some sort of background. From the
ChannelViewthere are two locations that draw a background:ChannelView- this is easy to change (I don't think it's really needed at all)MessageLayout- this isn't easy to change! While theoretically, one could pass yet another parameter toMessageLayout::paint, this isn't a healthy path forward.Changes
I added a
MessagePaintContextstruct which encapsulates all current rendering settings/colors as well as state from theChannelView(e.g. the message index or y-position). This context gets passed to all rendering methods and supplies e.g. colors (instead of going through the globalApplicationsingleton). This is only done for painting currently. In the future, this could be done for layout as well, which could open up possibilities to test the rendering without mockingApplicationandSettings(there are probably some more locations using these singletons).