Skip to content

Conversation

bcolloran
Copy link

@bcolloran bcolloran commented Apr 17, 2025

Adds a simple getter for the contact_id field of SolverContact. (Alternatively, could set that field to pub instead of pub(crate).)

My use case is that I need to access the local coords of a contact relative to each collider involved in the contact. To get that data, I need the contact_id to look up the entry in contact_pair.manifolds[i].points that corresponds to a specific SolverContact, because the SolverContact only has the world space position of the contact point. Example:

     narrow_phase
    .contact_pairs_with(this_handle)
    .filter(|contact_pair| {
        // filter out pairs that are not active
        contact_pair.has_any_active_contact
    })
    .flat_map(move |contact_pair| {
      
        let manifold = &contact_pair.manifolds[0];

        manifold.data.solver_contacts.iter().map(move |contact| {
            let contact_point = manifold.points[contact.contact_id() as usize];
            (
                contact_point.local_p1,
                contact_point.local_p2,
                manifold.local_n1,
                contact_point.dist,
                manifold.data.rigid_body2,
                contact_pair.collider2,
            )
        }
    })

Near as I can tell, there is not any existing shortcut API for accessing the local coords that I need. But that's ok as long as we can get this id; however there is no way of accessing the id either.

Since this is just a simple getter, and doesn't e.g. allow mutation of this id, I think this should be a trivial, non-risky way of satisfying my use case. (But then again I'm not sure why this field wasn't pub in the first place, so there may be some detail I'm not aware of.)

Please LMK if any changes are needed! Thanks!

@ThierryBerger
Copy link
Contributor

ThierryBerger commented May 19, 2025

This PR is controversial, because of the way rapier works: Several steps can occur in a given simulation, when that happens, the contact_ids may not be the same, or not exist anymore after the simulation, so they should not be made available in arbitrary places.

I'm curious how other physics engines (box2d..?) handle this fine-grained collision detection.

@bcolloran
Copy link
Author

Thanks for getting back to me @Vrixyz !

Dang, I didn't realize this would be controversial at all. I have been using my own Rapier fork with this workaround in my project, but it sounds like it may be unreliable? It has seemed to be working fine so far, but I'm not sure of all the edge cases you may be thinking of.

Can you think of any other workaround for getting local coords of a collision point relative to the bodies involved that already works in Rapier without needing this? I tried to dig around, but there is a lot of API surface, so I might have missed the way I'm suppose to do this :-)

Perhaps another option would be to just add a better doc comment to this proposed getter? I simply copied the comment from the existing field in the struct, but it could be expanded to add warning and caveats about whatever issues you think might be most concerning. For example, I think it should be clear to most users of a physics engine that a contact_id will only be valid within the simulation tick on which it was generated, but if this could be ambiguous it could be spelled out in the comment on this getter method? Maybe something like this?--

/// Returns the index of the manifold contact used to generate this solver contact.
///
/// Note that this index is only valid on the simulation tick on which the contact occurred, and should only be used to look up information about that manifold contact within that tick. Storing or serializing the index itself is not recommended.

Perhaps adding a warning like that would allow people like myself who need this low-level access to the existing local collision coords to get it, but at least warn people less experience about a potential pitfall?

Or alternatively, in addition to that warning in the docs, this could even be made an unsafe method to really emphasize to users that they are on their own if they go messing around with internal indices?

I'm just trying to think of some way to:

  1. unblock the use case that I am trying to achieve (I don't know how box2d or other engines handle this internally, but I believe getting contact point relative to contact bodies is a standard workflow, so it would be great if we could expose it somehow! :-) )
  2. address your concerns about inexperienced users misusing this index, but also
  3. expose this in a way that won't require a major rewrite or rethinking of how this currently works (i.e., adding some kind of handle instead of these id, or a closure that somehow gives access to the manifold points without exposing the contact_id etc seems like it might be more challenging?)

Thank you again for looking at this, please let me know if you have any ideas! :-)

@bcolloran
Copy link
Author

Hi @Vrixyz any thought on this? Please let me know if you think the warning in the method comments would be enough, or if you have other recommendations on how to proceed. Thanks! :-)

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.

2 participants