Skip to content

Conversation

@NickKhalow
Copy link
Contributor

@NickKhalow NickKhalow commented Dec 17, 2025

Pull Request Description

What does this PR change?

This PR introduces an explicit invariant around the global avatar job index to prevent invalid runtime state from propagating into native rendering code.

A raw int index is replaced with a strongly-typed struct (GlobalJobArrayIndex) that encodes validity rules in the type system and enforces them at architectural boundaries. As a result, invalid indices are no longer representable or passable into GPU paths.

This is an intentional invariant enforcement, not an ad-hoc guard or defensive workaround.


Problem

The current avatar skinning flow allows invalid state (e.g. negative or uninitialized indexInGlobalResultArray) to propagate deep into the rendering pipeline without semantic constraints.

As a result:

  • Invalid indices reach ComputeBuffer.SetData, causing:

    ArgumentOutOfRangeException:
    Bad indices/count arguments
    (nativeBufferStartIndex:-62 computeBufferStartIndex:0 count:62)
    
  • The same corrupted state may surface as:

    NullReferenceException
    at UnityEngine.ComputeBuffer.InternalSetNativeData
    
  • Failures are non-deterministic and occur far from the source of the invalid state.

  • Which exception triggers first is effectively probabilistic.

This is not a single-call bug, but a missing invariant that allows illegal state to exist and flow through the system.


Changes

  • Replaces a raw int with a domain-specific value type: GlobalJobArrayIndex
  • Encodes valid / uninitialized / unassigned states explicitly
  • Enforces index validity at all boundaries where it matters
  • Guarantees that:
    • ComputeBuffer.SetData
    • Skinning compute paths
      never receive invalid indices
  • Converts previously implicit assumptions into explicit, enforced invariants
  • Makes invalid state observable and diagnosable

This aligns the implementation with an invariant-driven design already used elsewhere in the codebase (e.g. BoneArray).

The scope is limited to enforcing a critical invariant, not reworking the entire data flow.


Summary

  • Prevents native rendering corruption by construction, not by convention
  • Replaces informal assumptions with explicit, type-level guarantees
  • Makes failure deterministic and traceable
  • Reduces the surface area of undefined behavior without large refactors
  • Establishes a pattern for future invariant-driven fixes

Test Instructions

Test Steps

  1. Run a scene with multiple avatars.
  2. Repeatedly create and destroy avatars, or attempt to reproduce the AllHands failure scenario.
  3. Observe runtime behavior and logs.

Expected Result

  • No ArgumentOutOfRangeException from ComputeBuffer.SetData
  • No NullReferenceException from native compute buffer paths
  • Invalid avatar states are detected and handled deterministically

Additional Testing Notes

  • This change enforces invariants, it does not silently mask errors.

Code Review Reference

[Code Review Standards](https://github.com/decentraland/unity-renderer/blob/master/docs/code-review-standards.md)

@NickKhalow NickKhalow self-assigned this Dec 17, 2025
@NickKhalow NickKhalow requested review from a team as code owners December 17, 2025 10:57
@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2025

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.

DCL.Diagnostics.EcsSystemException: [FinishAvatarMatricesCalculationSystem]

3 participants