-
Notifications
You must be signed in to change notification settings - Fork 1k
Address Graphics state reset issue in DarkMode Button renderers #13641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Address Graphics state reset issue in DarkMode Button renderers #13641
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13641 +/- ##
===================================================
+ Coverage 76.75572% 76.75863% +0.00290%
===================================================
Files 3256 3256
Lines 642016 642019 +3
Branches 47548 47548
===================================================
+ Hits 492784 492805 +21
+ Misses 145574 145554 -20
- Partials 3658 3660 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See note on the scope
@@ -44,39 +45,50 @@ public void RenderButton( | |||
ArgumentNullException.ThrowIfNull(paintImage); | |||
ArgumentNullException.ThrowIfNull(paintField); | |||
|
|||
// Clear the background over the whole button area. | |||
ClearBackground(graphics, parentBackgroundColor); | |||
GraphicsState? graphicsState = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the GraphicsStateScope
here and elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's assumingly the same approach with PenScope and BrushScope, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar, but simpler. Should be a lot of other uses. We have scopes for most things that need cleaned up or reset. Reduces nesting and ensures ordering. There are also opportunities to further optimize in some cases as well now that we control the internals of System.Drawing (could, for example, avoid creating wrapper classes in some cases,).
5e106ae
to
dbb74b0
Compare
a3468e5
to
a481476
Compare
a481476
to
0d940b7
Compare
|
||
if (focused && showFocusCues) | ||
// Scope the graphics state so all changes are reverted after rendering | ||
using (new GraphicsStateScope(graphics)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using (new GraphicsStateScope(graphics)) | |
using GraphicsStateScope graphicsScope = new(graphics); |
Out on vacation, and we need to get this merged.
This PR makes sure, that the Graphics state is reset to the original state after rendering the DarkMode Button but implementing the render logic based on
ButtonDarkModeRendererBase
.Follows up respective issues pointed out by the DarkMode feature reviews.
Microsoft Reviewers: Open in CodeFlow