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

Ogre2 Implementation for Lidar Visual #116

Merged
merged 9 commits into from
Aug 5, 2020
Merged

Conversation

mihirk284
Copy link
Contributor

Added the Ogre2 implementation for Lidar Visual
Pls see attached screenshots

image
image

@iche033
@ahcorde

Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
@mihirk284 mihirk284 requested a review from iche033 as a code owner July 22, 2020 07:26
Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
@ahcorde ahcorde self-requested a review July 22, 2020 08:33
@ahcorde ahcorde added enhancement New feature or request 🔮 dome Ignition Dome labels Jul 22, 2020
@@ -219,7 +219,7 @@ LidarVisualPtr createLidar(ScenePtr _scene)
// LVT_RAY_LINES -> Lines along the lidar sensor to the obstacle
// LVT_TRIANGLE_STRIPS -> Coloured triangle strips denoting hitting and
// non-hitting parts of the scan
lidar->SetType(LidarVisualType::LVT_RAY_LINES);
lidar->SetType(LidarVisualType::LVT_TRIANGLE_STRIPS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this as LVT_TRIANGLE_STRIPS was the standard visual before ( before we added types, and the only visual in Gazebo Classic), in order to retain the same in the example.

@ahcorde
Copy link
Contributor

ahcorde commented Jul 22, 2020

I tried to add another render engine here

https://github.com/ignitionrobotics/ign-rendering/blob/e22fd80e25aa89454a2be1d69891c83c7900f4c0/examples/lidar_visual/Main.cc#L279

  engineNames.push_back("ogre2");

To be able to change the render engine with Tab, but I'm getting the following error

lidar_visual: /build/ogre-1.9-kiU5_5/ogre-1.9-1.9.0+dfsg1/OgreMain/include/OgreSingleton.h:80: Ogre::Singleton<T>::Singleton() [with T = Ogre::LogManager]: Assertion `!msSingleton' failed.

I know this issue is unrelated but do you mind to have a look ?

@mihirk284
Copy link
Contributor Author

mihirk284 commented Jul 22, 2020

@ahcorde Regarding the tests, I think this is a known test failure which is happening on Windows with other tests as well. I'm not sure why this is failing on Windows

Even for the previous PRs, the same failure message was seen on Windows

Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well for me.

One minor note: I see this warning msg printed when running the example:

[Wrn] [Ogre2GpuRays.cc:280] Only one vertical ray but vertical min. and max. angle are not equal. Min. angle is used.

ogre2/include/ignition/rendering/ogre2/Ogre2LidarVisual.hh Outdated Show resolved Hide resolved
@iche033
Copy link
Contributor

iche033 commented Jul 22, 2020

lidar_visual: /build/ogre-1.9-kiU5_5/ogre-1.9-1.9.0+dfsg1/OgreMain/include/OgreSingleton.h:80: Ogre::Singleton::Singleton() [with T = Ogre::LogManager]: Assertion `!msSingleton' failed.

Regarding this error when loading ogre2 along side of ogre, I looked into this some time ago and found that it's likely due to symbol collision between the ogre and ogre2 libraries. When ogre1.x is loaded, a few singletons were created, and when switching to ogre 2.x, it tries to create more singletons that collide with the ones that are already in memory. I think the would either need to 1) namespace the ogre singletons (not sure how to do this without editing ogre itself or 2) completely unload ogre 1.x plugin before switching to ogre 2.x at runtime).

I haven't figured out a solution to this issue yet so any help / suggestions welcome.

Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
@Sarath18
Copy link

Hey @mihirk284. I was integrating LidarVisual to ign-rviz and found that ClearPoints API doesn't seem to clear the visual that is already been visualized, rather it just clears the internal state and there is no direct API to clear the visual. I went through the ogre LidarVisual code and found a method ClearVisualData but it is not exposed to the user. It will be great if we have a method to clear the visuals.

Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
examples/lidar_visual/GlutWindow.cc Outdated Show resolved Hide resolved
@iche033
Copy link
Contributor

iche033 commented Jul 29, 2020

can you fix DCO? instructions in Details next to the DCO check.

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
@iche033
Copy link
Contributor

iche033 commented Aug 3, 2020

Regarding the ClearVisual and ClearPoints functions, I think it could be confusing for the user of the API of what the differences are. As a user, when I clear or update points, I expect the visual to be updated as well otherwise we end up with inconsistent state between the data and the visualization. How about for the public API of the LidarVisual interface, we offer just the functionClearPoints and internally we clear both the points and the visualization?

Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
Signed-off-by: Mihir Kulkarni <mihirk284@gmail.com>
@chapulina chapulina added this to Inbox in Core development via automation Aug 4, 2020
@chapulina chapulina moved this from Inbox to In review in Core development Aug 4, 2020
@chapulina chapulina linked an issue Aug 4, 2020 that may be closed by this pull request
@chapulina chapulina mentioned this pull request Aug 4, 2020
@iche033 iche033 dismissed ahcorde’s stale review August 5, 2020 03:53

comment addressed

@iche033 iche033 merged commit bcf46db into gazebosim:master Aug 5, 2020
Core development automation moved this from In review to Done Aug 5, 2020
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create lidar visualization
4 participants