Dispatch traits aren't composable

[Ralith]

When processing queries on a compound shape, QueryDispatcher methods must be invoked recursively. This makes it infeasible for custom shapes that occur inside compound shapes to be handled. Ideally downstream code could provide chainable QueryDispatcher impls that only handle the shapes they introduce and otherwise return Unsupported, but in that case a compound shape would fall through to the default dispatcher, which would recurse internally and fail to handle custom shapes it encounters. Similar issues affect any user-defined composite shapes. This could be fixed by adding a root_dispatcher: &dyn QueryDispatcher argument to every trait method. The same issue also affects PersistentQueryDispatcher.

I can bang this out, but it's a bunch of boilerplate updates and given that this logic was not preserved from ncollide I want to be sure it's welcome before proceeding.

[sebcrozet]

Hi! I feel like adding a root_dispatcher arguments to all dispatch methods are a bit too intrusive, and makes it weird to use by the end-user (who has do call something like dispatcher.distance(dispatcher, ...).

A more transparent way of doing this is through composition:

1. Change the existing DefaultQueryDispatcher to a:

struct BaseQueryDispatcher<'a, D> {
    root_dispatcher: &'a D
}

impl QueryDispatcher for BaseQueryDispatcher {
    fn distance(...) {
        // ...
        if let Some(c1) = shape1.as_composite_shape() {
            Ok(query::details::distance_composite_shape_shape(
                self.root_dispatcher, pos12, c1, shape2,
            ))
        }
        // ....
    }
}

2. Redefine a new DefaultQueryDispatcher that calls the BaseQueryDispatcher methods (to avoid code duplication):

struct DefaultQueryDispatcher;
impl QueryDispatcher for DefaultQueryDispatcher {
    fn distance(...) {
        let dispatcher = BaseQueryDispatcher { root_dispatcher: self };
        dispatcher.distance(...)
    }
}

And then you can write your own dispatcher that passes itself to the BaseQueryDispatcher. That way there is no need to change the signature of the dispatcher trait methods.

[Ralith]

That does look better. It might take some creativity to make composing dispatchers ergonomic (so that independently implemented custom shapes can be used without the end user writing a ton of boilerplate), but I think it's tractable; I'll play with it.

[Ralith]

Consider two libraries that define custom compound shapes, and an application that wants to use both. Ideally the application, which does not itself define any custom shapes, should not have to implement QueryDispatcher. I don't think the revised proposal supports that, since there's no way for parry to provide for composition of recursive dispatch logic, unless I'm missing something.

[Ralith]

If external crates are to composably define shapes that need to dispatch recursively (i.e. compound shapes similar to HeightField), then I don't see any alternatives to the original proposal. However, end-user ergonomics could be preserved by relegating the root_dispatcher argument to a trait for use only by shape implementers, and turning QueryDispatcher into a blanket-implemented trait that encapsulates the self.query(self, ...) dance, and provides a chain helper for composition. How does that sound?