TextAreaContent: remove COLOR_RESET uses with Pen - #5874
Open
ChrisJohnsen wants to merge 1 commit into
Open
Conversation
COLOR_RESET should not be used with Pen. The first use looks like it could be a "restore Pen before ending" kind of action; but that should be unnecessary. - The only other drawing done after this in this function is debug-only drawing that sets its own colors. - Any subsequently rendered Views should fully establish their own Pen settings before drawing. A view's `onRenderBody`'s Painter (`dc`, here) *is* normally reused (primarily for the clipping rectangle?) with the subview's `onRenderFrame`, but it would be terribly fragile for a subview to rely on the Pen state left by its parent view (or earlier-rendered sibling views's frame). - TextAreaContent does not have any subviews. The second use is only used when `self.debug` is active, the background color is probably unimportant; use Pen's default background color: black.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Ref: #5864
There are a couple of Pen uses of COLOR_RESET remaining in TextAreaContent. Both are used as background colors in
onRenderBodyand neither is normally used to draw anything.The first use of COLOR_RESET had a slight change in 17c1dc0, but nothing significant with respect to how COLOR_RESET was being used.
This use looks to be intended to "restore" the Painter's Pen state. This "restored" Pen is never actually used anywhere though:
dc) toonRenderBodywould be reused and passed to subviews'onRenderFrame(seeView:render()), but TextAreaContent has no subviews.(In general, I don't think widgets/views should assume anything about the Pen state of the Painter they are given (unless they are highly coupled).)
Suggestion: Remove the "restore" adjustment.
The second use of COLOR_RESET seems to be unchanged since it was introduced.
This use of COLOR_RESET only happens when
self.debugis enabled (it doesn't seem to be enabled in any normal situation). The value of the debug field is inherited from its parent TextArea at initialization time. TextArea's debug defaults to false and doesn't seem to be set by any other Lua code. Settingdebug=truein the TextArea of (e.g.)gui/journalis a quick way to see this debug code activated.Suggestion: Simplify to just specifying a foreground color. The default background color (black) will be used. It probably doesn't really matter much what the background color is (since it is only for debug). This can produce no visible changes unless
debugis enabled.