Page 1 of 1

Raycast with filter additional filterdata parameter

Posted: Sat Sep 28, 2013 6:48 am
by ecosky
Hi Norbo,

I recently came across a problem while using Space.RayCast that was best solved by adding an additional "object filterContext" parameter to Space.RayCast, Space.ConvexCast, and all dependent code. Having the extra filterContext passed down to the user provided filter method allowed me to make some ray cast queries that needed extra data to be thread safe without jumping through a lot of hacks. Providing context object with a callback is pretty common as you know so hopefully you will find this change something worth keeping around.

These are the root of the changes in ISpace, Intellisense will point out a few dozen or so trivial changes needed to propagate these to the filters which would be easier to implement directly than merging diffs if you are interested in adding this. The biggest issue here IMHO isn't the complexity, I'd expect it to be more about changing the API.. anyway, I've made the changes locally and hope to not have to remerge them later ;)

Code: Select all

        // ISpace.cs

        bool RayCast(Ray ray, Func<BroadPhaseEntry, object, bool> filter, object filterContext, out RayCastResult result);
	bool RayCast(Ray ray, float maximumLength, Func<BroadPhaseEntry, object, bool> filter, object filterContext, out RayCastResult result);
        bool RayCast(Ray ray, float maximumLength, Func<BroadPhaseEntry, object, bool> filter, object filterContext, IList<RayCastResult> outputRayCastResults);
Thanks again,

Re: Raycast with filter additional filterdata parameter

Posted: Sat Sep 28, 2013 7:04 pm
by Norbo
The suggested way to solve this is to use a closure that encapsulates the needed state in a type-safe way. This avoids a cast and keeps the API a bit cleaner.

Is there a case/platform where closures wouldn't work?

Re: Raycast with filter additional filterdata parameter

Posted: Sat Sep 28, 2013 8:37 pm
by ecosky
Thanks for the reply.

I was looking at using closures, but they would generate garbage unless I take care to prepare and maintain a collection of unique closures that allow me to store the extra data required by the filter. Preventing garbage is really the core motivation behind adding the extra parameter. In my particular case, I could get away with making my higher level code more aware of and dependent on Bepu types, but it seems cleaner to propagate a general context object to the filter so that this extra dependency could be avoided. My current setup allows me to call a fairly generic raycast/convex cast wrapper using local game types that don't need to be aware of the inner workings of Bepu.

The rest of this is just more detail on what I'm doing if you are curious.

For my use case, in general terms, is I have an object handle (ICollidable) stored in the Entity.Tag which I do application-specific filtering. This handle is an object that allows game code to discover what type of game object is associated with an EntityCollidable. My filter method is FilterEntitiesByHandle, which basically extracts the ICollidable from the BroadphaseEntry (if available) and then calls the context-specific filtering logic which I now pass down through the filterContext object.

The code below is where I wrap access to the Bepu casting apis to allow me to more conveniently filter using my own local game-types. This wrapping centralizes the discovery of the local game types so I don't have to reimplement that logic for each of my game filters. The final results of my wrapper to ConvexCast is a list of game entities, as opposed to Bepu types.

