Contact.Normal direction inconsistent - expected or a bug?

Mar 26, 2009 at 4:27 PM
I have some jumping code that works fairly well (see end of post). Well it did work until I tried to let the player jump on top of enemies as well as the terrain.

Some info:
 - Tries to ensure the player is on a surface with a normal that is mostly upwards
 - The player is a circle geom
 - The jumpable surface Cat1 is a static polygon geom
 - The jumpable surface Cat4 is a dynamic circle geom
 - The problem is that Contacts with the Cat1 polygon geom reports the normal in the opposite direction to the Cat4 circle geoms.

I just resisted posting this and did some stepping through the engine code for Arbiter.Collide(). It seems that it (rightly) swaps geometry1 and geometry2 when calling geometry2.OnCollision(), however, it does not reverse the contact normals, so the user's implementation of OnCollision has no indication whether they are facing towards or away from geometry1.

Would anyone agree that this could be considered a bug? Maybe I've missed something, but it doesn't seem right to me.

 Here's the code.
//////////////////////////////////////////////////////////////////////////////////
        bool canJump = false;
        float jumpDisableTime = 0f;

        bool PlayerCollisionEvent(Geom geometry1, Geom geometry2, ContactList contactList)
        {
            Vector2 avgNml = Vector2.Zero;
            foreach (Contact ct in contactList)
            {
                avgNml.X -= ct.Normal.X;
                avgNml.Y -= ct.Normal.Y;
            }
            avgNml.Normalize();

            // Ensure player is on a jumpable surface and the normal is mostly upwards
            CollisionCategory jumpSurface = CollisionCategory.Cat1 | CollisionCategory.Cat4;
            if ((jumpSurface & geometry2.CollisionCategories) != CollisionCategory.None && avgNml.Y > 0.5f )
            {
                canJump = true;
            }
            return true;
        }
        void PlayerSeparationEvent(Geom geometry1, Geom geometry2)
        {
            canJump = false;
        }

            // And in Update()

            if (Input.ButtonDownMapped((int)PlayerInput.Jump) &&
                canJump &&
                jumpDisableTime <= 0.0f)
            {
                Vector2 up = Vector2.UnitY * jumpScale;
                bodyBody.ApplyImpulse(ref up);
                jumpDisableTime = 0.1f;
            }
/////////////////////////////////////////////////////////////////////////////





Mar 26, 2009 at 4:45 PM
I also wanted to test my theory. My idea was to simply reverse all the contact normals just before Arbiter.Collision(...) calls "geometry2.OnCollision(geometry2, geometry1, contactList)", however I ran into a stumbling block. Since ContactList is a generic list of structs, I would need to rebuild the whole list to change one value in each list entry :(...    Some days I love c# and some days I really don't. I can't really see a good solution to this on the engine side.

I guess since my player is simple circle I can use the vector from the contact point to the player position instead of the contact normal. Oh well more hacks  :)
Coordinator
Mar 26, 2009 at 4:48 PM
I've had another user ask about this: http://farseerphysics.codeplex.com/Thread/View.aspx?ThreadId=48482 (where the hell is my link button?...)
I will investigate the problem further.
Mar 26, 2009 at 5:27 PM
Thanks genbox. I've also got no link button, just a "break link" button... which is pretty useless under normal circumstances and completely useless without a link button  :D

I would second Yobiv's conclusion that the normal direction depends on the creation order. In terms of a solution, I can think of a few things, but none of them are pretty!

1) Change ContactList to store a Contact[] (allows easy normal reversal but breaks compatability)
2) Copy the ContactList to a Contact[] before passing to the OnCollision calls (still breaks compatability)
3) Pass a boolean to OnCollision to specify the normal swap (breaks compatability and forces user to reverse normals)
4) Don't swap the geometries and let the user work out which is which (doesn't break compatability, but as a user... yuck!)

I would personally tend towards option 1) on this, even though it does break compatability... but then I don't have a large project based on the engine (yet). Hopefully you can think of some better options.
Coordinator
Jun 10, 2009 at 11:54 PM
Edited Jun 10, 2009 at 11:55 PM

I've been looking into the "bug" (A feature actually) with the contact normals and everything is setup the correct way engine wise.

The engine arranges the geometries in the order in which they are created. It needs some way of determine what geometry to check first (in a consistent way) and in this case, the Id of the geometry is used. The geometry created first gets Id 0, the next is 1, and so on.
The geometry with the lowest Id always gets ordered as GeometryA in the Arbiter, this makes sure that the collision response is consistent.

So basically the engine asks the question: In what direction do I need to apply impulse in order to separate geometry A from geometry B.

Whenever you switch the geometries around in your sample, that question is still being answered correctly by the engine and the proper result comes out of it.
It does not care what geometry is on top. - All in all it does not care about orientation of the geometries.

Now, I've been thinking about this and I've not been able to come up with a decent fix. It would not be any good if I introduced any kind of orientation-awareness into the engine. If you have any ideas, let me know.
I still think that the solution that checks the orientation is the one to use. (simply: if (geom1.Position.Y > geom2.Position.Y) ) is better than relying on the contact normals.

Jun 11, 2009 at 6:00 AM

Thanks for looking into this some more genbox. I noticed in the source check ins you've made Geom.Id public so now the user can manually reverse the contact normal based on geom1.Id > geom2.Id. I think this is much better than just testing the orientation in the OnCollision event. Relying on geom1.Position vs geom2.Position is bad because both of the geoms might be concave or otherwise oddly shaped, meaning that the contact normals would be the only accurate approach.

When you say "It would not be any good if I introduced any kind of orientation-awareness into the engine", is this because the engine uses the contact list internally as well as passing it to the user? Either way, it is an unfortunate user experience to have a list of contacts with normals that may be pointing towards or away from geom1 based on the geom's order of creation. I still think my suggestion 1) above would be feasible, however I don't fully understand how the engine uses that contact list.

 

Coordinator
Jun 11, 2009 at 6:07 AM

Their position is based on the centroid, but you are right, you might have a very oddly shaped geometry.

As for the Id, I've made it public so that you can use it in your games, doing a geom1 > geom2 would yeld the same result because it also uses the Id internally.

The contact list is indeed used internally by the engine. Once the contactlist has been populated, it is used in arbiter.cs. Each contact contains separation and the normal (from geomA to geomB) and the arbiter applies an impulse that is required to separate the two geometries.

The engine does not know what is up or down like we do. It only knows what direction to apply impulse in. I hack might be in place that simply give the user the desired normal while the real normal is kept internally. If you come up with a working solution, be sure to let me know. If you have any question regarding some code or mechanisms, feel free to ask me.

Jun 11, 2009 at 10:30 AM

I haven't tried it yet, but how does this sound as an option? :

1) Add an internal bool Contact.reverseNormal

2) Make Contact.Normal a public property. Keep Contact.normal as an internal member and use this in Arbiter.cs. In Contact.Normal getter, reverse direction if reverseNormal is true.

3) Before calling the OnCollision delegate, set the reverseNormal to true based on the geoms' Ids.

 

I guess the main disadvantage is that it will break compatability for any users who pass the Contact.Normal into a function by ref. I'm guessing this would be a small amount and worth the change if they now get a reliable Contact.Normal direction.

Jun 12, 2009 at 10:43 AM
Edited Jun 12, 2009 at 1:01 PM

i personally would like to know the normal of the edge of the other Geom (geom2) given the Contact point

if Contact.Normal gives the seperating direction, i think it should be merged with Contact.Seperation to a Contact.SeperationForce vector