Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Light visuals are enable by default #1086

Open
nkoenig opened this issue Oct 5, 2021 · 10 comments
Open

Light visuals are enable by default #1086

nkoenig opened this issue Oct 5, 2021 · 10 comments
Labels
bug Something isn't working GUI Gazebo's graphical interface (not pure Ignition GUI) help wanted We accept pull requests!

Comments

@nkoenig
Copy link
Contributor

nkoenig commented Oct 5, 2021

Environment

  • OS Version: Ubuntu 18.04
  • Fortress Source

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.

@nkoenig nkoenig added the bug Something isn't working label Oct 5, 2021
@chapulina chapulina added the GUI Gazebo's graphical interface (not pure Ignition GUI) label Oct 5, 2021
@chapulina
Copy link
Contributor

+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
Copy link
Contributor Author

nkoenig commented Oct 5, 2021

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
Copy link
Contributor

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
Copy link
Contributor

azeey commented Oct 11, 2021

+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
Copy link
Contributor

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
Copy link
Contributor

chapulina commented Oct 13, 2021

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/

@chapulina chapulina added the help wanted We accept pull requests! label Nov 29, 2021
@ahcorde
Copy link
Contributor

ahcorde commented Mar 21, 2022

with these two PRs we can consider this as done

@chapulina
Copy link
Contributor

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
Copy link
Contributor

azeey commented Mar 21, 2022

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

@sea-bass
Copy link

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 #1948 for something I just found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working GUI Gazebo's graphical interface (not pure Ignition GUI) help wanted We accept pull requests!
Projects
None yet
Development

No branches or pull requests

5 participants