Skip to content

Fix controls for shadow-only casters - #3087

Open
adrian-andrzjewski wants to merge 1 commit into
jMonkeyEngine:masterfrom
adrian-andrzjewski:codex/issue-1875-animation-shadow-update
Open

adrian-andrzjewski wants to merge 1 commit into
jMonkeyEngine:masterfrom
adrian-andrzjewski:codex/issue-1875-animation-shadow-update

Conversation

@adrian-andrzjewski

Copy link
Copy Markdown

Fixes #1875

What changed

  • Run render controls for shadow casters and their ancestor chain before rendering each shadow map.
  • Temporarily clear and restore the forced shadow material and technique so controls can prepare casters with their own materials while the shadow camera is active.
  • Add regression coverage for shared ancestors and one control callback per shadow pass.

Validation

  • :jme3-core:test: 1,216 tests, 0 failures, 0 errors, 1 skipped.
  • Runtime reproduction using the migrated Jaime model and SpotLightShadowRenderer: the caster was outside the main camera frustum, was queued for the shadow map, and its render control ran 89 times over 90 frames (RTX 2060 SUPER).

@jaime-jmebot jaime-jmebot 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.

Hi Adrian, thanks for putting this fix together. The change addresses the shadow-only caster path directly: it runs render controls for queued casters and their ancestors with the shadow camera active, while deduplicating shared ancestors within a pass, and restores the render manager’s forced material and technique even if a control throws.

I reviewed the helper and its call site alongside the regression tests. The identity-based set is a good fit for scene-graph spatials, and creating it per invocation ensures controls can run again on the next shadow pass. The tests cover both shared-parent deduplication and repeated callbacks across separate passes, and the material/technique restoration is protected by finally, which is important for leaving subsequent rendering state intact.

Looks good to me. Thanks for including both regression coverage and a runtime reproduction!

@riccardobl

Copy link
Copy Markdown
Member

Is this something you need in your use case?
I suspect this can be a breaking change for some applications.

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.

Animation state gets only updated when inside main camera

3 participants