From 1912f8f08ba227f69cb1d1acd65fd9e8b8819b50 Mon Sep 17 00:00:00 2001
From: Emma Broman <emma.broman@liu.se>
Date: Fri, 2 Jul 2021 08:11:37 +0200
Subject: [PATCH] Avoid non-supported ranges for exponential slider (#1672)

* Warn if setExponent is called with an invalid minmax range

* Disable bounding and interaction sphere exponent, to suppress warning
---
 .../properties/numericalproperty.inl          | 31 +++++++++++++++++++
 src/scene/scenegraphnode.cpp                  | 12 ++++---
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/include/openspace/properties/numericalproperty.inl b/include/openspace/properties/numericalproperty.inl
index 3f5696f17a..23adef27f5 100644
--- a/include/openspace/properties/numericalproperty.inl
+++ b/include/openspace/properties/numericalproperty.inl
@@ -23,9 +23,11 @@
  ****************************************************************************************/
 
 #include <openspace/util/json_helper.h>
+#include <ghoul/logging/logmanager.h>
 #include <ghoul/lua/ghoul_lua.h>
 #include <glm/ext/matrix_common.hpp>
 #include <cmath>
+#include <type_traits>
 
 namespace openspace::properties {
 
@@ -90,6 +92,35 @@ float NumericalProperty<T>::exponent() const {
 template <typename T>
 void NumericalProperty<T>::setExponent(float exponent) {
     ghoul_assert(std::abs(exponent) > 0.f, "Exponent for property input cannot be zero");
+
+    auto isValidRange = [](const T& minValue, const T& maxValue) {
+        if constexpr (ghoul::isGlmVector<T>() || ghoul::isGlmMatrix<T>()) {
+            return glm::all(glm::greaterThanEqual(minValue, T(0))) &&
+                   glm::all(glm::greaterThanEqual(maxValue, T(0)));
+        }
+        else {
+            return (minValue >= T(0) && maxValue >= T(0));
+        }
+    };
+
+    // While the exponential slider does not support ranges with negative values,
+    // prevent setting the exponent for such ranges
+    // @ TODO (2021-06-30, emmbr), remove this check when no longer needed
+    if (!std::is_unsigned<T>::value) {
+        if (!isValidRange(_minimumValue, _maximumValue)) {
+            LWARNINGC(
+                "NumericalProperty: setExponent",
+                fmt::format(
+                    "Setting exponent for properties with negative values in "
+                    "[min, max] range is not yet supported. Property: {}",
+                    this->fullyQualifiedIdentifier()
+                )
+            );
+            _exponent = 1.f;
+            return;
+        }
+    }
+
     _exponent = exponent;
 }
 
diff --git a/src/scene/scenegraphnode.cpp b/src/scene/scenegraphnode.cpp
index 0611b4cdd1..6ce162c4fa 100644
--- a/src/scene/scenegraphnode.cpp
+++ b/src/scene/scenegraphnode.cpp
@@ -229,7 +229,7 @@ namespace {
             // A user-facing description about this scene graph node
             std::optional<std::string> description;
 
-            // If this value is specified, GUI applications are incouraged to ignore this 
+            // If this value is specified, GUI applications are incouraged to ignore this
             // scenegraph node. This is most useful to trim collective lists of nodes and
             // not display, for example, barycenters
             std::optional<bool> hidden;
@@ -447,7 +447,9 @@ SceneGraphNode::SceneGraphNode()
             _overrideBoundingSphere = std::nullopt;
         }
     });
-    _boundingSphere.setExponent(10.f);
+    // @TODO (2021-06-30, emmbr) Uncomment this when exponential sliders support
+    // negative values
+    //_boundingSphere.setExponent(10.f);
     addProperty(_boundingSphere);
     _interactionSphere.onChange([this]() {
         if (_interactionSphere >= 0.0) {
@@ -456,8 +458,10 @@ SceneGraphNode::SceneGraphNode()
         else {
             _overrideInteractionSphere = std::nullopt;
         }
-        });
-    _interactionSphere.setExponent(10.f);
+    });
+    // @TODO (2021-06-30, emmbr) Uncomment this when exponential sliders support
+    // negative values
+    //_interactionSphere.setExponent(10.f);
     addProperty(_interactionSphere);
     addProperty(_showDebugSphere);
 }