Add Ogre2Heightmap functionality #386

darksylinc · 2021-08-24T16:19:35Z

🎉 New feature

Closes #187

Summary

This PR adds Terra samples' code and integrates it into Gazebo to provide Heightmap functionality.

It's yet missing the ability for terrain to cast shadows coming from spot and point lights (only directional lights cast terrain shadows onto everything). This feature will come later. The code is ready but integration with ign-rendering is not and due to time constraints (Fortress feature freeze) it's being submitted for review.

Note: All code under ogre2/src/terrain folder should be considered external code.

Test it

There are not many ways to test heightmap in Gazebo AFAIK. Build ign-rendering normally then:

Build ign-rendering/examples/heightmap
Run it

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

Signed-off-by: ahcorde <[email protected]>

Signed-off-by: Michael Carroll <[email protected]>

Co-authored-by: Michael Carroll <[email protected]> Signed-off-by: ahcorde <[email protected]>

Signed-off-by: Ian Chen <[email protected]>

Signed-off-by: Michael Carroll <[email protected]>

Signed-off-by: Ian Chen <[email protected]>

…ign-rendering into ahcorde/update/ogre2.2

Signed-off-by: Ian Chen <[email protected]>

Signed-off-by: Michael Carroll <[email protected]>

Signed-off-by: ahcorde <[email protected]>

…obotics/ign-rendering into ahcorde/update/ogre2.2

Signed-off-by: Michael Carroll <[email protected]>

Signed-off-by: ahcorde <[email protected]>

…ardware gamma to fix dark sky color Signed-off-by: Ian Chen <[email protected]>

* Local updates for Ogre2.2 against main branch Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Ian Chen <[email protected]>

Signed-off-by: ahcorde <[email protected]>

Signed-off-by: Ian Chen <[email protected]>

Signed-off-by: Michael Carroll <[email protected]>

darksylinc · 2021-09-09T01:18:37Z

I merged with main and fixed the merge conflicts. However this now causes DCO check to fail

I've also updated to latest Hlms files to fix the compiler error when using shadow mapping; these files also add the missing support for HlmsPbs::setShadowReceiversInPixelShader (which fixes errors when too many shadow mapped lights are on the scene)

mjcarroll · 2021-09-09T13:05:10Z

However this now causes DCO check to fail

It can be forced to pass in this case. It gets a little picky when merge commits and rebases happen.

iche033 · 2021-09-11T02:38:49Z

I pulled the latest changes and rebuilt the workspace and no more crashes!

I still get the same output as the one in: #386 (comment), and I verified that I have the fix. Let me know if you're able to reproduce it.

Signed-off-by: Matias N. Goldberg <[email protected]>

Update Terra to latest from upstream Signed-off-by: Matias N. Goldberg <[email protected]>

darksylinc · 2021-09-11T19:04:22Z

Thanks for the patch to test spot shadows. It was a Z-up issue.

It's now working!

Terra's workspace listener was being executed before the camera is rotated for cubemap rendering Signed-off-by: Matias N. Goldberg <[email protected]>

darksylinc · 2021-09-11T20:40:17Z

Thanks to your patch I was able to notice point light shadows were not working properly due to a silly bug.

Fixed:

BTW you might notice those "bright spots" at the bottom of the shadowed side of a mountain or cliffs:

These can be alleviated by messing with shadow bias and/or toggling Ogre::Root::getSingleton().getHlmsManager()->setShadowMappingUseBackFaces(false); however it will always be problematic (each fix causes another problem, usually bigger) until Ogre 2.3

That issue is fixed in Ogre 2.3 (it cannot be backported) as we implemented Normal-offset bias which results in much higher quality shadow mapping.

iche033

lights + shadows are working well for me now! Can you add the point light to the demo?

To get green builds, we'll need to suppress compile warnings coming from the terra directory. I think this works:

diff --git a/ogre2/src/terrain/Terra/CMakeLists.txt b/ogre2/src/terrain/Terra/CMakeLists.txt
index 7dbf9683..531a7763 100644
--- a/ogre2/src/terrain/Terra/CMakeLists.txt
+++ b/ogre2/src/terrain/Terra/CMakeLists.txt
@@ -12,6 +12,13 @@ if(IGN_ADD_fPIC_TO_LIBRARIES AND NOT _ign_add_library_INTERFACE)
   target_compile_options(${PROJECT_NAME} PRIVATE -fPIC)
 endif()
 
+target_compile_options(${PROJECT_NAME} PRIVATE
+    -Wno-unused-parameter
+    -Wno-float-equal
+    -Wno-sign-compare
+    -Wno-strict-aliasing)
+add_definitions(-DOGRE_IGNORE_UNKNOWN_DEBUG)

Can you address the remaining comments from @ahcorde?

iche033 · 2021-09-13T22:31:27Z

ogre2/include/ignition/rendering/ogre2/Ogre2Heightmap.hh

