FIX - Keep moving linkset children aligned with focused root - #6234
FIX - Keep moving linkset children aligned with focused root#6234trish-sl wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Addresses a long-standing camera-focus rendering/update issue where focusing (alt-click) on a moving linkset root can visually desynchronize the root from its children and seated avatars, by forcing consistent movement updates for the whole focused linkset/occupants.
Changes:
- Adds
LLVOAvatar::idleUpdateVoiceVisualizerPosition()and uses it to centralize voice visualizer position updates. - Extends
LLAgentCamera::calcFocusPositionTargetGlobal()to also update focused root linkset children (and seated avatars/attachments) when tracking a focused moving object. - Minor whitespace cleanup at EOF in touched headers/sources.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| indra/newview/llvoavatar.h | Declares a new helper to update voice visualizer position independently. |
| indra/newview/llvoavatar.cpp | Implements the helper and routes existing position update through it. |
| indra/newview/llagentcamera.cpp | Updates focused-root tracking to also update child drawables and seated avatar visuals/attachments to prevent apparent dislocation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
indra/newview/llagentcamera.cpp:1665
- This early-move alignment only iterates the focus root’s direct children (mFocusObject->getChildren()). Seated avatars are typically parented to the sit target prim (often a child prim), which makes them grandchildren of the root, so they won’t be updated here and can still visually lag/dislocate when the root is focused.
|
I already had a local fix for this issue so I figured I'd give some input on the differing approach. The existing movement update in For my fork's approach, removing the late camera-owned movement update kept the linkset stable without needing the separate cascade updating states. I also found that children may retain drawable generation |
The main reason I went this way with my implementation is to try to avoid changing the logic in hopes of minimizing the chance of breaking changes to existing content which had apparently happened in the past with other attempts. Will give your version of the fix a try to compare, if there's no perceptible side effect with it, it would be simpler to take #6265 instead. |
|
Tested #6265 isolated from any other fixes and I couldn't find any side effect so far. I also refactored my implementation to be smarter in regards to the EARLY_MOVE state. Both fixes look good to me: on vehicles, seated avatars and their rigged/unrigged attachments, nametags and voice icons are properly smoothed, moving children of the linksets as well. There is no more stuttering/hiccups/dislocation of objects to be observed. I could not find any regression with other objects that have movement (doors, physical prims like socces balls, and other animated or physical objects) QA might want to test both thoroughly and pick whichever they're most comfortable with. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
indra/newview/llagentcamera.cpp:110
- This comment overstates what the
EARLY_MOVEguard guarantees. The main loop processes and clears the move list beforeupdateCamera()(llappviewer.cpp:5590-5610;pipeline.cpp:2112), so the first focus calculation can replay movement already processed by the normal pipeline. The flag only prevents another update until the move list is processed again; documenting that distinction avoids misleading future changes to this timing-sensitive path.
// Keep the focused linkset's render state in step with the focused root without
// replaying movement that the normal pipeline has already processed this frame.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
94b9b83 to
3233b16
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Not moving root objects outside out of pipeline seems like a better approach, but any idea why was that movement added in first place? |
|
It appears to be very old code that was seemingly already present in the 2007 import. I'm guessing the idea was to update the object's drawable before reading
|
Related to this canny
Alt-clicking on a root prim of a moving object results in a long standing bug where the root will move faster than its child prims. This causes visual dislocation of any other moving parts of the object such as an avatar sitting on a vehicle or the wheels turning. This also extends to the nametag and voice visual indicator of an avatar.
This bug is 100% reproducible by taking any vehicle and focusing on the root while its in motion. The old tickets (MAINT-1742 MAINT-2247, MAINT-2275) are referenced in the code and mention that some stuff was reverted in the past due to doors breaking. The nature of the changes were different, and I could not spot any undesirable side effect of my version of this fix (including with doors). If anyone has history on these old tickets or suggestions to rework the nature of the fix, I'd love to hear it.
With this fix in place, focusing on the root of a moving object keeps all of its parts and seated avatars together as you would expect.