Skip to content

Fix: Make SetPose UI not editable for planes#3811

Open
silverducks143 wants to merge 1 commit into
gazebosim:gz-sim10from
silverducks143:gz-sim10
Open

Fix: Make SetPose UI not editable for planes#3811
silverducks143 wants to merge 1 commit into
gazebosim:gz-sim10from
silverducks143:gz-sim10

Conversation

@silverducks143

Copy link
Copy Markdown

🦟 Bug fix

Fixes #3483

Summary

Fix makes Pose UI fields read-only when selection has plane geometry.
To achieve this, Component Inspector was extended with an extra property - modelContainsPlane - and that property was added to readOnly block in Pose3d.qml. Screenshot with PR applied:
image

Steps to reproduce the bug:

  1. Open empty built-in world
  2. Choose ground plane
  3. Try to adjust it's position

Expected behavior: Pose UI for planes is either not editable or applies edits
Actual behavior: Pose UI for planes is editable but silently fails to apply edits

Backport Policy

  • This is safe to backport to the following versions:
    • Jetty
    • Ionic
    • Harmonic
    • Fortress
  • This should not be backported
  • I am not sure
  • Other (fill in yourself)

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Assisted-by" to your commits. (See this policy for more info.)

@silverducks143
silverducks143 requested a review from arjo129 as a code owner July 9, 2026 14:06
@github-actions github-actions Bot added the 🪵 jetty Gazebo Jetty label Jul 9, 2026

// Check if any collision in this model has plane geometry.
bool containsPlane = false;
_ecm.EachNoCache<components::Collision,

@luca-della-vedova luca-della-vedova Jul 10, 2026

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.

⚠️ that this API performs horribly, in fact removing it from our SimulationRunner alone doubled our RTF #3650.
Did you check how this change performs in a world with many entities (i.e. 3k_shapes)? Does it need to be the NoCache variant of Each?
If it affects performance significantly I think we should not backport it, if #3447 gets in EachNoCache will have no significant performance hit so merging it forward should be OK.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I wasn't really paying attention to performance. You're right - even on a mid-size scene impact is noticeable, let alone on 3k_shapes.
Yeah, NoCache variant is indeed unnecessary here.
It's also iterating over everything, which is also unnecessary.

Thanks for the catch.
I've modified the check to fix both problems.

Signed-off-by: Pavel Petrykin <silverducks@altlinux.org>
@arjo129 arjo129 added the needs-behavorial-test This represents PRs which need behavioral validation on reviewers machines. label Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🪵 jetty Gazebo Jetty needs-behavorial-test This represents PRs which need behavioral validation on reviewers machines.

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

4 participants