search

gazebosim / gz-sim

Open source robotics simulator. The latest version of Gazebo.
https://gazebosim.org
Apache License 2.0
714 stars 270 forks source link

Light visuals are enable by default #1086

Open nkoenig opened 3 years ago

nkoenig commented 3 years ago

Environment

Description

The green light visuals should not be visible by default. Similar to other diagnostic visuals, such as joints an inertia, the light visuals should be toggled on/off manually by the user via the right-mouse menu.

chapulina commented 3 years ago

+1 for having the ability of toggling the visibility through the GUI. It would also be nice to do it on the SDF, like how sensors have the <visualize> tag, we could add one for lights.

I'm not so sure about hiding them by default though. The user view is meant to help build and debug the simulation. Hiding the light visuals may confuse users who don't know how to find the lights. I believe other 3D software packages also display lights to the user in edit mode by default.

nkoenig commented 3 years ago

Is the light visualization logic in ign-gui or ign-gazebo now?

My reasons for off-by-default are:

  1. The light visuals are a debug tool, which doesn't seem like it should appear unless you need it. Like turning up verbosity.
  2. It's not entirely intuitive what the green lines are unless you are familiar with Ignition.
  3. We don't display other meta-visuals by default.
  4. Probably the most important is that people creating videos and screenshots will likely not turn off the light visuals. This will result in media and videos containing the green light visuals.
chapulina commented 3 years ago

Is the light visualization logic in ign-gui or ign-gazebo now?

It's here:

https://github.com/ignitionrobotics/ign-gazebo/blob/fc2a98ec5e1b149d773eeb6cae4a407bd82a81a2/src/rendering/SceneManager.cc#L1000

The light visuals are a debug tool, which doesn't seem like it should appear unless you need it. Like turning up verbosity.

I get the point. I think it would be good to be consistent though. Should the grid be invisible by default too? That's currently on by default as per the SDF spec.

Maybe it makes sense to make world light visuals on by default, like the grid, but link light visuals off by default, like joint, collision, etc?

It's not entirely intuitive what the green lines are unless you are familiar with Ignition.

I'd argue that Gazebo classic users are familiar with the lines, but I'm not the best person to judge.

We don't display other meta-visuals by default.

See the point above about the grid. The world origin is also displayed by default on Gazebo classic, but it hasn't been ported to Ignition yet.

people creating videos and screenshots will likely not turn off the light visuals

If they create them with a camera sensor, then the lights will not show. If they create them with the GUI camera, then I think we should offer them the tools to toggle the visualization. They also need to explicitly toggle the grid right now.

Another use case is when users insert a light from the GUI; I think it's good to show the light visuals by default then, no?

azeey commented 3 years ago

+1 for not displaying light visuals by default. From the perspective of creating videos and screenshots, the grid doesn't really get in the way unless you have a textured ground plane. The light visualizations however do get in the way and can be distracting. Maybe its because I'm more used to having the grid on the ground.

I get the point. I think it would be good to be consistent though. Should the grid be invisible by default too? That's currently on by default as per the SDF spec.

Consistency is good in general, but in this case I don't think we should have a binary approach where we either enable all debug visualizations by default or not. If we did that, like you said, we'd have to display link and joint frames, collisions, contacts, etc. Instead, we should make judgment calls as to what are good defaults and allow users to enable the ones that are disabled by default when they need to.

chapulina commented 3 years ago

These are all fair points, @azeey . I just think they're coming from a specific use case of creating videos and screenshots, which I don't think most users are concerned about. When you open a scene to edit on Blender, Unity, etc, the light has a visual representation to aid in scene construction. These other applications do have a very clear separation between "edit" and "run" views though, which is different from Gazebo's workflow. I just want us to keep the most common use case in mind.

chapulina commented 3 years ago

I found it interesting that a user is missing the light visuals on Citadel; and they thought it was related to sensors.

https://answers.gazebosim.org//question/27446/visible-sensor-ray-in-citadel/

ahcorde commented 2 years ago

with these two PRs we can consider this as done

chapulina commented 2 years ago

The question here is about whether we should flip the default behavior.

@azeey / @nkoenig , are you ok keeping the default behavior now that the visibility is configurable through SDF?

azeey commented 2 years ago

My preference would be to have them off by default similar to how debugging visuals for inertials are off by default.

sea-bass commented 1 year ago

I think a default of on from something you drag in the GUI is fine, but if the light was defined in an SDF file, it should follow not only the default in the specification, but the actual value specified in the file. See https://github.com/gazebosim/gz-sim/issues/1948 for something I just found.