Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.

Most efficient way to do ColliderCasts that must do custom hits filtering inside parallel jobs?

Discussion in 'Physics for ECS' started by PhilSA, Nov 5, 2019.

  1. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    The following code is an example of how we can do parallel spherecasts in a job (kinda untested because I simplified the code a bit for this post). It's a projectile hit detection job.

    In this case, we can't just rely on the closest hit, because we want to be able to filter out hits based on specific gameplay considerations (ignore characters that are in a "dodging" state, ignore specific entities regardless of physics layers, ignore hits with a certain dot product threshold, do some piercing-bullets logic, etc....). So as a consequence of that need, we need a collector that can store several hits in order to manually filter them.

    Code (CSharp):
    1.  
    2. public struct HitDetectionJob : IJobForEach<Translation, Rotation, Projectile>
    3. {
    4.     [ReadOnly]
    5.     public PhysicsWorld PhysicsWorld;
    6.  
    7.     public void Execute(ref Translation translation, ref Rotation rotation, ref Projectile projectile)
    8.     {
    9.         bool foundHit = false;
    10.         ColliderCastHit closestValidHit = default;
    11.         closestValidHit.Fraction = float.MaxValue;
    12.  
    13.         // HERE
    14.         NativeArray<ColliderCastHit> castHits = new NativeArray<ColliderCastHit>(16, Allocator.Temp, NativeArrayOptions.UninitializedMemory);
    15.         // HERE
    16.         MaxHitsCollector<ColliderCastHit> collector = new MaxHitsCollector<ColliderCastHit>(1.0f, ref castHits);
    17.  
    18.         SphereGeometry sphereGeom = new SphereGeometry
    19.         {
    20.             Center = default,
    21.             Radius = projectile.radius,
    22.         };
    23.  
    24.         // HERE
    25.         BlobAssetReference<Unity.Physics.Collider> sphereCollider = SphereCollider.Create(sphereGeom, projectile.filter);
    26.  
    27.         ColliderCastInput input = new ColliderCastInput()
    28.         {
    29.             Collider = (Collider*)sphereCollider.GetUnsafePtr(),
    30.             Orientation = quaternion.identity,
    31.             Start = projectile.previousPos,
    32.             End = translation.Value,
    33.         };
    34.  
    35.         if (PhysicsWorld.CollisionWorld.CastCollider(input, ref collector))
    36.         {
    37.             for (int i = 0; i < collector.NumHits; i++)
    38.             {
    39.                 ColliderCastHit hit = collector.AllHits[i];
    40.                 if (hit.Fraction > 0f && hit.Fraction < closestValidHit.Fraction)
    41.                 {
    42.                     if ( /* do some additional filtering here based on some game-specific rules */)
    43.                     {
    44.                         closestValidHit = hit;
    45.                         foundHit = true;
    46.                     }
    47.                 }
    48.             }
    49.         }
    50.  
    51.         if (foundHit)
    52.         {
    53.             // here, "closestValidHit" is our closest valid filtered hit
    54.         }
    55.     }
    56. }
    57.  

    However, I get the feeling that it's not an efficient way to do this. The places with a "// HERE" comment in the code are the parts I'm concerned with. For each projectile, we allocate new hits arrays, create a new collector, and create a new sphere collider to cast with. I don't have a solid understanding of how costly any of these things are

    I have several thoughts on alternatives:
    • Would there be a way to allocate one hits array and one MaxHitsCollector per "thread" when the job starts, instead of allocating for every single projectile in the game?
    • Would it be safe/efficient to pass just one SphereCollider to the entire IJobForEach and resize it for every entity?
    • Would it be better if every projectile already had their physicsShape on their entity, instead of creating a new sphere collider for each? Even if we have 10k projectiles in game?
    • Is it maybe a better to launch several simultaneous IJobs (based on a thread count hint), each handling hit detection for a subset of all projectiles, and each with their pre-initialized hits array, hits collector, and sphere collider? But then if two jobs try to apply damage to the same entity, we'll have a problem....
    • Actually I'm realizing just now that I can't safely apply damage on hit with the approach posted above, because there would be parallel writing issues. I guess I'd need to write damage events into a concurrent NativeQueue and process those events later in an IJob

    How would you change this to make it more performant?
     
    Last edited: Nov 5, 2019
    Antypodish and NotaNaN like this.
  2. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    I just encountered this script and it's giving me some answers:
    https://github.com/Unity-Technologi...s/Common/Scripts/RayTracer/RayTracerSystem.cs

    I'm drawing two conclusions from this:
    1. I guess this means creating a new sphere collider for every new spherecast is okay.
    2. It seems like BlockStream.Writer is the best way to write results efficiently in parallel

    Now I just need to solve the problem of having to allocate a new array of hits and create a new Hits Collector on every iteration. Is there some kind of BlockStream-esque strategy for passing one preallocated writeable NativeArray<ColliderCastHits> to each thread?
     
    Last edited: Nov 6, 2019
    NotaNaN and florianhanke like this.
  3. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    New thought:

    In order to prevent allocating new arrays of hits in jobs, I'm considering 2 things and they are both collector-related:
    • Making a custom collector and doing the filtering inside the collector. That way I can do a "closest hit that satisfies some custom filtering rules" collector without requiring any array allocations. I would, for example, pass a NativeArray of ignored rigidbodyIndexes to the collector, and make it ignore those hits as it's collecting
    • If I do need to return several hits (imagine trying to implement a laser that pierces and damages everything except one specific blocking Entity that stops it), and also have the previously-described rigidbodyIndex filtering on top of that, would it be a good idea to simply pass a BlockStream.Writer to the collector and make it write directly in there instead of in a NativeList or NativeArray?
    If these are actually good ideas, is there a possibility that both of these concepts could be made more "official" in the future? Like having the ICollector.AddHit() have the rigidbodyIndex of the hit be properly updated when it is called, and maybe also a ICollector.RequiresRigidbodyIndex() to tell the system if we need to get the rigidbodyIndex before AddHit is called (in order to not lose performance for collectors who don't need it). And also having the builtin AllHitsCollector take a BlockStream.Writer as parameter?
     
    Last edited: Nov 6, 2019
    NotaNaN likes this.
  4. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    3,602
    I keep forgetting to check this subforum at night, but I am pretty sure that allocating the SphereCollider per job is a memory leak. The SphereCollider ultimately gets malloc'ed with Allocator.Persistent.

    As for BlockStreams, they are a pre-evolved form of NativeStream. Unity.Physics hasn't switched over to NativeStream yet, though I suspect that is planned.
     
    NotaNaN likes this.
  5. Rory_Havok

    Rory_Havok

    Joined:
    Jun 25, 2018
    Posts:
    70
    Those are both good ideas.
    Improving usability of ICollector.AddHit() to always have the rigidBodyIndex is on the TODO list (I don't think there is much perf benefit of offering an opt-out for that).
    AllHitsCollector taking a native stream writer instead of native list is a great idea. Should be trivial to add that. (PS We have already switched to NativeStream and deleted BlockStream, that will be in the next release.)

    About your earlier question of the sphere allocation, it is overkill to allocate one per cast. You only need one per thread and you can modify its parameters before each cast. I would probably do this by allocating a pool of them beforehand and use [NativeSetThreadIndex] to select which to use per job.

    Longer term it probably makes sense for us to add a CastSphere() so you don't have to deal with allocating the sphere at all - you just provide the parameters and we build what we need on the stack - it is trival to build a sphere collider after all (same for capsule/box/etc).
     
  6. justin_sunblink

    justin_sunblink

    Joined:
    Dec 3, 2019
    Posts:
    12
    Any ETA on adding RigidBodyIndex to the hits? I was attempting to implement a custom collector that picks the first hit satisfying some criteria, but without a body index I cannot access the owning entity to actually implement my checks.

    It's a pretty significant performance issue for me to be forced into collecting *all* hits when I only need to care about one.
     
    lclemens likes this.
  7. sstrong

    sstrong

    Joined:
    Oct 16, 2013
    Posts:
    2,127
    Check doco here. RaycastHit.RigidBodyIndex seems to work for us.
     
  8. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    We're talking about accessing hit rigidbodyIndex from the AddHit() callback in a custom collector. Currently it's not possible
     
  9. justin_sunblink

    justin_sunblink

    Joined:
    Dec 3, 2019
    Posts:
    12
    Yep, this.