[SWM-220][perf] GUI: only load the tabs necessary based on the microscope role#3366
Open
pieleric wants to merge 1 commit intodelmic:masterfrom
Conversation
pieleric
changed the title
📝 WalkthroughWalkthroughThe PR changes the GUI tab system to load tabs conditionally based on the detected microscope role rather than loading all tabs and hiding unused ones. The package public exports were reduced to only TabBarController and Tab. Tab button visibility is handled during tab creation (shown immediately) instead of via a post-creation cleanup. Many XRC GUI elements (tab buttons and other controls) are marked hidden by default in resource files. Sequence DiagramsequenceDiagram
participant Main as GUI Main Initialization
participant RoleDetector as Microscope Role Detection
participant TabDef as Tab Definition Builder
participant TabCtrl as TabBarController
participant Tab as Tab Instances
participant UI as UI Components
Main->>RoleDetector: Query microscope role
RoleDetector-->>Main: Return role (e.g., secom, sparc, meteor, mbsem, None)
Main->>TabDef: Build role-specific tab_defs
TabDef-->>Main: Return tab_defs for role
Main->>TabCtrl: Initialize with tab_defs
TabCtrl->>Tab: Instantiate each tab in tab_defs
Tab-->>TabCtrl: Tab instance created
TabCtrl->>UI: Show corresponding tab button (explicit Show())
UI-->>TabCtrl: Button becomes visible
TabCtrl-->>Main: TabBar initialization complete
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/odemis/gui/main.py`:
- Around line 253-297: The code creates a CryoMainGUIData for roles ("meteor",
"enzel", "mimas") but only registers meteor tabs for main_data.role == "meteor",
leaving enzel/mimas without their expected UI and causing warnings/errors; fix
by making the role checks consistent: either remove "enzel" and "mimas" from the
CryoMainGUIData creation (in the code that instantiates CryoMainGUIData) or
expand the tab-registration branch (the elif that imports
CorrelationTab/CryoChamberTab/LocalizationTab/FibsemTab) to include "enzel" and
"mimas" (e.g., main_data.role in ("meteor","enzel","mimas")) and add a
deprecation log when role is "enzel" or "mimas" so they still get the cryo tabs
but are flagged as deprecated.
🧹 Nitpick comments (1)
src/odemis/gui/main.py (1)
284-296: Commented-out code for deprecatedenzel/mimasalign tabs.If these roles are fully deprecated and no longer supported, consider removing the commented-out blocks entirely rather than leaving dead code. The git history preserves the old definitions if they're ever needed again.
There was a problem hiding this comment.
Pull request overview
This pull request optimizes GUI initialization by conditionally loading tabs based on the microscope role, rather than loading all tabs upfront. This change aims to avoid failures from missing external modules imported by unused tabs and potentially improve loading time.
Changes:
- Modified tab loading to be role-based, importing tab classes only when needed for the current microscope role
- Updated XRC resource files to hide all tab buttons by default, showing them programmatically when tabs are loaded
- Removed bulk imports from tabs/init.py, keeping only essential TabBarController and Tab imports
- Updated tab_bar_controller.py to show buttons for loaded tabs instead of hiding unused ones
- Simplified secom_streams_tab.py get_display_priority method since role filtering now happens earlier
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/odemis/gui/xmlh/resources/frame_main.xrc | Hidden all tab buttons by default to be shown programmatically when loaded |
| src/odemis/gui/main_xrc.py | Generated XRC file synced with frame_main.xrc changes |
| src/odemis/gui/main.py | Implemented conditional tab loading based on microscope role with on-demand imports |
| src/odemis/gui/cont/tabs/tab_bar_controller.py | Changed to show buttons for loaded tabs instead of hiding unused ones |
| src/odemis/gui/cont/tabs/secom_streams_tab.py | Simplified get_display_priority by removing redundant role checks |
| src/odemis/gui/cont/tabs/init.py | Removed tab class imports, keeping only TabBarController and Tab, and updated copyright to 2026 |
tepals
reviewed
Feb 13, 2026
Each tab is shown or not dependening the hardware. However, until now, all the tab were loaded, and then a call to get_display_priority() by the tab controller would decide whether it's worthy showing or not. However, just based on the microscope role, we can have already a pretty good idea of which tab is certainly not useful. => load only the tabs that could be used for the current microscope role. This mainly avoids failures due to a missing an external module imported by a non-used tab. In theory, it should also improve the loading time of the GUI. However, tests on the viewer (were the saving should be most noticeable) show it barely improved: going from ~3.34 to ~3.28 (-> 0.06s faster)
pieleric
force-pushed
the
perf-gui-only-load-the-tabs-necessary-based-on-the-microscope-role
branch
from
3a142f1 to
dbae6e8
Compare
nandishjpatel
approved these changes
Feb 16, 2026
tepals
approved these changes
Feb 16, 2026
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
3 participants
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.
Each tab is shown or not depending the hardware. However, until now, all the tab were loaded, and then a call to get_display_priority() by the tab controller would decide whether it's worthy showing or not.
However, just based on the microscope role, we can have already a pretty good idea of which tab is certainly not useful.
=> load only the tabs that could be used for the current microscope role.
This mainly avoids failures due to a missing an external module imported by a non-used tab.
In theory, it should also improve the loading time of the GUI. However, tests on the viewer (were the saving should be most noticeable) show it barely improved: going from ~3.34 to ~3.28 (-> 0.06s faster)