Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #1 where models do not immediately load in the UI after downloading. The fix ensures that whenever a model is downloaded, imported, or converted, a callback is triggered to refresh the model lists across all task panels (Generate, Img2Img, Upscale). Additionally, the PR introduces a comprehensive logging infrastructure with a new Logs panel, enhances GPU execution provider selection with support for TensorRT and OpenVINO, and improves the build system with GitHub Actions CI/CD.
Changes:
- Fixed model loading by invoking
onModelsUpdatedcallback after download, import, and conversion operations - Introduced centralized
AppLoggerwith two channels (APPLICATION, MODEL) and a new Logs panel UI for real-time log viewing - Expanded GPU execution provider support to include TensorRT, OpenVINO, with improved fallback logic and diagnostic logging
- Simplified pom.xml GPU profiles and added GitHub Actions workflow for building universal and NVIDIA-specific JAR artifacts
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/atri/palaash/lumenforge/ui/ModelManagerPanel.java | Added onModelsUpdated.run() calls after download, import, and conversion; added logging statements |
| src/main/java/atri/palaash/lumenforge/ui/MainFrame.java | Integrated LogsPanel into UI with new Diagnostics section and keyboard shortcut |
| src/main/java/atri/palaash/lumenforge/ui/LogsPanel.java | New panel with filtered log views (Application/Model) and auto-scroll functionality |
| src/main/java/atri/palaash/lumenforge/ui/AppLogger.java | New centralized logger broadcasting to UI listeners instead of stdout/stderr |
| src/main/java/atri/palaash/lumenforge/storage/ModelDownloader.java | Added logging for download lifecycle events (start, complete, fail, retry) |
| src/main/java/atri/palaash/lumenforge/inference/GenericOnnxService.java | Enhanced EP selection with TensorRT, OpenVINO support; added comprehensive diagnostic logging |
| src/main/java/atri/palaash/lumenforge/app/LumenForgeApp.java | Added startup logging with Java version and system info |
| pom.xml | Simplified GPU profiles to use properties instead of duplicate dependencies |
| .github/workflows/build.yml | New CI/CD workflow building universal and NVIDIA JAR variants with automated releases |
| README.md | Updated documentation with GPU acceleration details, build instructions, and downloads section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private boolean showModel = true; | ||
|
|
||
| /* Coalesce rapid-fire updates (e.g. per-step progress) to ~60 fps. */ | ||
| private boolean dirty = false; |
There was a problem hiding this comment.
The dirty flag is accessed from multiple threads (the listener callback and the Timer's event dispatch thread) without synchronization. This field should be declared as volatile to ensure visibility of updates across threads, preventing potential stale reads.
| private boolean dirty = false; | |
| private volatile boolean dirty = false; |
| Consumer<LogEntry> listener = entry -> { | ||
| synchronized (allEntries) { allEntries.add(entry); } | ||
| scheduleDirty(); | ||
| }; | ||
| AppLogger.addListener(listener); | ||
|
|
||
| /* Coalesce timer fires every 16 ms (~60 fps) if dirty */ | ||
| coalesceTimer = new Timer(16, e -> { | ||
| if (dirty) { | ||
| dirty = false; | ||
| rebuild(); | ||
| } | ||
| }); | ||
| coalesceTimer.setRepeats(true); | ||
| coalesceTimer.start(); |
There was a problem hiding this comment.
The LogsPanel registers a listener with AppLogger and starts a Timer but never cleans up these resources. The listener will remain registered even if the panel is removed from the UI, causing a memory leak. The Timer will continue running indefinitely. Consider adding cleanup logic (stop timer, remove listener) in a method that can be called when the panel is no longer needed, or override removeNotify() to perform cleanup when the component is removed from its container.
Closes #1