Skip to content

FIX - Keep moving linkset children aligned with focused root - #6234

Open
trish-sl wants to merge 7 commits into
secondlife:developfrom
trish-sl:fix-vehicledisjointed
Open

FIX - Keep moving linkset children aligned with focused root#6234
trish-sl wants to merge 7 commits into
secondlife:developfrom
trish-sl:fix-vehicledisjointed

Conversation

@trish-sl

@trish-sl trish-sl commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions Bot added the c/cpp label Sep 2, 2026
@akleshchev
akleshchev self-requested a review September 2, 2026 15:56
Comment thread indra/newview/llvoavatar.cpp

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread indra/newview/llagentcamera.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Viscerous

Copy link
Copy Markdown
Contributor

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 calcFocusPositionTargetGlobal() occurs after the normal movement pass, so updating the focused drawable there can desynchronise render state and leave EARLY_MOVE state affecting subsequent frames. Cascading that update to the rest of the linkset improves focus, but the camera path becomes responsible for updating linksets, seated avatars, attachments, name tags, and voice indicators.

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 -1, getRenderPosition() then returns their logical object position rather than the current composed drawable position, which explains the remaining child-focus discrepancy. I have a smaller replacement patch and runtime test results available at #6265 if you’d be open to comparing implementations before this merges.

@trish-sl

trish-sl commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

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 calcFocusPositionTargetGlobal() occurs after the normal movement pass, so updating the focused drawable there can desynchronise render state and leave EARLY_MOVE state affecting subsequent frames. Cascading that update to the rest of the linkset improves focus, but the camera path becomes responsible for updating linksets, seated avatars, attachments, name tags, and voice indicators.

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 -1, getRenderPosition() then returns their logical object position rather than the current composed drawable position, which explains the remaining child-focus discrepancy. I have a smaller replacement patch and runtime test results available at #6265 if you’d be open to comparing implementations before this merges.

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.

@trish-sl

trish-sl commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

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.
For getRenderPosition, I gated a bit more conservatively on the off chance it might cause side effects we are not expecting.

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_MOVE guard guarantees. The main loop processes and clears the move list before updateCamera() (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.

Comment thread indra/newview/llagentcamera.cpp Outdated
Comment thread indra/newview/llagentcamera.cpp
@trish-sl
trish-sl marked this pull request as draft September 9, 2026 18:00
trish-sl and others added 5 commits September 9, 2026 14:00
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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread indra/newview/llagentcamera.cpp
Comment thread indra/newview/llagentcamera.cpp
trish-sl and others added 2 commits September 9, 2026 14:33
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@trish-sl
trish-sl marked this pull request as ready for review September 9, 2026 18:55
@akleshchev

Copy link
Copy Markdown
Contributor

Not moving root objects outside out of pipeline seems like a better approach, but any idea why was that movement added in first place?

@Viscerous

Copy link
Copy Markdown
Contributor

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 getRenderPosition() so the camera follows the latest rendered position. A later update exempted avatars from this so I suspect developers already recognised that those were on their own update path.

gPipeline.updateMove() runs before gAgentCamera.updateCamera() however, so the camera's extra update happens after the linkset's normal movement pass. Since it sets EARLY_MOVE, the next normal pass sees and clears that and skips the drawable's move, introducing the out of phase movement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Child prims and avatars of a moving linkset can appear dislocated from the root prim if the camera is focused on the root.

4 participants