Skip to content

Lyrical compatibility#163

Open
mxgrey wants to merge 2 commits into
mainfrom
lyrical_compatibility
Open

Lyrical compatibility#163
mxgrey wants to merge 2 commits into
mainfrom
lyrical_compatibility

Conversation

@mxgrey
Copy link
Copy Markdown
Collaborator

@mxgrey mxgrey commented May 20, 2026

This removes a line that was causing an undefined behavior segfault in gz_sim.

The stack trace of the crash can be seen here. It seems that removing the component from the entity inside the Each callback was somehow invalidating the iterators that Each was relying on.

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
// TODO(luca) this could be a component instead of a hash map
_initial_aabbs[entity] = aabb->Data();
_initial_poses[entity] = pose->Data();
enableComponent<components::AxisAlignedBox>(ecm, entity, false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This was added because of the comment above (RTF dropping as soon as lift starts moving).
Does this still happen? If it does we should still remove the components but not inside the Each call (i.e. we collect them into a vector then iterate and remove outside the loop).
If it doesn't happen anymore we should just remove the optimization comment above since it's not relevant anymore

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for calling this out, I wasn't aware of the reason the component was being removed.

I haven't looked at whether there's a frame drop, but if it's a possible concern then I don't mind collecting the entities and removing the component after the Each.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Restored: d2860aa

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

2 participants