Skip to content

[MISC] Fix PBD entities to properly add visual vertices to solver for BatchRendererCamera compatibility#2696

Closed
vlordier wants to merge 3 commits intoGenesis-Embodied-AI:mainfrom
vlordier:feature/pbd-batch-renderer-fix
Closed

[MISC] Fix PBD entities to properly add visual vertices to solver for BatchRendererCamera compatibility#2696
vlordier wants to merge 3 commits intoGenesis-Embodied-AI:mainfrom
vlordier:feature/pbd-batch-renderer-fix

Conversation

@vlordier
Copy link
Copy Markdown

Summary

Fixed PBD entities to properly add visual vertices to solver for BatchRendererCamera compatibility.

Previously, PBD entities were not calling _add_vverts_to_solver() in their _add_to_solver() method, which meant that visual vertices were not being added to the solver. This caused the BatchRendererCamera to fail when trying to render PBD entities because it couldn't find the visual vertex data.

This fix ensures that PBD entities properly add their visual vertices to the solver, making them compatible with the BatchRendererCamera.

Testing

  • Verified that PBD entities now work correctly with BatchRendererCamera
  • Ran existing tests to ensure no regression
  • The fix follows the same pattern used by other entity types (like rigid entities)

Copilot AI review requested due to automatic review settings April 11, 2026 09:41
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44d22c5ddd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread genesis/engine/entities/pbd_entity.py Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

- ParticleEntity._add_to_solver() already calls _add_vverts_to_solver()
  when _need_skinning=True, which is the case for all PBD tet entities
- The duplicate call caused redundant precomputation (pairwise distance
  matrix, pseudoinverse, kernel upload) with no functional benefit
- PBDParticleEntity and SPHEntity pass need_skinning=False so they
  were not affected

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@duburcqa
Copy link
Copy Markdown
Collaborator

@claude review

@vlordier
Copy link
Copy Markdown
Author

Analysis

After investigation, this PR is currently a net-zero change.

  • Commit 44d22c5 adds a _add_vverts_to_solver() call inside PBDBaseEntity._add_to_solver().
  • Commit 57eee9d removes those exact same 4 lines.

The reason for the revert (documented in 57eee9d) is correct: ParticleEntity._add_to_solver() (the parent class, particle_entity.py:187–188) already calls _add_vverts_to_solver() when _need_skinning=True, which is the case for all PBD tet entities. The duplicate call would have caused redundant precomputation with no functional benefit.

If the original issue (BatchRendererCamera not rendering PBD entities) is still open, the fix should be investigated at a different layer — the batch_renderer.py GenesisGeomRetriever does not include PBD visual geometry today. A more complete fix is included in PR #2695 which adds retrieve_pbd_meshes_static() to GenesisGeomRetriever.

Closing this PR as the net change is zero and the fix belongs elsewhere.

@vlordier vlordier closed this Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants