Performance/Glitches in UI #22
Conversation
…different resolutions, this was happening cuz of the comapct album image height being too much to fit inside the notch when the notch is closed
There was a problem hiding this comment.
Pull Request Overview
This pull request addresses performance issues and UI glitches across the ComfyNotch application, focusing on optimizing panel proximity handling and refactoring the album widget for better display adaptation.
- PanelProximityHandler performance optimizations with throttling and early exit conditions
- Album widget refactoring to support dynamic sizing based on screen resolution
- Minor UI optimizations and code cleanup in notch view and shell script
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Scripts/stats_longest_file.sh | Code cleanup and optimization of the shell script with temporary file handling improvements |
| ComfyNotch/Widgets/CompactWidgets/CompactAlbumWidget.swift | Major refactor introducing dynamic sizing based on screen resolution and new WidgetSizeConfig struct |
| ComfyNotch/Managers/UIManager.swift | Minor comment cleanup in compact widget layout method |
| ComfyNotch/Main/ComfyNotchView.swift | State tracking optimization and commented code removal |
| ComfyNotch/Handlers/PanelProximityHandler.swift | Performance optimizations with throttling and streamlined condition checking |
Comments suppressed due to low confidence (2)
ComfyNotch/Widgets/CompactWidgets/CompactAlbumWidget.swift:42
- Variable names 'w' and 'h' are too abbreviated and unclear. Consider using more descriptive names like 'width' and 'height'.
let w = resolution.width
ComfyNotch/Widgets/CompactWidgets/CompactAlbumWidget.swift:43
- Variable names 'w' and 'h' are too abbreviated and unclear. Consider using more descriptive names like 'width' and 'height'.
let h = resolution.height
|
|
||
| print("🖥️ Resolution: \(Int(w))x\(Int(h)), Expanded: \(isExpanded)") |
There was a problem hiding this comment.
Debug print statement should be removed from production code. This will cause console spam and impact performance.
| print("🖥️ Resolution: \(Int(w))x\(Int(h)), Expanded: \(isExpanded)") | |
| // Debug print statement removed |
| if w < 2800 { | ||
| return isExpanded ? .init(width: 20, height: 20) : .init(width: 15, height: 14) | ||
| } else if w <= 3500 { |
There was a problem hiding this comment.
Magic numbers (2800, 3500) should be extracted as named constants to improve maintainability and explain their significance.
| if w < 2800 { | |
| return isExpanded ? .init(width: 20, height: 20) : .init(width: 15, height: 14) | |
| } else if w <= 3500 { | |
| if w < ScreenWidthThresholds.low { | |
| return isExpanded ? .init(width: 20, height: 20) : .init(width: 15, height: 14) | |
| } else if w <= ScreenWidthThresholds.high { |
| /// Throttling properties | ||
| private var lastProcessedTime: TimeInterval = 0 | ||
| /// ~30fps (0.016 for 60) | ||
| private let throttleInterval: TimeInterval = 0.033 |
There was a problem hiding this comment.
Magic number 0.033 should be extracted as a named constant or calculated from a frame rate constant to make the relationship clearer.
| private let throttleInterval: TimeInterval = 0.033 | |
| private let frameRate: Double = 30.0 | |
| private let throttleInterval: TimeInterval = 1.0 / frameRate |
| } | ||
|
|
||
|
|
||
| func widgetSize() -> WidgetSizeConfig { |
There was a problem hiding this comment.
The widgetSize() function is called on every body evaluation and performs expensive screen queries. Consider caching the result or using a computed property that only recalculates when screen configuration changes.
PanelProximity Optimizations,
Small Optimizations to Main Notch
Album Widget gets a major refactor for snapping/glitches in the UI