Reduce LLFolderView per-item memory + fix three folderview defects#297
Conversation
LLUICtrl never disconnected its control-variable signal connections on destruction, leaking a dead slot onto the session-lived LLControlVariable signal for every control bound via control_name/enabled/visibility that was later destroyed. Fold the five control-variable pointers and their connections into a lazily-allocated ControlVariables struct whose connections are scoped_connection, so destruction auto-disconnects; the struct is allocated only when a control actually binds a variable (~120 bytes saved per unbound control). Route llmenugl/llcombobox through getControlVariable()/getEnabledControlVariable() accessors. Also make LLView::mToolTipMsg a lazily-allocated LLUIString*, null until a tooltip is set, so tooltip-less widgets no longer carry ~100 bytes of empty string storage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
View items (cost paid per item, per open inventory window): - Intern per-item-constant layout + colors into a shared LLFolderViewItemStyle const*, deduplicated across items (~100 B/item); also drop two dead fields (mLeftPad, mTextPadRight). - Lazily build the label/suffix LLFontVertexBuffers and release them when an item goes off-screen (setVisible override), so only on-screen items hold text geometry instead of every built item. Defects: - draw(): the icon-overlay branch dereferenced the nullable mIcon for its height; fall back to the overlay's own height. - onIdleUpdateFavorites(): the 'nothing changed' path left mFavoritesDirtyFlags set, leaking a per-idle callback that rescanned children every frame; clear it and deregister. - arrange(): drop two redundant sFonts map lookups (use getLabelFont()/sSuffixFont). Inventory bridge: - Remove dead shadowed base LLInvFVBridge::mTimeSinceRequestStart (~16 B per bridge, per window); only LLFolderBridge's own copy is used. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughRefactors LLUI to consolidate control variables into a lazy container, lazily allocate LLView tooltips, intern shared LLFolderViewItemStyle instances, and update callers to use the new getters and style fields. ChangesLLUI Infrastructure and Memory Optimization
🎯 4 (Complex) | ⏱️ ~75 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)indra/llui/llfolderviewitem.cppindra/llui/llfolderviewitem.cpp:26:10: fatal error: 'linden_common.h' file not found ... [truncated 1090 characters] ... all/lib/clang/18/include" 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
indra/llui/llfolderviewitem.cpp (1)
397-420:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMirror the new buffer invalidation in
refreshSuffix().
refresh()now clears the lazy font buffers, butarrange()can refresh visible rows throughrefreshSuffix()instead. If a row loses its suffix on that path,draw()stops entering the suffix branch and the oldmSuffixFontBufferstays allocated until the row is hidden. The label buffer should be invalidated there too becausemLabelStyleis updated in the same block.Suggested fix
void LLFolderViewItem::refreshSuffix() { LLFolderViewModelItem const* vmi = getViewModelItem(); @@ if (mRoot->useLabelSuffix()) { // Very Expensive! // Can do a number of expensive checks, like checking active motions, wearables or friend list mLabelStyle = vmi->getLabelStyle(); pLabelFont = nullptr; mLabelSuffix = utf8str_to_wstring(vmi->getLabelSuffix()); + if (mLabelFontBuffer) + { + mLabelFontBuffer->reset(); + } + if (mSuffixFontBuffer) + { + mSuffixFontBuffer->reset(); + } }🤖 Prompt for 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. In `@indra/llui/llfolderviewitem.cpp` around lines 397 - 420, The issue: refreshSuffix() updates mLabelStyle/mLabelSuffix like refresh() but doesn't invalidate the lazy font buffers, leaving mLabelFontBuffer and mSuffixFontBuffer allocated when the suffix is removed; update refreshSuffix() to mirror refresh() by resetting/invalidation of both mLabelFontBuffer and mSuffixFontBuffer when the label suffix/state changes so draw() will recreate buffers lazily; locate refreshSuffix(), and inside the branch that updates mLabelStyle/mLabelSuffix ensure you call mLabelFontBuffer->reset() (if non-null) and mSuffixFontBuffer->reset() (if non-null), and avoid allocating new buffers there (keep lazy creation in draw()).
🧹 Nitpick comments (1)
indra/llui/llview.cpp (1)
227-237: ⚡ Quick winFree tooltip storage when tooltip is cleared.
Line 233 currently keeps
mToolTipMsgallocated whenmsgis empty, which weakens the lazy-allocation memory savings for views that clear tooltips at runtime.Proposed patch
void LLView::setToolTip(const LLStringExplicit& msg) { - if (mToolTipMsg) - { - *mToolTipMsg = msg; - } - else if (!msg.empty()) - { - mToolTipMsg = new LLUIString(msg); - } + if (msg.empty()) + { + delete mToolTipMsg; + mToolTipMsg = nullptr; + return; + } + + if (mToolTipMsg) + { + *mToolTipMsg = msg; + } + else + { + mToolTipMsg = new LLUIString(msg); + } }🤖 Prompt for 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. In `@indra/llui/llview.cpp` around lines 227 - 237, LLView::setToolTip currently leaves mToolTipMsg allocated when called with an empty msg; update the method so that if msg.empty() it frees the existing tooltip storage (delete mToolTipMsg and set mToolTipMsg = nullptr), otherwise preserve the existing behavior (assign to *mToolTipMsg if present, or new LLUIString(msg) if null). Refer to LLView::setToolTip and the mToolTipMsg/LLUIString symbols when making the change.
🤖 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 `@indra/llui/llfolderviewitem.cpp`:
- Around line 1038-1050: The current setVisible() only releases mLabelFontBuffer
and mSuffixFontBuffer when the widget visibility bit flips, so items clipped by
the scroll viewport keep their font buffers; change the logic to evict buffers
whenever the item is not actually visible in the viewport (not just when
LLView::setVisible is called). Add a check (either in
LLFolderViewItem::setVisible or in an arrange/early-draw path such as
LLFolderViewFolder::arrange or LLFolderViewItem::drawLabel/draw) that tests
whether the item's rect intersects the folder/viewport visible rect (use the
view/parent clipping/visible-rect helper or compute intersection) and call
mLabelFontBuffer.reset() and mSuffixFontBuffer.reset() when the item is
clipped/out-of-viewport so buffers are released for off-screen rows and rebuilt
lazily in drawLabel()/draw().
---
Outside diff comments:
In `@indra/llui/llfolderviewitem.cpp`:
- Around line 397-420: The issue: refreshSuffix() updates
mLabelStyle/mLabelSuffix like refresh() but doesn't invalidate the lazy font
buffers, leaving mLabelFontBuffer and mSuffixFontBuffer allocated when the
suffix is removed; update refreshSuffix() to mirror refresh() by
resetting/invalidation of both mLabelFontBuffer and mSuffixFontBuffer when the
label suffix/state changes so draw() will recreate buffers lazily; locate
refreshSuffix(), and inside the branch that updates mLabelStyle/mLabelSuffix
ensure you call mLabelFontBuffer->reset() (if non-null) and
mSuffixFontBuffer->reset() (if non-null), and avoid allocating new buffers there
(keep lazy creation in draw()).
---
Nitpick comments:
In `@indra/llui/llview.cpp`:
- Around line 227-237: LLView::setToolTip currently leaves mToolTipMsg allocated
when called with an empty msg; update the method so that if msg.empty() it frees
the existing tooltip storage (delete mToolTipMsg and set mToolTipMsg = nullptr),
otherwise preserve the existing behavior (assign to *mToolTipMsg if present, or
new LLUIString(msg) if null). Refer to LLView::setToolTip and the
mToolTipMsg/LLUIString symbols when making the change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 44ed3c4f-fa40-45a8-a5c0-6548ee4b1f25
📒 Files selected for processing (11)
indra/llui/llcombobox.cppindra/llui/llfolderview.cppindra/llui/llfolderviewitem.cppindra/llui/llfolderviewitem.hindra/llui/llmenugl.cppindra/llui/lluictrl.cppindra/llui/lluictrl.hindra/llui/llview.cppindra/llui/llview.hindra/newview/llconversationview.cppindra/newview/llinventorybridge.h
💤 Files with no reviewable changes (1)
- indra/newview/llinventorybridge.h
LLFontVertexBuffer::render does not compare the rendered string, so any path that changes mLabelSuffix must reset the cached geometry or draw() replays the old text. refresh() already does this; mirror it in refreshSuffix(), which is reached via arrange() for items deferred from postBuild(). Currently the buffer is always null on that path (items arrange before first draw), so this is hardening against reordering or future callers rather than a user-visible defect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Reduces per-item memory in
LLFolderView(cost paid per item, per open inventory window) and fixes several latent defects found along the way.Memory reductions
View items:
LLFolderViewItemStyleconst*, deduplicated across items (~100 B/item). Drop two dead fields (mLeftPad,mTextPadRight).LLFontVertexBuffersand release them when an item goes off-screen (setVisibleoverride), so only on-screen items hold text geometry instead of every built item.Inventory bridge:
LLInvFVBridge::mTimeSinceRequestStart(~16 B per bridge, per window); onlyLLFolderBridge's own copy is used.Defects
draw(): the icon-overlay branch dereferenced the nullablemIconfor its height; fall back to the overlay's own height.onIdleUpdateFavorites(): the "nothing changed" path leftmFavoritesDirtyFlagsset, leaking a per-idle callback that rescanned children every frame; clear it and deregister.arrange(): drop two redundantsFontsmap lookups (usegetLabelFont()/sSuffixFont).🤖 Generated with Claude Code