scale ambient by 0.5 for collada and other assimp-loaded resources

[mikeferguson]

I've been bringing up a new robot in RVIZ/Gazebo using Collada meshes and URDF and have found that it was nearly impossible to get the meshes to look remotely similar within Gazebo and RVIZ. If the robot looked OK in Gazebo it was washed out in RVIZ, if it was OK in RVIZ it was too dark in Gazebo.

Searching the RVIZ codebase for "setAmbient" shows that almost always the r,g,b values have been scaled by 0.5 (not sure why). As soon as I add this commit, I no longer get washed out collada files (Tested with both my new robot and the PR2). This commit does not change anything for users specifying pure STL files or OGRE mesh files, only Collada users.

This is a potential solution for #515

[wjwwood]

@mikeferguson does it have anything to do with this change? #837

That is merged into indigo-devel but not released yet.

[mikeferguson]

@wjwwood #837 plays no part in this. The problem this addresses exists both in indigo-devel as well as the latest released debians, and has been around for quite some time and exists even when using non-zero ambient values.

[adolfo-rt]

Wow, halving the rgb coordinates definitely explains the darker rendering produced by rviz. So are meshes rendered qualitatively similar now (at least color-wise) between rviz and Gazebo?.

[mikeferguson]

So, in the case of collada meshes with ambient = 1 1 1 1 and emission = 0 0 0 0 (as in the case of the PR2), this makes them render almost as nicely as they do in gazebo. It certainly makes the colors correct (my white robot is actually a nice white in both rviz and gazebo, and you can actually make out the robot's shape).

[wjwwood]

I believe you that it helps, but I just don't understand the change. It seems pretty arbitrary to adjust the ambient values in a collada file by multiplying them by 0.5. #837 may not be related, but if I were to cherry-pick this commit without the one in #837 it would certainly behave differently for collada files which have 0 for any of rgb.

Searching the RVIZ codebase for "setAmbient" shows that almost always the r,g,b values have been scaled by 0.5 (not sure why).

I guess there is a good reason for this, but I'd like to know why. Maybe @hersh or @dgossow have some historical perspective on it.

Also, because we apparently "cribbed" (wtf does that even mean) this from Gazebo:

https://github.com/mikeferguson/rviz/blob/ambient_fix/src/rviz/mesh_loader.cpp#L430

maybe @nkoenig has some idea why the ambients are scaled by 0.5?

[wjwwood]

@mikeferguson for historical reasons, could you please post before and after images of a box or something which shows the difference this change makes?

[mikeferguson]

@wjwwood -- I will post some images tomorrow -- I was rushing to submit the PR last night before leaving the office.

As for the gazebo reference -- from what I can tell, this code was old gazebo code in ~2010 or so when rviz was first written. This file appears to have not been updated much over time, and the lines in question have no edits since inception. Gazebo appears to have rewritten their mesh loading significantly since then, and I could find no overlap between this code and anything in trunk of gazebo.

[wjwwood]

@mikeferguson Ok, thanks.

[wjwwood]

Searching the RVIZ codebase for "setAmbient" shows that almost always the r,g,b values have been scaled by 0.5 (not sure why).

For posterity, I was curious about this, and currently there a many instances of this, in several forms:

• rviz/src/rviz/default_plugin/markers/triangle_list_marker.cpp

  Line 194 in fc39173

  material_->getTechnique(0)->setAmbient( r/2,g/2,b/2 );

• rviz/src/rviz/robot/robot_link.cpp

  Line 461 in 8a010bb

  mat->getTechnique(0)->setAmbient(col.r * 0.5, col.g * 0.5, col.b * 0.5);

• rviz/src/rviz/robot/robot_link.cpp

  Line 906 in 8a010bb

  color_material_->getTechnique(0)->setAmbient( 0.5 * color );

• rviz/src/rviz/visualization_manager.cpp

  Line 273 in 38d6d14

  mat->setAmbient(color * 0.5f);

• rviz/src/rviz/ogre_helpers/shape.cpp

  Line 127 in 8a010bb

  material_->getTechnique(0)->setAmbient( c * 0.5 );

• rviz/src/rviz/ogre_helpers/line.cpp

  Line 114 in 923d86c

  manual_object_material_->getTechnique(0)->setAmbient( c * 0.5 );

• rviz/src/rviz/default_plugin/markers/mesh_resource_marker.cpp

  Line 230 in 8a010bb

  (*it)->setAmbient(0.5 * r, 0.5 * g, 0.5 * b);

Notably this one is not like that:

• rviz/src/rviz/default_plugin/interactive_markers/interactive_marker_control.cpp

  Line 1007 in 8a010bb

  (*it)->setAmbient(a,a,a);

But it's the only instance I could find that seems to set the ambient without scaling it (other than the one being modified in this pull request).

[wjwwood]

@mikeferguson (or anyone else), I'd like to ask again for either before and after images or a test mesh so that I can reproduce the before and after myself. I know you're busy, but I'd like to get this merged in for the next release which is imminent for more soak testing time, but I don't feel confident until I can try it out and observe it myself (or at least see what you're talking about in images).

Thanks!

[mikeferguson]

@wjwwood here you go. This is a cube, in collada, with ambient = 1 1 1 1. I also tested a mesh.xml with ambient and got similar results. In addition, I included an image of the mesh in Gazebo 2.0 (which is my main need for this fix, to get (more) consistent results between Gazebo and RVIZ)

[wjwwood]

Awesome, thanks for getting those for me.

I guess the remaining inconsistency between rviz w/ this patch and gazebo is the placement of the lights?

[wjwwood]

I'm gonna merge this now, but one thought is that rather than changing the mat for every mesh we might be able to achieve the same effect with global lighting. However, that's out of my expertise, so I'll punt that for later.

[mikeferguson]

@wjwwood I actually tried the lighting -- it can get closer to Gazebo, but it still requires this fix to avoid washing out. My guess is that Gazebo does this divide by 2 as well inside (although I couldn't figure out where that exact code is, I went down several layers in their codebase but eventually it looks like they rely on an external library for initial collada loading).

I am planning to eventually put a pull request together that adds a menu to select either the point light (as we have right now) or a global light (like gazebo has) or both. I figured out the lighting aspects, but not the menuing thus far.

[wjwwood]

Cool, let me know if I can help.

[adolfo-rt]

I guess the remaining inconsistency between rviz w/ this patch and gazebo is the placement of the lights?

In the general case Gazebo and rviz will render differently because of lighting. This is because Gazebo allows you to control the number, type and location of light sources, whereas rviz only has a fixed directional light source (I think, I haven't checked). What could maybe be done is make the direction of the rviz light source with that of the typical Gazebo empty world, which I believe is overhead.

[hersh]

Lighting has largely been ignored in the development of RViz, at least during my tenure with it. I think it might be a great addition to add a configurable lighting system. It might already be possible with rviz plugins, since you can use all of Ogre from rviz::Display plugins.

Shadows would also be really useful at times. I'm sure they would be a performance hit in some cases, so they should be optional, but when you're moving interactive markers in 3D, shadows could give a nice extra bit of feedback about where your object is. Because of the extra rendering step required for shadows, I would expect that to be a change within rviz itself and not just a plugin.