Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Possible Unity Bug? Contact Normal is NaN in OnCollisionEnter.

Discussion in 'Editor & General Support' started by Andrew-Garrison, Oct 28, 2014.

  1. Andrew-Garrison

    Andrew-Garrison

    Joined:
    Oct 3, 2012
    Posts:
    19
    I'm getting a NaN for the ContactPoint.normal in the OnCollisionEnter method. I thought it was my code for hours until I finally discovered the root of the NaN was in the OnCollisionEnter method when Unity passed me a bad contact normal.

    Has anyone else seen this? It seems that this would certainly be a bug, I don't know why Unity would pass in an invalid normal.

    Here's the code I'm talking about:

    Code (CSharp):
    1.       private void OnCollisionEnter(Collision collision)
    2.       {
    3.          foreach (var contact in collision.contacts)
    4.          {
    5.             if (float.IsNaN(contact.normal.x))
    6.             {
    7.                Debug.Log("Unity, why are you doing this to me?");
    8.             }
    9.          }
    10.       }
     
    Menion-Leah likes this.
  2. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    I'm experiencing the same issue with ContactPoint2D and OnCollisionEnter2D: relativeVelocity and tangentImpulse/normalImpulse are randomly set to NaN (IE: running the same identical simulation, approx 1 time on 1000 those values are set to NaN)
     
  3. paul-masri-stone

    paul-masri-stone

    Joined:
    Mar 23, 2020
    Posts:
    49
    @Menion-Leah I am experiencing this with ContactPoint2D.tangentImpulse in Unity 2020.1. I'm getting it about 1 in 4 so I've been able to capture video and submit a bug report. If I can get a public link to the bug report, I'll post it here.
     
    Menion-Leah likes this.
  4. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    That would be awesome. Thanks.
    As I side note, I discovered that in my case this behaviour happens more often when I check for secondary impact points (IE: second, third, fourth.. collision points in the Collision2D.contacts array).
     
    paul-masri-stone likes this.
  5. paul-masri-stone

    paul-masri-stone

    Joined:
    Mar 23, 2020
    Posts:
    49
    Menion-Leah likes this.
  6. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
  7. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Hey all.

    So this bug was on my radar today so after a day of trying to narrow it down I believe I've found what the issue is or at least one source of the issue but I suspect this is the problem.

    For continuous collision detection the contacts are updated during the sweep. In Box2D, each fixture/fixture contact is in a contact manifold that can have up to 2 contact points and we maintain unlimited manifolds to send to you as ContactPoint2D. The problem was that in certain circumstances during the pre/post solve cycle, the number of contacts in a single manifold can go from 1 to 2 for polygon collision. This is fine but we maintain a sum of both normal/tangent impulse which starts by them being reset. The problem was that they were reset with 1 contact point then the contact was updated (by Box2D) and 2 contacts were then present and both normal/tangent impulse were added, unfortunately the 2nd one was not initialized to zero so was floating/undefined, leading to bad values and even NaNs.

    Now these are reset no matter how many contacts are present in the manifold and for the last hour I've not been able to reproduce the issue. Reverting that changes results in reproducing the issue pretty quickly.

    So I would say the issue (https://issuetracker.unity3d.com/is...returns-nan-when-called-in-oncollisionenter2d) is tentatively fixed. I'm producing a build now for a QA engineer to test. When that's done I'll submit it to my next round of bug fixes.

    Sorry for the issues this has caused but I think it's finally been narrowed down and fixed. I'll post back here when it's either been confirmed fixed or the QA engineer can reproduce the issue still.
     
    Menion-Leah likes this.
  8. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    Thanks, that's awesome news!
     
    MelvMay likes this.
  9. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,520
    @MelvMay thanks a lot for an awesome, detailed summary. About this:

    Can we infer this issue does not apply when using discrete collision detection?
     
    MelvMay likes this.
  10. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Right now it's looking that way as it's the most direct way I found that would cause this but I'll add a little caution on my stating that it's true because in theory there's a few other paths that could potentially trigger this. Those though would be covered by this fix because regardless of contact count, the normal/tangent impulse arrays are fully reset.

    QA are testing this fix today. If you have any other simple reproduction project you want me to host or you can send to me then please do so and I can test it directly against this fix. If you want me to host then DM your email address and I'll set-up a private workspace. Please only consider relatively simple reproduction projects if possible as I'll need to upgrade them to our latest trunk.
     
    Kurt-Dekker likes this.
  11. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Just want to confirm that QA gave this a pass and it fixes the issue completely. Nobody was able to reproduce the issue at all in the new build.

    Good to go.
     
    Kurt-Dekker likes this.
  12. paul-masri-stone

    paul-masri-stone

    Joined:
    Mar 23, 2020
    Posts:
    49
    @MelvMay That's awesome news. Thanks for looking into this and resolving it. I look forward to removing my shim!

    Which release of Unity will contain this fix?
     
  13. paul-masri-stone

    paul-masri-stone

    Joined:
    Mar 23, 2020
    Posts:
    49
    Actually I can see by looking at the bug report that it's "Fix in review for 2021.2.X" which I presume means it hasn't been reviewed yet and will hopefully be part of 2021.2.X.

    (Not sure why I didn't get a notification about the update on the bug report as I'd filed that. :confused:)
     
  14. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    It says "Fix in review" until it lands in the branch and the case closed AFAIK. The PRs have passed on the build-farm, had a code review and a successful QA pass so are just waiting to go into the release streams themselves.
     
    paul-masri-stone likes this.
  15. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    Hi @MelvMay!
    Is there any news about this fix landing on 2019.4.x?

    I cannot find anything related.
    Thanks!
     
  16. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    It wasn't backported to LTS releases. I believe the risk of doing so was too high due to the changes involved to fix the issue. Not everything is allowed to be backported unfortunately although most my fixes are.

    The metric known as "user pain" needs to be high enough to justify risks. This happens all the time with backporting bug fixes.
     
  17. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    Oh, ok.
    Thanks for you quick answer.
     
  18. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    It's not to say it cannot be reconsidered. Sometimes the decision can be changed. I've set myself a reminder to go over the code changes and see if we could ask for a backport but right now it'd be to 2020 and then to 2019. I've set myself a reminder to look at this tomorrow.
     
  19. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    Thanks, @MelvMay

    This exact bug is not impacting us in a dramatic way: the workaround I found is just to ignore collision contacts after the first one, in
    OnCollisionEnter2D
    .

    I'm asking because I'm facing new bugs and I was wondering if they could be somehow related to this one.
    For instance, I can see inconsistent results on some collisions, and also you can get some discrepancies when setting Rigidbody rotations - basically, this code:

    Code (CSharp):
    1. transform.localRotation = Quaternion.Euler(0, 0, - rotation);
    2. rigidbody.rotation = - rotation;
    gives some weird results on the last significant digits, on some devices:

    Code (CSharp):
    1. samsung SM-J530F | Android OS 9 / API-28 (PPR1.180610.011/J530FXXS8CUB1)     BEFORE: rigidbody.rotation: 34.6667023 - AFTER: -34.6667023
    2. samsung SM-J600FN | Android OS 10 / API-29 (QP1A.190711.020/J600FNXXUACUD1)  BEFORE: rigidbody.rotation: 34.6667023 - AFTER: -34.6667061
     
    Last edited: Aug 2, 2021
  20. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Different devices are not guaranteed to give you the same floating-point results. That isn't a bug but I know you know this because we've had this discussion before I recall (at least I think so).

    Anyway, has nothing to do with the bug report which was a logic issue.

    To be clear, there's no platform specific code in 2D physics. Differences like this are due to floating-point handling only whether that comes from the platform or device. Neither 2D or 3D physics are fully deterministic because of this issue.
     
  21. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    Yes, we had. :)

    Sorry, I think I didn't explain myself correctly.
    The bug I'm seeing on setting rotation is not dependent on the model. It can give you different results on the same device.
    Also, I find quite strange that when I just set a rotation to be - X, I'll get -Y.
    In this case, 34.6667023 sometimes becomes -34.6667023, sometimes -34.6667061.
     
  22. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    The above code is assigning a Vector3 to a float value yet you say this is the 2D physics code?

    EDIT: I read it wrong. Not sure what the Transform part is all about and how it relates to the Rigidbody2D so I was confused over the assignment.
     
  23. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    So assigning Rigidbody2D.rotation simply stores that value, it doesn't modify it in any way so I don't see how it can be different if you assign it one value then read it back.
     
    Menion-Leah likes this.
  24. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    I left them both (transform rotation and rigidbody rotation), because I'm not sure which one of the two is messing up with the value.
     
  25. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Right well there's no bug possible, we are storing what you pass. I will modify what I said because you assign a value in degrees but it's stored in radians. When you read, it's converted to radians but that's just the usual multiplying by Mathf.Deg2Rad and Mathf.Rad2Deg.

    Unity stores it directly in the Box2D body like this.

    Code (CSharp):
    1. m_Body->SetTransform(m_Body->GetPosition(), Radians(rotation));
    It always has been and likely always will be the above code.
     
  26. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    Good to know..
    The point is that rigidbody.rotation is called after the transform assignment.
    So, if it just stores the value, I should be able to see it correctly.
    The two last lines to be executed are:

    Code (CSharp):
    1. rigidbody.rotation = -34.6667023f;
    2. entity.LogMatchInfo(string.Format("BEFORE: rigidbody.rotation: {0:G9} - AFTER: {1:G9} ({2})", rotation, rigidbody.rotation, gameObject.name));
    3.  
    ...and yet it sometimes return something like -34.6667061.
     
  27. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
  28. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    I still don't get how the formatting or the DegToRad could affect operations performed on the same device, same build...

    Nevermind, I got rid of that just by truncating the last few significant digits when I'm setting some rigidbody rotation; my main concern was that this behaviour is internally creating some inconsistencies in reproducing the same simulation again, as the original bug in this thread does.
     
  29. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Well the physics engine isn't a place to address this because how do I fix a single multiply? You have some other issue there not related to physics. Please understand that floating-point problems like this are not caused by the physics engine. The physics engine is often used to demonstrate them though. It applies to everything.

    Assign this value to something else? Perform a ToRad and ToDeg on it yourself them format the output and see what happens.
     
  30. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    Yes, and thanks to your explanation now I better understand how it works, internally.
    Unfortunately, since the Physics2D engine is not open-source (as UI, for instance), it's more difficult to understand what's happening and to find the culprit to some incorrect behaviours.

    I did what I can to fix the effects of this strange floating point-problem on my side, but I also suspect it's affecting the physics engine somehow... and I cannot put my hands in there. That's why I asked you, that know it better, if that could be the case, if it could be somehow related to the OP, and if you were aware of this bug existance.
     
  31. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Well open source or not, there are no random behaviours or code-paths in the physics engine so it's deterministic. The only part that stops it being fully deterministic is the CPU floating-point problem. The thing you're seeing here has nothing to do with the problem reported above and I don't even see the relationship TBH.

    95% of the physics engine is Box2D. The Unity side of things is authoring set-up, object handling and reporting. Setting properties like this pretty much just go through to Box2D. The simulation itself is 100% Box2D. You can see the source for that. We have a bunch of fixes in it but it's pretty much stock Box2D.

    Do the same in 3D physics. Do the same with animation or UI. Check if the differences appear in one place but not another.
     
  32. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    The bug this thread is discussing is breaking determinism, without involving CPU floating-point issues.
    That's why I asked if some other inconsistencies I'm seeing could be related to 2D Physics, especially since I spotted them in
    OnCollisionEnter2D
    and in
    rigidbody.rotation
    .

    Thanks for your answers.
     
  33. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    I don't see that subject at all. The original post was about 3D physics and nothing to do with even the 2D content. I saw a bug reported for 2D physics in returning a value, that's not related to determinsim and floating point. By that thinking, any bug in physics breaks determinsim and is the same thing. It doesn't matter though, I think we've established this needs more investigation or maybe you're happy with your solution.

    Don't here's anything more I can add here.

    Good luck! :)
     
  34. Menion-Leah

    Menion-Leah

    Joined:
    Nov 5, 2014
    Posts:
    189
    90% of the post in this thread are about a 2D physics bug, and the bug report itself is about it.

    And I would agree with you if this bug would have had a deterministic behaviour: same input, same error.
    But it doesn't: same machine, same build, same simulation - sometimes it gives back a correct value, sometimes it gives back NaN.

    This unpredictable, inconsistent behaviour, in my opinion, is about breaking determinism, not the NaN itself.
     
  35. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    11,321
    Not going to argue about your opinio. If you're happy to link them and post any oddity you find here then that that's totally up to you and perfectly fine by me. I can only give you my opinion about them not being related, it's obviously up to you to decide what to do with it. :)