+      : public BaseHeightmap<Ogre2Geometry>
+    {
+      /// \brief Constructor
+      public: explicit Ogre2Heightmap(const HeightmapDescriptor &_desc);


something like Parameters that describe how a heightmap should be loaded. should suffice.

this is needed to fix: https://github.com/ignitionrobotics/ign-rendering/pull/386/checks?check_run_id=3576496054

iche033 · 2021-09-13T22:37:05Z

ogre2/include/ignition/rendering/ogre2/Ogre2Heightmap.hh

+      public: virtual void PreRender() override;
+
+      /// \brief Returns NULL, heightmaps don't have movable objects.
+      /// \return Null pointer.


This is true in ogre 1.x but looking at the Ogre2Heightmap::OgreObject() implementation, it returns an Ogre::Terra object. Can you update the comment here @darksylinc ?

iche033 · 2021-09-13T22:38:31Z

ogre2/include/ignition/rendering/ogre2/Ogre2Heightmap.hh

+
+      /// \brief Returns NULL, heightmap materials don't inherit from
+      /// MaterialPtr.
+      /// \return Null pointer.


yeah, looks like still always returns null. Textures are specified through HeightmapDescriptor

iche033 · 2021-09-13T22:49:48Z

ogre2/src/media/2.0/scripts/materials/Terra/GLSL/GpuNormalMapper_ps.glsl

+	vNormal += cross( (vNN - v00), (vN0 - v00) );
+	vNormal += cross( (vN0 - v00), (v01 - v00) );
+
+//	vNormal += cross( (v01 - v00), (v11 - v00) );


I think this file came from ogre-next samples. We can leave the comment here to minimize diff with upstream version.

iche033 · 2021-09-13T22:54:40Z

BTW you might notice those "bright spots" at the bottom of the shadowed side of a mountain or cliffs:

ok that's fine. We'll likely play with different bias values if needed, which is what we've done in the past but it was always hard to find something that works for all cases.

Silence warnings from external code to pass build checks Signed-off-by: Matias N. Goldberg <[email protected]>

Signed-off-by: Ian Chen <[email protected]>

iche033 · 2021-09-16T20:18:39Z

I made some changes in the iche033/ogre22-terra branch to fix windows and homebrew CI builds:
https://github.com/ignitionrobotics/ign-rendering/compare/260255b53bb98ca4a68304e911ee821c2e509692..bb0119bb396ac8a432fa4b0d18ef8046ddf0c962

If that looks good to you, can you merge with that branch? That should make CI green.

darksylinc · 2021-09-18T20:14:16Z

I made some changes in the iche033/ogre22-terra branch to fix windows and homebrew CI builds:
https://github.com/ignitionrobotics/ign-rendering/compare/260255b53bb98ca4a68304e911ee821c2e509692..bb0119bb396ac8a432fa4b0d18ef8046ddf0c962

If that looks good to you, can you merge with that branch? That should make CI green.

All good. Changes pushed.

iche033

CI looks good.

I'll address a couple of small style comments in a separate PR.

Need to get this in for pre-release

will address remaining comments in a separate PR.

ahcorde and others added 30 commits March 23, 2021 17:57

Ogre2.2

025f72a

Signed-off-by: ahcorde <[email protected]>

Added media files

1cff215

Signed-off-by: ahcorde <[email protected]>

Added Headless mode

e5979b8

Signed-off-by: ahcorde <[email protected]>

Added feedback

fc89bef

Signed-off-by: ahcorde <[email protected]>

Added feedback

c5c077c

Signed-off-by: ahcorde <[email protected]>

Add OGRE2.2 to dependencies

8033623

Signed-off-by: Michael Carroll <[email protected]>

Fixed GPUray and depthCamera test

a82820c

Co-authored-by: Michael Carroll <[email protected]> Signed-off-by: ahcorde <[email protected]>

Merge branch 'main' into ahcorde/update/ogre2.2

b5c639e

cast based on pf

b664ade

Signed-off-by: Ian Chen <[email protected]>

Fix cleanup with packaged debs

d6808b8

Signed-off-by: Michael Carroll <[email protected]>

fix copy raw data

e940e9a

Signed-off-by: Ian Chen <[email protected]>

Merge branch 'ahcorde/update/ogre2.2' of github.com:ignitionrobotics/…

92e8fbf

…ign-rendering into ahcorde/update/ogre2.2

fix material test

11cd876

Signed-off-by: Ian Chen <[email protected]>

Lint and compiler warning

88cfd40

Signed-off-by: Michael Carroll <[email protected]>

Trim whitespace

db609f3

Signed-off-by: Michael Carroll <[email protected]>

Apply gamma correction to sky

bc12f40

Signed-off-by: ahcorde <[email protected]>

Merge branch 'ahcorde/update/ogre2.2' of https://github.com/ignitionr…

fcf51b1

…obotics/ign-rendering into ahcorde/update/ogre2.2

Test fixes to Ogre2.2 branch (gazebosim#279)

797675b

Signed-off-by: Michael Carroll <[email protected]>

Merge branch 'main' into ahcorde/update/ogre2.2

7046608

Revert gamma correction in the sky

d55ee47

Signed-off-by: ahcorde <[email protected]>

Merge branch 'main' into ahcorde/update/ogre2.2

01aeec2

enable grayscale albedo map to fix red color highlights and disable h…

62bb629

…ardware gamma to fix dark sky color Signed-off-by: Ian Chen <[email protected]>

Merge branch 'main' into ahcorde/update/ogre2.2

032ce0f

Local updates for Ogre2.2 against main branch (gazebosim#317)

adfdb05

* Local updates for Ogre2.2 against main branch Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Ian Chen <[email protected]>

Fix merge

24fb7df

Signed-off-by: ahcorde <[email protected]>

Merge branch 'main' into ahcorde/update/ogre2.2

72f23ab

fix shadows in ogre 2.2

96b2eb5

Signed-off-by: Ian Chen <[email protected]>

Merge branch 'main' into ahcorde/update/ogre2.2

5e2f7b5

Merge branch 'main' into ahcorde/update/ogre2.2

21b5993

Lint

c2bbbe9

Signed-off-by: Michael Carroll <[email protected]>

Merge branch 'main' into matias-ogre22-terra

93d88e1

darksylinc added 2 commits September 11, 2021 13:19

Fix compiler warning

24d957a

Signed-off-by: Matias N. Goldberg <[email protected]>

Fix Terra shadows on Spot and Point lights

27ea80e

Update Terra to latest from upstream Signed-off-by: Matias N. Goldberg <[email protected]>

Fix Terra's point light shadows

383d80e

Terra's workspace listener was being executed before the camera is rotated for cubemap rendering Signed-off-by: Matias N. Goldberg <[email protected]>

iche033 reviewed Sep 13, 2021

View reviewed changes

darksylinc and others added 6 commits September 15, 2021 12:26

Fix documentation

6715cbb

Silence warnings from external code to pass build checks Signed-off-by: Matias N. Goldberg <[email protected]>

Merge branch 'main' into matias-ogre22-terra

260255b

fixing CI

6894fd3

Signed-off-by: Ian Chen <[email protected]>

add include path

19d922c

Signed-off-by: Ian Chen <[email protected]>

windows warning

c9d5a17

Signed-off-by: Ian Chen <[email protected]>

fix

bb0119b

Signed-off-by: Ian Chen <[email protected]>

iche033 approved these changes Sep 20, 2021

View reviewed changes

iche033 merged commit b2da7e1 into gazebosim:main Sep 20, 2021

This was referenced Sep 20, 2021

[Testing] Fix Ogre2 Terra CI #412

Closed

Address remaining Ogre2 heightmap comments #416

Merged

chapulina mentioned this pull request Nov 12, 2021

Support heightmaps on Ogre 2 #187

Closed

chapulina mentioned this pull request Mar 30, 2022

Flip heightmap's Y position on Ogre 2 and add sanity checks for NaN #585

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Ogre2Heightmap functionality #386

Add Ogre2Heightmap functionality #386

darksylinc commented Aug 24, 2021 •

edited

Loading

darksylinc commented Sep 9, 2021

mjcarroll commented Sep 9, 2021

iche033 commented Sep 11, 2021

darksylinc commented Sep 11, 2021

darksylinc commented Sep 11, 2021 •

edited

Loading

iche033 left a comment

iche033 Sep 13, 2021

iche033 Sep 13, 2021

iche033 Sep 13, 2021

iche033 Sep 13, 2021

iche033 commented Sep 13, 2021

iche033 commented Sep 16, 2021

darksylinc commented Sep 18, 2021

iche033 left a comment

Add Ogre2Heightmap functionality #386

Add Ogre2Heightmap functionality #386

Conversation

darksylinc commented Aug 24, 2021 • edited Loading

🎉 New feature

Summary

Test it

Checklist

darksylinc commented Sep 9, 2021

mjcarroll commented Sep 9, 2021

iche033 commented Sep 11, 2021

darksylinc commented Sep 11, 2021

darksylinc commented Sep 11, 2021 • edited Loading

iche033 left a comment

Choose a reason for hiding this comment

iche033 Sep 13, 2021

Choose a reason for hiding this comment

iche033 Sep 13, 2021

Choose a reason for hiding this comment

iche033 Sep 13, 2021

Choose a reason for hiding this comment

iche033 Sep 13, 2021

Choose a reason for hiding this comment

iche033 commented Sep 13, 2021

iche033 commented Sep 16, 2021

darksylinc commented Sep 18, 2021

iche033 left a comment

Choose a reason for hiding this comment

darksylinc commented Aug 24, 2021 •

edited

Loading

darksylinc commented Sep 11, 2021 •

edited

Loading