Fixed crash when collision size is zero

[ahcorde]

This PR fixed an issue when the collision size of a link is 0 0 0.

Signed-off-by: ahcorde ahcorde@gmail.com

[mabelzhang]

The fix works for me. Looks like the CI doesn't like isnan. I think std::isnan would fix it.

Steps to reproduce the original GUI crash:
Create an SDF that has a link with collision sized 0 0 0, e.g.:

...
      <link name="link">
        <collision name="collision">
          <geometry>
            <box>
              <size>0 0 0</size>
            </box>
          </geometry>
...

Run Gazebo with GUI,

$ gazebo file.world

Go to menu View > Collisions, and GUI will crash with this error

gzclient: /build/ogre-1.9-B6QkmW/ogre-1.9-1.9.0+dfsg1/OgreMain/src/OgreNode.cpp:630: virtual void Ogre::Node::setScale(const Ogre::Vector3&): Assertion `!inScale.isNaN() && "Invalid vector supplied as parameter"' failed.

With this PR, the crash no longer happens.

[ahcorde]

Thanks for the feedback @mabelzhang. I solved the compilation error but there are several unrelated test failures. I also improved the error message.

I will backport and forwardport to gazebo 7 and 11 when this is ready to merge.

[mabelzhang]

Looks good. I was going to say approval on green CI, but the buildfarm status board CI page isn't loading for me, so I can't compare to tell if the tests normally failed... Are these known flaky tests?

PhysicsEngines/JointTest.SpringDamperTest
PluginTest.ModelExceptionConstructor
PluginTest.ModelExceptionInit
PluginTest.ModelExceptionLoad
PluginInterface.LoadParams
Issue1208Test.Reset

[mabelzhang]

Okay buildfarm is back up. I can see the Linux failures are not new, and Windows looks fine, Mac for some reason didn't run the tests?
@j-rivero Could you help confirm this? Do we need to rerun the Mac tests?

[j-rivero]

Okay buildfarm is back up. I can see the Linux failures are not new, and Windows looks fine, Mac for some reason didn't run the tests?

Ouch there was a cut in the connection of the Mac machine:

FATAL: command execution failed
java.nio.channels.ClosedChannelException
	at org.jenkinsci.remoting.protocol.NetworkLayer.onRecvClosed(NetworkLayer.java:154)
	at org.jenkinsci.remoting.protocol.impl.NIONetworkLayer.ready(NIONetworkLayer.java:192)
	at org.jenkinsci.remoting.protocol.IOHub$OnReady.run(IOHub.java:795)
	at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
	at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:59)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused: java.io.IOException: Backing channel 'JNLP4-connect connection from 70-35-50-58.static.wiline.com/70.35.50.58:59041' is disconnected.

I've rerun it: waiting in the queue

[scpeters]

I don't see any regressions caused by this PR, but it would be nice to have a test that reproduces this problem. I wasn't able to reproduce on my mac, so maybe it's based on a difference between the Ubuntu and homebrew ogre 1.9 packages.

[mabelzhang]

Approving as it's not breaking CI.

[ahcorde]

I don't have permissions to merge in the repository. Feel free to merge it

[chapulina]

I like @scpeters 's idea of adding a test, we should always try to add tests that are broken before a fix, and pass after the fix.

@ahcorde , do you think you could add a test similar to this one that exercises the bug being fixed? I think you could add a test world with a model that has 0 0 0 collision size and then try to get its Scale(), maybe that's enough to cause the crash?

https://github.com/osrf/gazebo/blob/7200818c3f1b905505d25c66d5ad004cd0003e88/gazebo/rendering/Visual_TEST.cc#L1547

I think it would be enough to add the test to #2769 and then we can merge the test forward to gazebo9. Let me know what you think!

[ahcorde]

Sure, I will add the test 👍

[mabelzhang]

Ah sorry I misunderstood and merged early. I thought Steve was talking about reproducing the error in Jenkins... :X