fix: defer UV rebuild on import to prevent UI freeze - #763
Conversation
UVEditorController was rebuilding the full HalfEdge/island cache on every entityCreated + selectOne during import, freezing the UI on large meshes. Skip rebuilds while the UV Editor is closed, debounce when open, and add vertex→triangle indices for edit-mode context islands. Also lazy-load hidden dock QML (UV Editor, Asset Browser, AI Chat, Dope Sheet, Curve Editor), defer Asset Browser directory scan and LLMManager init, and stop re-applying custom palette after QML widgets exist. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
More reviews will be available in 37 minutes and 21 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds deferred refresh handling and cached UV selection lookups in ChangesUV editor refresh and context lookup
Main window lazy dock startup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mainwindow.cpp`:
- Around line 1152-1157: The AI Chat open paths in MainWindow are missing a
ui.action breadcrumb for user-facing navigation. Update both entrypoints that
open the chat (the aiChatButton clicked lambda and the other AI Chat action path
referenced in MainWindow) to call SentryReporter::addBreadcrumb with category
ui.action and a message indicating the chat was opened before showing/raising
m_chatDock, so both routes are consistently tracked.
- Around line 882-892: The `AssetBrowserController::importMeshRequested` signal
hookup in `MainWindow` was moved into the `m_assetBrowserDock` visibilityChanged
lambda, which makes the import handler register too late and can drop
`openFile()` requests before the dock is shown. Move the connect/disconnect
logic for `importMeshRequested` back into the startup/initialization path of
`MainWindow` so the signal is always wired eagerly, and keep only
`ensureLazyDockQml(m_assetBrowserDock)` inside the visibilityChanged handler.
- Around line 255-271: The theme menu check state is reading only the “palette”
setting, which can diverge from the actual theme chosen by
ThemeManager::applySavedThemeFromSettings(). Update the logic in the mainwindow
sync block to follow the same precedence as ThemeManager, using
“appearanceTheme” first and falling back to “palette” only when appropriate,
while still keeping the QAction state updates in sync for ui->actionLight,
ui->actionDark, and ui->actionCustom.
In `@src/UVEditorController_test.cpp`:
- Around line 198-208: The test in UVEditorController should not assume the
singleton starts with an empty mesh cache, since UVEditorController::instance()
may already retain mesh state from earlier tests. Update the setup so the test
explicitly establishes a known state before asserting hasMesh(), and remove the
extra ctrl->refresh() after ctrl->setPanelActive(true) so the test verifies that
setPanelActive(true) itself flushes any deferred refresh. Use the existing
UVEditorController methods and meshDataChanged spy to keep the assertions
focused on the panel activation behavior.
In `@src/UVEditorController.cpp`:
- Around line 68-86: The refresh queue in UVEditorController is not cleared when
the panel becomes inactive, so a previously started m_refreshTimer can still
fire and trigger refresh/rebuildMeshCache while hidden. Update
setPanelActive(bool) to stop any pending timer when active becomes false, and
keep scheduleRefresh() using m_refreshPending only for deferred refreshes; use
the existing m_refreshTimer, m_refreshPending, and refresh() flow to locate the
fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7566518-969e-4782-8c82-28fff33b30de
📒 Files selected for processing (5)
src/AssetBrowserController.cppsrc/UVEditorController.cppsrc/UVEditorController.hsrc/UVEditorController_test.cppsrc/mainwindow.cpp
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0443d1d0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| UVEditorController::instance()->setPanelActive(vis); | ||
| if (vis) { | ||
| ensureLazyDockQml(m_uvEditorDock); | ||
| UVEditorController::instance()->refresh(); |
There was a problem hiding this comment.
When the UV editor was previously opened and then hidden during an import/selection change, scheduleRefresh() leaves m_refreshPending set; the call to setPanelActive(true) just above this line consumes that pending state by calling refresh(), and then this unconditional refresh() immediately rebuilds the same mesh cache a second time. For large meshes this doubles the UI-thread work exactly when the user opens the UV Editor after an import, so skip this call when activation has already flushed a pending refresh or move the refresh responsibility to one place.
Useful? React with 👍 / 👎.
…cancel Match ThemeManager settings precedence for palette menu checkboxes, keep importMeshRequested wired at startup (lazy QML only), stop debounce timer when UV Editor hides, add AI Chat breadcrumbs, and tighten hidden-panel test. Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
Addressed CodeRabbit review in
Latest CI run (28274082203) is green. |
1 participant
Summary
Fixes UI freezes when importing meshes (especially large/skinned FBX assets).
UVEditorControllerwas rebuilding the full HalfEdge/island cache +QVariantListtriangle payload on everyentityCreated/selectOneduring import (each sub-entity auto-selects). Now skips rebuilds while the UV Editor dock is hidden, debounces (75ms) when open, and indexes vertices→triangles for edit-mode context islands.LLMManagerinit, and stop re-applying custom palette viasetPalette()after QML widgets exist.Test plan
UnitTests --gtest_filter="UVEditorControllerTest.*"— 16/16 passMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes