Draft
Conversation
Add showEvent to reset the cleanup guard and rebuild the working settings from the latest accepted settings, then repopulate the dialog. Allow removing/moving active cameras even while a scan is running and adjust preview button enablement logic. Auto-select the first active camera when populating the list and ensure multi_camera_settings is updated from the working copy before emitting settings_changed on apply.
Add two end-to-end GUI tests for camera configuration dialog to prevent regressions: - test_remove_active_camera_works_while_scan_running: Verifies that removing the active camera still works while a discovery scan is running. The test slows CameraFactory.detect_cameras via monkeypatch to keep the scan running, ensures the remove button is enabled during scan, removes the selected camera, and cleans up by cancelling the scan. - test_ok_updates_internal_multicamera_settings: Ensures that after adding a second camera and accepting the dialog (OK), the settings_changed signal emits the updated MultiCameraSettings and the dialog's internal _multi_camera_settings is updated to match the accepted settings. These tests guard against regressions where scan-running state blocked structure edits (remove/move) and where dialog acceptance did not update the dialog's internal settings.
Introduce a helper to detect live previews and replace scattered direct PreviewState checks with it. Add validation for selected detected-camera items (show a warning if invalid). Normalize camera backend id casing when formatting labels. Remove a redundant try/except around cleanup guard reset and always reset _cleanup_done on show. Drop an extra in-place settings assignment in the apply flow and remove a now-unneeded call to _populate_from_settings on show. These changes tidy preview control, improve robustness for camera selection, and unify camera identity handling.
Clean up and shorten comments in tests/gui/camera_config/test_cam_dialog_e2e.py: remove parenthetical notes about expected failures tied to implementation details and replace them with concise, direct expectations. No functional test logic changed.
Refine CameraConfigDialog behavior to prevent races and invalid states: only clean up the scan worker when it's not running; disable the Add Camera button unless the selected item is a DetectedCamera; improve probe worker cancellation and waiting to avoid concurrent probes; emit the multi-camera model copy instead of a deepcopy when settings change. Add _enabled_count_with and enforce MAX_CAMERAS when enabling a camera and before closing the dialog. Clamp crop coordinates and only apply crop when the rectangle is valid to avoid invalid-frame cropping. Small comment/organization tweaks included.
Ensure selection state is synchronized after initialization by calling _populate_from_settings and a new _post_init_sync_selection to set _current_edit_index. Install event filters on camera numeric spinboxes and their lineEdits, and improve Enter-key handling so interpretText() is invoked on the correct spinbox (whether the event comes from the spinbox or its lineEdit) before applying camera settings. Introduce _selected_detected_camera helper to centralize retrieval of the selected detected camera and use it to control the Add button enablement; simplify scan-related UI enable/disable logic and remove duplicated state handling. Miscellaneous cleanup and minor refactors in camera_config_dialog.py.
Normalize dlc_camera_id values (split on ':' and lowercase backend part, fallback to lowercasing whole string) and refresh camera labels accordingly. Improve camera discovery shutdown: set scan state to CANCELING when stopping, avoid immediate cleanup if the scan thread is still running, and route worker finished to a new handler (_on_scan_thread_finished) that performs cleanup and sets state to IDLE. Also stop calling _populate_from_settings() when opening the camera dialog to avoid overwriting unsaved changes; let the dialog refresh itself when shown.
Introduce caching for camera discovery to avoid unnecessary rescans by adding _last_scan_backend and _has_scan_results and a _maybe_refresh_available_cameras(force=False) helper that only calls _refresh_available_cameras when backend or cache state requires it. Refactor population logic: add _populate_active_list_from_working to fill the active cameras list (preserving selection optionally) and simplify _populate_from_settings to use it. Add public set_settings(...) to update the dialog with new MultiCameraSettings and optional dlc_camera_id without destroying unsaved UI state. Improve scan worker cleanup logic and add a debug message to clarify deferred cleanup. Update backend-change handling to invalidate cache, and update scan result handlers to set cache flags appropriately. Minor UI tweaks: remove a stray currentRow() call, ensure button states are updated, and switch main window to call set_settings(...) when reusing the dialog so the dialog manages its own refresh behavior.
Replace a bare exception handler when installing event filters with specific (AttributeError, RuntimeError) handling and log a warning so failures are not silently swallowed. Enhance _on_scan_thread_finished to ignore finished signals from stale workers, attempt deleteLater() on old senders, and only transition the active worker to IDLE to avoid race conditions affecting scan state.
Introduce explicit cleanup flags and tighten cleanup logic to avoid races and double-run teardown. Add _cleanp_requested and _cleanup_completed, replace uses of the old _cleanup_done flag, and only mark cleanup completed when no scan/preview/probe worker is active. Also always transition the scan state to IDLE when cleaning up. Update the e2e test to wait for scans to finish earlier and reduce related timeouts to reflect the more deterministic teardown behavior.
Introduce a new skeleton utility and integrate skeleton overlays into the GUI. Adds dlclivegui/utils/skeleton.py (SkeletonModel, Skeleton, loaders, render status/codes) and a helper to load DLC config skeleton. Wire skeleton drawing into display.draw_pose and the main window: add UI checkbox, auto-enable/disable logic, model-based skeleton configuration, and safe handling when keypoint counts or shapes mismatch. Also add a QScrollArea for the controls dock to allow scrolling. Remove unused demo code Add skeleton UI controls and gradient coloring Add UI and rendering support for configurable skeleton appearance: color mode (solid or keypoint-gradient) and line thickness. main_window.py: introduce skeleton color combo, thickness spinbox, handlers (_on_skeleton_style_changed, _sync_skeleton_controls_from_model), and wire these into model loading and drawing flow; enable/disable controls appropriately and preserve auto-disable behavior. gui/misc/color_dropdowns.py: add gradient swatch icon and helpers to create/populate/get/set a skeleton color combo (supports Gradient and solid BGR swatches). utils/display.py: add keypoint_colors_bgr(colormap, num_keypoints) to produce exact BGR colors from a Matplotlib colormap and remove direct skeleton coupling from draw_pose. utils/skeleton.py: ensure draw_many forwards style, color_override and keypoint_colors to per-pose draw calls and validates pose shapes/keypoint counts. Overall this enables gradient coloring of skeleton lines based on keypoint colormap and exposes user controls to tweak skeleton rendering.
C-Achard
added
enhancement
Introduce a first-class SkeletonStyle and SkeletonColorMode (and BGR alias) in config, and wire them through the GUI and skeleton utilities. Updates include: - dlclivegui/config.py: add BGR type, SkeletonColorMode enum, Pydantic SkeletonStyle model, expose skeleton fields on VisualizationSettings and with_overlays on RecordingSettings, and helper color accessors. - dlclivegui/gui/main_window.py: add _apply_viz_settings_to_ui/_apply_viz_settings_to_skeleton, create a unified _draw_skeleton_on_frame renderer, read/write skeleton style from UI, refactor and simplify skeleton enable/disable and model heuristics, and wire recording.with_overlays. - dlclivegui/gui/misc/color_dropdowns.py: reuse BGR from config. - dlclivegui/utils/skeleton.py: remove duplicate style/type definitions, import style and enums from config, and add module docstring. These changes centralize skeleton styling, enable UI control and persistence of style, and clean up duplicate definitions across modules.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds skeleton-overlay support to the GUI, including a new skeleton utility module, GUI controls for skeleton appearance (solid vs keypoint-gradient + thickness), and improved controls-dock layout via a scroll area. This fits into the existing visualization/overlay pipeline by drawing skeleton edges on top of pose predictions in both preview and recording overlays.
Changes:
- Introduces
dlclivegui/utils/skeleton.pyfor skeleton definition validation and drawing (single- and multi-pose). - Extends GUI visualization settings + config models to support skeleton toggling and styling; integrates skeleton rendering into preview and recording overlay paths.
- Improves the controls dock usability by wrapping controls in a
QScrollAreaand syncing minimum width to content.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
dlclivegui/utils/skeleton.py |
New skeleton schema/IO + rendering utilities used by the GUI overlay pipeline. |
dlclivegui/utils/display.py |
Adds keypoint colormap → BGR helper and refactors primary color enum (with alias). |
dlclivegui/gui/misc/color_dropdowns.py |
Adds skeleton color dropdown helpers, including a “Gradient” swatch option. |
dlclivegui/gui/main_window.py |
Adds skeleton UI controls, model-based skeleton loading, and skeleton overlay drawing; wraps controls in scroll area. |
dlclivegui/config.py |
Adds SkeletonStyle/SkeletonColorMode and persists visualization + recording overlay settings. |
dlclivegui/temp/yolo/__init__.py |
Empty module placeholder (added). |
dlclivegui/assets/skeletons/__init__.py |
Empty package placeholder for bundled skeleton assets (added). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+29
to
44
| class SkeletonColors(enum.Enum): | ||
| GRADIENT = "gradient" # special mode | ||
| RED = PrimaryColors.RED.value | ||
| GREEN = PrimaryColors.GREEN.value | ||
| BLUE = PrimaryColors.BLUE.value | ||
| YELLOW = PrimaryColors.YELLOW.value | ||
| CYAN = PrimaryColors.CYAN.value | ||
| MAGENTA = PrimaryColors.MAGENTA.value | ||
| WHITE = PrimaryColors.WHITE.value | ||
| BLACK = PrimaryColors.BLACK.value | ||
|
|
||
| @staticmethod | ||
| def get_all_display_names() -> list[str]: | ||
| return ["Gradient"] + [c.name.capitalize() for c in SkeletonColors if c != SkeletonColors.GRADIENT] | ||
|
|
||
|
|
There was a problem hiding this comment.
SkeletonColors is introduced here but is not referenced anywhere else in the repository (only defined in this file). If it’s not going to be used, it’s dead code that adds maintenance overhead and potential confusion (especially since it mixes a string value for GRADIENT with tuple values for the other members). Consider removing it or wiring it into the UI instead of leaving it unused.
Suggested change
| class SkeletonColors(enum.Enum): | |
| GRADIENT = "gradient" # special mode | |
| RED = PrimaryColors.RED.value | |
| GREEN = PrimaryColors.GREEN.value | |
| BLUE = PrimaryColors.BLUE.value | |
| YELLOW = PrimaryColors.YELLOW.value | |
| CYAN = PrimaryColors.CYAN.value | |
| MAGENTA = PrimaryColors.MAGENTA.value | |
| WHITE = PrimaryColors.WHITE.value | |
| BLACK = PrimaryColors.BLACK.value | |
| @staticmethod | |
| def get_all_display_names() -> list[str]: | |
| return ["Gradient"] + [c.name.capitalize() for c in SkeletonColors if c != SkeletonColors.GRADIENT] |
Contributor
Author
There was a problem hiding this comment.
Should be used in color_dropdowns.py
Comment on lines
+229
to
+233
| def check_pose_compat(self, pose: np.ndarray) -> SkeletonRenderStatus: | ||
| pose = np.asarray(pose) | ||
|
|
||
| if pose.ndim != 2 or pose.shape[1] not in (2, 3): | ||
| return SkeletonRenderStatus( |
There was a problem hiding this comment.
This PR introduces a new skeleton rendering module with non-trivial behaviors (schema validation, pose compatibility checks, gradient rendering, multi-pose handling), but there are no unit tests covering it yet. Since the repo already has overlay tests in tests/utils/test_display.py, consider adding focused tests for Skeleton.check_pose_compat() / draw() / draw_many() (including gradient mode and mismatch cases) to prevent regressions.
Call _sync_skeleton_controls_from_model() to centralize enabling/disabling of skeleton UI controls (ensures controls are disabled when self._skeleton is None) instead of manually toggling the checkbox. Change save_skeleton to use model.model_dump_json() for .json output so Pydantic's JSON serialization is used (ensures Enums and other non-primitive types are serialized correctly); keep YAML branch using model_dump and safe_dump. Raises same error for unsupported suffixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces a new skeleton utility and integrate skeleton overlays into the GUI.
TODO
This pull request adds comprehensive support for skeleton visualization in the main GUI window, allowing users to display and customize skeleton overlays based on model definitions. It introduces new UI controls for skeleton display and style, ensures compatibility with loaded models, and manages automatic enabling/disabling of skeleton features.
Additionally, the controls dock is improved for better layout and usability.
Skeleton visualization features:
_build_viz_group). These controls are enabled only when a compatible skeleton is available for the loaded model.config.yaml, synchronize UI controls with skeleton style, and automatically enable or disable skeleton controls based on model compatibility (_configure_skeleton_for_model,_sync_skeleton_controls_from_model)._render_overlays_for_recording,_try_draw_skeleton,_update_video_display). [1] [2] [3]_connect_signals,_on_show_skeleton_changed,_on_skeleton_style_changed). [1] [2]Controls dock usability improvements:
QScrollAreaand implemented logic to synchronize the dock’s minimum width with the controls content, ensuring proper layout and usability even with many controls (_setup_ui,_sync_controls_dock_min_width). [1] [2] [3]