Freeze SolidColorBrush to improve performance#4319
Conversation
Refactor background brush creation to assign, freeze, and reuse SolidColorBrush instances for better WPF performance. Replace string-based property checks with type-safe Control.BackgroundProperty comparisons for improved robustness.
|
🥷 Code experts: jjw24 jjw24 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR aims to improve WPF performance and safety by freezing SolidColorBrush instances that are created for theme/window background rendering, reducing mutability and improving rendering efficiency.
Changes:
- Freeze newly created
SolidColorBrushinstances before assigning them toStylesetters in blur scenarios. - Freeze
SolidColorBrushused for the preview window background style. - Freeze
SolidColorBrushassigned toMainWindow.Backgroundwhen blur is available.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughThe Theme.cs file was refactored to improve brush immutability and property matching. Property matching logic for Background was updated to compare against Control.BackgroundProperty instead of checking Property.Name. SolidColorBrush instances are now frozen before assignment across multiple code paths including SetBlurForWindow, ApplyPreviewBackground, and ColorizeWindow methods. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Flow.Launcher.Core/Resource/Theme.cs (1)
676-686: Cache constant frozen brushes instead of recreating them on each refresh.Line 677, Line 684, and Line 926 still allocate new
SolidColorBrushinstances for fixed colors. Reusing class-level frozen singletons would better match the PR’s reuse/perf goal and remove repeated allocations.♻️ Suggested refactor
+ private static readonly SolidColorBrush MicaTransparentBrush = CreateFrozenBrush(Color.FromArgb(1, 0, 0, 0)); + private static readonly SolidColorBrush TransparentBrush = CreateFrozenBrush(Colors.Transparent); + + private static SolidColorBrush CreateFrozenBrush(Color color) + { + var brush = new SolidColorBrush(color); + brush.Freeze(); + return brush; + } @@ - var brush = new SolidColorBrush(Color.FromArgb(1, 0, 0, 0)); - brush.Freeze(); - windowBorderStyle.Setters.Add(new Setter(Border.BackgroundProperty, brush)); + windowBorderStyle.Setters.Add(new Setter(Border.BackgroundProperty, MicaTransparentBrush)); @@ - var brush = new SolidColorBrush(Colors.Transparent); - brush.Freeze(); - windowBorderStyle.Setters.Add(new Setter(Border.BackgroundProperty, brush)); + windowBorderStyle.Setters.Add(new Setter(Border.BackgroundProperty, TransparentBrush)); @@ - var backgroundBrush = new SolidColorBrush(Color.FromArgb(1, 0, 0, 0)); - backgroundBrush.Freeze(); - mainWindow.Background = backgroundBrush; + mainWindow.Background = MicaTransparentBrush;Also applies to: 926-934
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher.Core/Resource/Theme.cs` around lines 676 - 686, Create and reuse class-level frozen SolidColorBrush singletons for the fixed colors instead of allocating new instances each refresh: add readonly static fields (e.g., FrozenAlmostTransparent = new SolidColorBrush(Color.FromArgb(1,0,0,0)).Freeze() and FrozenTransparent = new SolidColorBrush(Colors.Transparent).Freeze()) and replace the local allocations where windowBorderStyle.Setters.Add(new Setter(Border.BackgroundProperty, brush)) is used (the blocks around the current SolidColorBrush creations and the similar code in the region referenced at 926-934) to reference these frozen fields; ensure the fields are frozen once at initialization and used everywhere the same constant brushes are needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Flow.Launcher.Core/Resource/Theme.cs`:
- Around line 676-686: Create and reuse class-level frozen SolidColorBrush
singletons for the fixed colors instead of allocating new instances each
refresh: add readonly static fields (e.g., FrozenAlmostTransparent = new
SolidColorBrush(Color.FromArgb(1,0,0,0)).Freeze() and FrozenTransparent = new
SolidColorBrush(Colors.Transparent).Freeze()) and replace the local allocations
where windowBorderStyle.Setters.Add(new Setter(Border.BackgroundProperty,
brush)) is used (the blocks around the current SolidColorBrush creations and the
similar code in the region referenced at 926-934) to reference these frozen
fields; ensure the fields are frozen once at initialization and used everywhere
the same constant brushes are needed.
ℹ️ Review info
Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea83e89b-ddb2-4ede-a1ae-202bbb0172b9
📒 Files selected for processing (1)
Flow.Launcher.Core/Resource/Theme.cs
Refactor background brush creation to assign, freeze, and reuse SolidColorBrush instances for better WPF performance.
Separated from #4302
Summary by cubic
Freeze SolidColorBrush instances and switch to type-safe Background property checks to improve WPF performance and robustness. This reduces allocations and avoids unnecessary mutable brushes across Theme.cs.
Written for commit 9abf136. Summary will update on new commits.