Skip to content

Add toggleable per-frame bounding-volume updates for animated models (SkinningControl)#2665

Open
Copilot wants to merge 6 commits intomasterfrom
copilot/fix-animated-models-bound
Open

Add toggleable per-frame bounding-volume updates for animated models (SkinningControl)#2665
Copilot wants to merge 6 commits intomasterfrom
copilot/fix-animated-models-bound

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 28, 2026

Animated models retained their bind-pose bounding volumes throughout animation, causing incorrect frustum culling when poses extended beyond bind-pose extents.

SkinningControl changes

  • setUpdateBounds(boolean) — opt-in flag (default false); no cost unless enabled.
  • Software skinning: calls geometry.updateModelBound() after each CPU skin pass.
  • Hardware skinning: position-only CPU skinning against BindPosePosition updates the BoundingBox in-place each frame without touching GPU buffers.
  • setBoundingUpdateBudget(int) — caps vertices processed per frame; work resumes from a per-geometry cursor the next frame, committing the bbox only once a full pass completes. Default is Integer.MAX_VALUE (all at once, preserving previous behaviour). Values ≤ 0 are normalised to MAX_VALUE.
SkinningControl sc = model.getControl(SkinningControl.class);
sc.setUpdateBounds(true);
sc.setBoundingUpdateBudget(500); // process at most 500 verts/frame

Both flags are serialised in write()/read() for J3O persistence. The deprecated SkeletonControl is unchanged.

Test & example

  • testIssue343() added to PreventCoreIssueRegressions: software-skinned Elephant, legUp animation, asserts world bound shifts from bind pose.
  • jme3test/animation/TestAnimatedModelBound.java in jme3-examples: Elephant with yellow wireframe bbox overlay; press B to toggle updateBounds at runtime.

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI changed the title [WIP] Fix animated models to have proper model bound Fix animated model bounding volumes not updating during animation Mar 28, 2026
Copilot AI requested a review from riccardobl March 28, 2026 10:11
@riccardobl
Copy link
Copy Markdown
Member

@copilot make this toggleable, off by default, add a visual example in jme3-examples

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 28, 2026

@copilot make this toggleable, off by default, add a visual example in jme3-examples

Done in commit f4661dc.

  • Added setUpdateBounds(boolean) / isUpdateBounds() to both SkinningControl and the deprecated SkeletonControl. The flag defaults to false, so existing behaviour is unchanged.
  • The flag is also serialized in write()/read() so it persists in J3O files.
  • Added jme3test/animation/TestAnimatedModelBound.java as a visual example: the Elephant model plays its "legUp" animation while a yellow wireframe box shows the world bound. Press B to toggle updateBounds on/off and see the box either track the animated pose or stay stuck at the bind pose.

@riccardobl riccardobl marked this pull request as ready for review March 28, 2026 10:23
Copilot AI changed the title Fix animated model bounding volumes not updating during animation Add toggleable per-frame bounding-volume updates for animated models Mar 28, 2026
@riccardobl
Copy link
Copy Markdown
Member

@copilot undo the changes to the deprecated SkeletonControl, lets implement and test this only for the new SkinningControl

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 28, 2026

@copilot undo the changes to the deprecated SkeletonControl, lets implement and test this only for the new SkinningControl

Done in commit 166a5d7. All changes to SkeletonControl have been reverted to its original state. The setUpdateBounds/isUpdateBounds feature now exists only in SkinningControl.

Copilot AI changed the title Add toggleable per-frame bounding-volume updates for animated models Add toggleable per-frame bounding-volume updates for animated models (SkinningControl) Mar 28, 2026
}
// setModelBound() updates the mesh bound and triggers a world-bound refresh.
geometry.setModelBound(bbox);
}
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 partially defeats the point of having hardware skinning, can you distribute the bbox update in multiple frames by allowing the developer to set a maximum boundingUpdateBudget to decide how many vertices are considered per frame when updating the bounds?

Agent-Logs-Url: https://github.com/jMonkeyEngine/jmonkeyengine/sessions/0b0ee217-37c6-4d49-9616-3c58c9350a06

Co-authored-by: riccardobl <4943530+riccardobl@users.noreply.github.com>
…n SkinningControl

Agent-Logs-Url: https://github.com/jMonkeyEngine/jmonkeyengine/sessions/3d4465a6-f250-44fe-b834-04690fbb77f2

Co-authored-by: riccardobl <4943530+riccardobl@users.noreply.github.com>
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.

Animated models should have proper model bound

2 participants