Code: Select all

		Func<BroadPhaseEntry, object, bool> FilterEntitiesByHandleCallback;
		Func<BroadPhaseEntry, object, bool> FilterEntitiesCallback;

		private ICollidable TryGetCollidable(BroadPhaseEntry entry)
		{
			var iCollidable = entry as ICollidable;

			if (iCollidable == null)
			{
				// Check for acceptable tags for this entity.
				iCollidable = entry.Tag as ICollidable;
				if (iCollidable == null)
				{
					var entCol = entry as BEPUphysics.Collidables.MobileCollidables.EntityCollidable;
					if (entCol == null)
					{
						return null;
					}
					var ent = entCol.Entity;
					if (ent == null)
					{
						return null;
					}
					iCollidable = ent.Tag as ICollidable;
					if (iCollidable == null)
					{
						return null;
					}
				}
			}
			return iCollidable;
		}

		bool FilterEntities(BroadPhaseEntry entry, object filterContext)
		{
			var entity = entry as ICollidable;

			if (entity == null)
				return false;

			if (entry.CollisionRules.Personal > CollisionRule.Normal)
				return false;

			return true;
		}


		bool FilterEntitiesByHandle(BroadPhaseEntry entry, object filterContext)
		{
			var entity = TryGetCollidable(entry);

			if (entity == null)
				return false;

			if (entry.CollisionRules.Personal > CollisionRule.Normal)
				return false;

			var handle = entity.EntityHandle;
                       // return Filter(ref handle)
			return ((FilterEntity)filterContext)(ref handle);
		}

		//FilterEntity Filter;  <-- the old single thread callback
		FactoryPool<List<BEPUphysics.RayCastResult>> OutputRayCastResultPool = new FactoryPool<List<RayCastResult>>(1, () => new List<RayCastResult>());
		//List<BEPUphysics.RayCastResult> OutputRayCastResults = new List<BEPUphysics.RayCastResult>();

		public bool ConvexCast(ref ConvexCastRequest request, FilterEntity filter, out BBG.Engine.Physics.RayCastResult outputCastResults)  // <--- note my RayCastResult is a different type than Bepu's and stores game entity info.
		{
			bool result;
			BEPUphysics.RayCastResult castResult;
			if (filter != null)
				result = Space.ConvexCast(request.Shape, ref request.Transform, ref request.Sweep, FilterEntitiesByHandleCallback, filter, out castResult);
			else
				result = Space.ConvexCast(request.Shape, ref request.Transform, ref request.Sweep, FilterEntitiesCallback, null, out castResult);

			if (!result)
			{
				outputCastResults = default(BBG.Engine.Physics.RayCastResult);
				return false;
			}
			var entity = TryGetCollidable(castResult.HitObject);
			CollidableHandle entityHandle;
			if (entity != null)
				entityHandle = entity.EntityHandle;
			else
				entityHandle = default(CollidableHandle);

			outputCastResults = new BBG.Engine.Physics.RayCastResult()
			{
				Location = castResult.HitData.Location,
				Normal = castResult.HitData.Normal,
				T = castResult.HitData.T,
				Entity = entityHandle
			};
			return true;
		}

In the single threaded case (which is not the code shown above), I called raycast/convex cast, providing a filter method that I retained the delegate for and passed that to the raycast method instead of continually generating the closure. It might not seem like much garbage but it does add up in my situation given the volume of calls, so I prefer to cache these delegates. The FilterEntitiesByHandle would use a member "Filter" (commented out in the code) instead of having it passed through as shown above. The problem with this of course is there is only one member Filter, and when I started using this with multithreading there was obviously a conflict there. This is when I added the filterContext to the arguments to the parameters.

Now, I suppose I could make my game logic filters (the methods being tracked by filterContext) have the same signature as the Bepu filter, and have them all embed the logic currently handled by FilterEntitiesByHandle, but I had been hoping to keep the logic a little higher level since these filters are currently general purpose and passing the game filter as a parameter to the Bepu filter keeps the game filters independent of Bepu types and generally a little simpler to use from the game logic side of things.

In practice, I don't have a huge number of ICollidable filters so I could convert them to match the Bepu signatures. It just seemed cleaner to add an extra parameter to the arguments and pass it down to the filters, because that way I could continue to make filters that only have to be aware of and process game-specific types extracted by the common FilterEntities...Callback delegates. So, to wrap this up, I'm not saying there is a flaw in the api design, just that the extra context parameter would be useful in situations where you don't want to create unique closures for each call to the ray/convex cast methods or if you want use a standard Bepu filter to wrap a game specific filter after extracting the game data from the BroadPhaseEntry.

Thanks,

Re: Raycast with filter additional filterdata parameter

Posted: Sat Sep 28, 2013 11:47 pm
by Norbo
To handle that case without a context parameter in a simple way while avoiding a bunch of allocations, a 'manual' closure could be created: a class that contains your desired state and instance function that uses that state. The delegate passed into the ray cast would be the instance function. A set of these closures would be stored locally on the threads or in a pool for reuse, just like a state object would otherwise need to be stored/pooled.

Built in closures could also be used similarly by explicitly pairing the closure with the state it contains. There's a little silliness to this since the closure already contains the state, but it can make it easier to access. (There might also be a way to access the state in a proper closure directly. I haven't bothered to look, but I assume it would be a little roundabout if it did exist.)

While these solutions can be a little more obtuse in some cases, the difference is small enough that I lean towards avoiding the breaking change and keeping a less parameterful signature.

Re: Raycast with filter additional filterdata parameter

Posted: Sun Sep 29, 2013 5:19 am
by ecosky
Understood. Thanks for taking the time to review my suggestion. I can appreciate not wanting to change the API for such a small thing.