Adding a predicate to select thin instance in ray picking function

I am using thin instances in my project to improve the performance and it has been very helpful!
However, there are some instances I wish to exclude from the ray picking. Currently, the pick function only accepts a normal predicate that supports selecting meshes, but there is no way I can select which instances to be pickable.

From the need above, I would like to make a PR to add a thinInstancePredicate parameter to the pick function so that we have a way to skip the picking for unwanted thin instances.

The pick method in the Scene class would look like this after the change

Scene.prototype.pick = function (
x: number,
y: number,
predicate?: (mesh: AbstractMesh) => boolean,
fastCheck?: boolean,
camera?: Nullable,
trianglePredicate?: TrianglePickingPredicate,
_enableDistantPicking = false,
thinInstancePredicate?: (mesh: AbstractMesh, thinInstanceIndex: number) => boolean
): PickingInfo

Please let me know what you think!

I like it, I wonder if it needs to be a separate params or just a new parameter of the predicate, could be -1 in case of none thinInstanced meshes.

Feel free to make a PR for it :slight_smile:

1 Like

Guys, I’m always very cautios and undecided whether using predicates is a good approach. Sometimes it’s just better/faster to filter the resulting array.

If we allow predicates for thin instances (or anything else) I propose to optimize the code and check for the existence of a predicate only once when entering the function. If it exists, it should run the appropriate code which will call the predicate function for every thin instance and into a second part which will run basically the same code without checking for the predicate in each loop while evaluating the thin instances thus we save some cpu cycles checking for an existing predicate.

Some horrible pseudo code:

if (thinInstancePredicate) {
 return pickWithPredicate(..., thinInstancePredicate)
}
return pickWithoutPredicate(...)

function pickWithPredicate(..., thinInstancePredicate) {
  thinInstancesMatrices.each(
     if (thinInstancePredicate && thinInstancePredicate(...)) {
       addToPickingResult()
     }
  )
}

function pickWithoutPredicate(...) {
  thinInstancesMatrices.each(
 //  if (thinInstancePredicate && thinInstancePredicate(...)) { <-- LEAVE THIS OUT
       addToPickingResult()
 //  }
  )
}
1 Like

I think the cost of GC will overweight the cost of a single if

1 Like

Yes, that’s the point :wink:

My point is that I vote for the if :slight_smile:

1 Like

Me too. To be precise if in the loop or if outside the loop?

outside probably