From 817d440769dc779426cd18fcad3ab5b25e73f9e9 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Fri, 4 Mar 2022 18:05:38 -0800 Subject: [PATCH 1/4] Flip heightmap's Y position on Ogre 2 and add sanity checks for NaN Signed-off-by: Louise Poubel --- examples/heightmap/Main.cc | 2 +- ogre/src/OgreHeightmap.cc | 9 ++++++++- ogre2/src/Ogre2Heightmap.cc | 31 +++++++++++++++++++++++++------ 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/examples/heightmap/Main.cc b/examples/heightmap/Main.cc index ac7306d3d..875a279ae 100644 --- a/examples/heightmap/Main.cc +++ b/examples/heightmap/Main.cc @@ -152,7 +152,7 @@ void buildScene(ScenePtr _scene) textureC2.SetDiffuse("../media/dirt_diffusespecular.png"); textureC2.SetNormal("../media/flat_normal.png"); desc2.AddTexture(textureC2); - desc2.SetPosition({30, 0, 0}); + desc2.SetPosition({30, 10, 0}); auto heightmapGeom2 = _scene->CreateHeightmap(desc2); auto vis2 = _scene->CreateVisual(); diff --git a/ogre/src/OgreHeightmap.cc b/ogre/src/OgreHeightmap.cc index d43f4da3f..4b06c0851 100644 --- a/ogre/src/OgreHeightmap.cc +++ b/ogre/src/OgreHeightmap.cc @@ -491,7 +491,14 @@ void OgreHeightmap::Init() for (unsigned int x = 0; x < vertSize; ++x) { int index = (vertSize - y - 1) * vertSize + x; - this->dataPtr->heights.push_back(lookup[index] - minElevation); + + // Sanity check in case we get NaNs from ign-common, this prevents a crash + // in Ogre + auto value = lookup[index]; + if (!std::isfinite(value)) + value = minElevation; + + this->dataPtr->heights.push_back(value - minElevation); } } diff --git a/ogre2/src/Ogre2Heightmap.cc b/ogre2/src/Ogre2Heightmap.cc index 04273b12b..ccd026c79 100644 --- a/ogre2/src/Ogre2Heightmap.cc +++ b/ogre2/src/Ogre2Heightmap.cc @@ -158,17 +158,35 @@ void Ogre2Heightmap::Init() // Obtain min and max elevation and bring everything to range [0; 1] // Terra should support non-normalized ranges but there are a couple // bugs preventing that, so it's just easier to normalize the data - float minElevation = 0.0; - float maxElevation = 0.0; + double minElevation = this->descriptor.Data()->MinElevation(); + double maxElevation = this->descriptor.Data()->MaxElevation(); + + // Sanity check + if (minElevation >= maxElevation) + { + ignerr << "Internal error: min elevation [" << minElevation + << "] >= max elevation [" << maxElevation << "]" << std::endl; + return; + } for (unsigned int y = 0; y < newWidth; ++y) { for (unsigned int x = 0; x < newWidth; ++x) { const size_t index = y * srcWidth + x; - const float heightVal = lookup[index]; - minElevation = std::min(minElevation, heightVal); - maxElevation = std::max(maxElevation, heightVal); + float heightVal = lookup[index]; + + // Sanity check in case we get NaNs from ign-common, this prevents a crash + // in Ogre + if (!std::isfinite(heightVal)) + heightVal = minElevation; + + if (heightVal < minElevation || heightVal > maxElevation) + { + ignerr << "Internal error: height [" << heightVal + << "] is out of bounds [" << minElevation << " / " + << maxElevation << "]" << std::endl; + } this->dataPtr->heights.push_back(heightVal); } } @@ -208,9 +226,10 @@ void Ogre2Heightmap::Init() const math::Vector3d newSize = this->descriptor.Size() * math::Vector3d(1.0, 1.0, heightDiff); + // The position's Y sign ends up flipped math::Vector3d center( this->descriptor.Position().X(), - this->descriptor.Position().Y(), + -this->descriptor.Position().Y(), this->descriptor.Position().Z() + newSize.Z() * 0.5 + minElevation); Ogre::Root *ogreRoot = Ogre2RenderEngine::Instance()->OgreRoot(); From a8be6564694e9da3708aa120dda4acde5c53daed Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Tue, 5 Apr 2022 18:09:14 -0700 Subject: [PATCH 2/4] migration guide Signed-off-by: Louise Poubel --- Migration.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Migration.md b/Migration.md index d65899954..919de259c 100644 --- a/Migration.md +++ b/Migration.md @@ -5,6 +5,12 @@ Deprecated code produces compile-time warnings. These warning serve as notification to users that their code should be upgraded. The next major release will remove the deprecated code. +## Ignition Rendering 6.0.1 to 6.1.0 + +### Modifications + +1. Ogre 2 heightmaps: the Y position sign was flipped + ## Ignition Rendering 5.x to 6.x ### Modifications From bb1504170891aea74a4877cd78b93d8e54516891 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Wed, 6 Apr 2022 08:36:59 -0700 Subject: [PATCH 3/4] Revert min/max changes on Fortress Signed-off-by: Louise Poubel --- ogre2/src/Ogre2Heightmap.cc | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/ogre2/src/Ogre2Heightmap.cc b/ogre2/src/Ogre2Heightmap.cc index ccd026c79..06ad21fcf 100644 --- a/ogre2/src/Ogre2Heightmap.cc +++ b/ogre2/src/Ogre2Heightmap.cc @@ -158,16 +158,8 @@ void Ogre2Heightmap::Init() // Obtain min and max elevation and bring everything to range [0; 1] // Terra should support non-normalized ranges but there are a couple // bugs preventing that, so it's just easier to normalize the data - double minElevation = this->descriptor.Data()->MinElevation(); - double maxElevation = this->descriptor.Data()->MaxElevation(); - - // Sanity check - if (minElevation >= maxElevation) - { - ignerr << "Internal error: min elevation [" << minElevation - << "] >= max elevation [" << maxElevation << "]" << std::endl; - return; - } + float minElevation = 0.0; + float maxElevation = 0.0; for (unsigned int y = 0; y < newWidth; ++y) { @@ -181,12 +173,8 @@ void Ogre2Heightmap::Init() if (!std::isfinite(heightVal)) heightVal = minElevation; - if (heightVal < minElevation || heightVal > maxElevation) - { - ignerr << "Internal error: height [" << heightVal - << "] is out of bounds [" << minElevation << " / " - << maxElevation << "]" << std::endl; - } + minElevation = std::min(minElevation, heightVal); + maxElevation = std::max(maxElevation, heightVal); this->dataPtr->heights.push_back(heightVal); } } From 6a3465e0ff8163c55da800b2442d6334b659f473 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Wed, 6 Apr 2022 08:41:16 -0700 Subject: [PATCH 4/4] Update migration guide Signed-off-by: Louise Poubel --- Changelog.md | 2 +- Migration.md | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Changelog.md b/Changelog.md index 0d1f7d048..73c15774b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,7 +10,7 @@ 1. Increase particle emitter quota * [Pull request #562](https://github.com/ignitionrobotics/ign-rendering/pull/562) -1.Make sure shader param exists before setting its value +1. Make sure shader param exists before setting its value * [Pull request #558](https://github.com/ignitionrobotics/ign-rendering/pull/558) 1. Backport wave changes diff --git a/Migration.md b/Migration.md index b07754b02..9855e61c9 100644 --- a/Migration.md +++ b/Migration.md @@ -5,12 +5,18 @@ Deprecated code produces compile-time warnings. These warning serve as notification to users that their code should be upgraded. The next major release will remove the deprecated code. -## Ignition Rendering 6.0.1 to 6.1.0 +## Ignition Rendering 6.2.1 to 6.X ### Modifications 1. Ogre 2 heightmaps: the Y position sign was flipped +1. `Scene::SetTime` is often unset. Ignition's `Ogre2` now defaults to 60hz otherwise rendering won't advance forward. + + Mostly affects Particles. + + Also may affect gaussian postprocessing and other filters dependant on time. + + Previous behavior was using real time instead of simulation time, which is wrong. + + See https://github.com/ignitionrobotics/ign-rendering/issues/556 for details. + ## Ignition Rendering 5.x to 6.x ### Modifications @@ -46,12 +52,6 @@ release will remove the deprecated code. 1. **depth_camera_fs.glsl** and **depth_camera_final_fs.glsl** + Far clipping changed from clipping by depth to clipping by range, i.e. distance to point, so that the data generated will never exceed the specified max range of the camera. -1. `Scene::SetTime` is often unset. Ignition's `Ogre2` now defaults to 60hz otherwise rendering won't advance forward. - + Mostly affects Particles. - + Also may affect gaussian postprocessing and other filters dependant on time. - + Previous behavior was using real time instead of simulation time, which is wrong. - + See https://github.com/ignitionrobotics/ign-rendering/issues/556 for details. - ## Ignition Rendering 4.0 to 4.1 ## ABI break