-
-
Notifications
You must be signed in to change notification settings - Fork 10
优化侧边栏和列表性能 #80
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
base: master
Are you sure you want to change the base?
优化侧边栏和列表性能 #80
Conversation
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.
Pull request overview
This PR optimizes the performance of the sidebar and list controls in the Context Menu Manager application by implementing asynchronous loading for shell items and refactoring the sidebar rendering approach.
Changes:
- Implemented asynchronous loading for ShellList items using Task.Run with cancellation support
- Added a new ShellItem constructor that accepts pre-computed data to avoid redundant registry reads
- Refactored MySideBar rendering from panel-based to direct painting approach
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 40 comments.
| File | Description |
|---|---|
| ContextMenuManager/MainForm.cs | Added ItemsLoaded event handler to coordinate search filtering with async list loading |
| ContextMenuManager/Controls/ShellList.cs | Implemented async loading with CancellationTokenSource, added helper methods to pre-compute item data, and introduced ItemsLoaded event |
| ContextMenuManager/Controls/ShellItem.cs | Added optimized constructor accepting pre-computed text, image, and multi-item flag to bypass expensive registry operations |
| ContextMenuManager/BluePointLilac.Controls/MySideBar.cs | Refactored from panel-based rendering to OnPaint-based approach, consolidated code, and abbreviated variable names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private bool isMultiItem(string regPath) // helper | ||
| { | ||
| return GetIsMultiItem(regPath); | ||
| } | ||
|
|
Copilot
AI
Jan 27, 2026
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.
The helper method isMultiItem is redundant and adds no value - it simply calls GetIsMultiItem(regPath). This wrapper method should be removed and callers should directly use GetIsMultiItem instead. This eliminates unnecessary indirection and reduces maintainability burden.
| private bool isMultiItem(string regPath) // helper | |
| { | |
| return GetIsMultiItem(regPath); | |
| } |
| private readonly Timer animTimer = new() { Interval = 16 }; | ||
| private string[] itemNames; | ||
| private int itemHeight = 36; | ||
| private int selectIndex = -1; | ||
| private int hoverIndex = -1; | ||
|
|
||
| private int animationTargetIndex = -1; | ||
| private int animationCurrentIndex = -1; | ||
| private float animationProgress = 0f; | ||
| private const float ANIMATION_SPEED = 0.25f; | ||
| private int itemHeight = 36, selectIndex = -1, hoverIndex = -1; | ||
| private int animTarget = -1, animCurrent = -1; | ||
| private float animProgress = 0f, curSelTop = -1; |
Copilot
AI
Jan 27, 2026
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.
Poor variable naming: Variable names have been excessively abbreviated (e.g., animTimer instead of animationTimer, animTarget/animCurrent instead of animationTargetIndex/animationCurrentIndex, curSelTop instead of currentSelectionTop). While shorter names can sometimes improve readability, these abbreviations make the code harder to understand, especially for developers unfamiliar with the codebase. The original names were clearer and self-documenting. Consider reverting to more descriptive names or at least using less cryptic abbreviations.
| void DrawItem(int idx, Color back, Color fore, float y) | ||
| { | ||
| brush.InterpolationColors = new ColorBlend | ||
| if (idx < 0 || idx >= ItemNames.Length) return; | ||
| var r = new RectangleF(0, y, Width, ItemHeight); | ||
| if (back == Color.Transparent) | ||
| { | ||
| Colors = new[] { SelectedGradientColor1, SelectedGradientColor2, SelectedGradientColor3 }, | ||
| Positions = new[] { 0f, 0.5f, 1f } | ||
| }; | ||
| e.Graphics.FillRectangle(brush, new Rectangle(0, 0, ctr.Width, ctr.Height)); | ||
| using var b = new LinearGradientBrush(r, Color.Empty, Color.Empty, 0f) { InterpolationColors = new ColorBlend { Colors = new[] { SelectedGradientColor1, SelectedGradientColor2, SelectedGradientColor3 }, Positions = new[] { 0f, 0.5f, 1f } } }; | ||
| e.Graphics.FillRectangle(b, r); | ||
| } | ||
| else { using var b = new SolidBrush(back); e.Graphics.FillRectangle(b, r); } | ||
| e.Graphics.SmoothingMode = SmoothingMode.AntiAlias; e.Graphics.TextRenderingHint = System.Drawing.Text.TextRenderingHint.ClearTypeGridFit; | ||
| using var fb = new SolidBrush(fore == Color.Empty ? ForeColor : fore); | ||
| e.Graphics.DrawString(ItemNames[idx], Font, fb, HorizontalSpace, y + vSpace); |
Copilot
AI
Jan 27, 2026
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.
Potential null reference exception: The local function DrawItem (line 155) doesn't check if ItemNames[idx] is null before using it in DrawString (line 167). While the callers at lines 170-171 should only pass valid indices, if ItemNames[idx] is null (which is used for separators elsewhere), this will throw an exception. Add a null check before drawing the string, or ensure the calling code never passes indices where ItemNames[idx] is null.
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -15); | ||
| } | ||
| else if (hasLUAShield) | ||
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -78); | ||
| else | ||
| icon = ResourceIcon.GetIcon(iconPath = itemFilePath, iconIndex = 0); | ||
|
|
||
| if (icon == null) | ||
| icon = ResourceIcon.GetExtensionIcon(iconPath = itemFilePath) ?? ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -2); |
Copilot
AI
Jan 27, 2026
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.
This assignment to iconPath is useless, since its value is never read.
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -15); | |
| } | |
| else if (hasLUAShield) | |
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -78); | |
| else | |
| icon = ResourceIcon.GetIcon(iconPath = itemFilePath, iconIndex = 0); | |
| if (icon == null) | |
| icon = ResourceIcon.GetExtensionIcon(iconPath = itemFilePath) ?? ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -2); | |
| icon = ResourceIcon.GetIcon("imageres.dll", iconIndex: -15); | |
| } | |
| else if (hasLUAShield) | |
| icon = ResourceIcon.GetIcon("imageres.dll", iconIndex: -78); | |
| else | |
| icon = ResourceIcon.GetIcon(itemFilePath, iconIndex: 0); | |
| if (icon == null) | |
| icon = ResourceIcon.GetExtensionIcon(itemFilePath) ?? ResourceIcon.GetIcon("imageres.dll", iconIndex: -2); |
| else if (hasLUAShield) | ||
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -78); | ||
| else | ||
| icon = ResourceIcon.GetIcon(iconPath = itemFilePath, iconIndex = 0); |
Copilot
AI
Jan 27, 2026
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.
This assignment to iconIndex is useless, since its value is never read.
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -15); | ||
| } | ||
| else if (hasLUAShield) | ||
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -78); | ||
| else | ||
| icon = ResourceIcon.GetIcon(iconPath = itemFilePath, iconIndex = 0); | ||
|
|
||
| if (icon == null) | ||
| icon = ResourceIcon.GetExtensionIcon(iconPath = itemFilePath) ?? ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -2); |
Copilot
AI
Jan 27, 2026
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.
This assignment to iconPath is useless, since its value is never read.
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -15); | |
| } | |
| else if (hasLUAShield) | |
| icon = ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -78); | |
| else | |
| icon = ResourceIcon.GetIcon(iconPath = itemFilePath, iconIndex = 0); | |
| if (icon == null) | |
| icon = ResourceIcon.GetExtensionIcon(iconPath = itemFilePath) ?? ResourceIcon.GetIcon(iconPath = "imageres.dll", iconIndex = -2); | |
| icon = ResourceIcon.GetIcon("imageres.dll", iconIndex: -15); | |
| } | |
| else if (hasLUAShield) | |
| icon = ResourceIcon.GetIcon("imageres.dll", iconIndex: -78); | |
| else | |
| icon = ResourceIcon.GetIcon(itemFilePath, iconIndex: 0); | |
| if (icon == null) | |
| icon = ResourceIcon.GetExtensionIcon(itemFilePath) ?? ResourceIcon.GetIcon("imageres.dll", iconIndex: -2); |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
No description provided.