feat: Use new FloatingPoint type (#62)

Co-authored-by: ReenigneArcher <[email protected]>

src/common/include/displaydevice/detail/jsonserializer.h

@@ -11,6 +11,7 @@ namespace display_device {
 
   // Structs
   DD_JSON_DECLARE_SERIALIZE_TYPE(Resolution)
+  DD_JSON_DECLARE_SERIALIZE_TYPE(Rational)
   DD_JSON_DECLARE_SERIALIZE_TYPE(Point)
   DD_JSON_DECLARE_SERIALIZE_TYPE(EnumeratedDevice::Info)
   DD_JSON_DECLARE_SERIALIZE_TYPE(EnumeratedDevice)

src/common/include/displaydevice/detail/jsonserializerdetails.h

@@ -43,9 +43,38 @@
     }
 
 namespace display_device {
+  /**
+   * @brief Holds information for serializing variants.
+   */
+  namespace detail {
+    template <class T>
+    struct JsonTypeName;
+
+    template <>
+    struct JsonTypeName<double> {
+      static constexpr std::string_view m_name { "double" };
+    };
+
+    template <>
+    struct JsonTypeName<Rational> {
+      static constexpr std::string_view m_name { "rational" };
+    };
+
+    template <class T, class... Ts>
+    bool
+    variantFromJson(const nlohmann::json &nlohmann_json_j, std::variant<Ts...> &value) {
+      if (nlohmann_json_j.at("type").get<std::string_view>() != JsonTypeName<T>::m_name) {
+        return false;
+      }
+
+      value = nlohmann_json_j.at("value").get<T>();
+      return true;
+    }
+  }  // namespace detail
+
   // A shared function for enums to find values in the map. Extracted here for UTs + coverage
   template <class T, class Predicate>
-  std::map<T, nlohmann::json>::const_iterator
+  typename std::map<T, nlohmann::json>::const_iterator
   findInEnumMap(const char *error_msg, Predicate predicate) {
     const auto &map { getEnumMap(T {}) };
     auto it { std::find_if(std::begin(map), std::end(map), predicate) };
@@ -58,7 +87,7 @@ namespace display_device {
 
 namespace nlohmann {
   // Specialization for optional types until they actually implement it.
-  template <typename T>
+  template <class T>
   struct adl_serializer<std::optional<T>> {
     static void
     to_json(json &nlohmann_json_j, const std::optional<T> &nlohmann_json_t) {
@@ -80,5 +109,30 @@ namespace nlohmann {
       }
     }
   };
+
+  // Specialization for variant type.
+  // See https://github.com/nlohmann/json/issues/1261#issuecomment-2048770747
+  template <typename... Ts>
+  struct adl_serializer<std::variant<Ts...>> {
+    static void
+    to_json(json &nlohmann_json_j, const std::variant<Ts...> &nlohmann_json_t) {
+      std::visit(
+        [&nlohmann_json_j]<class T>(const T &value) {
+          nlohmann_json_j["type"] = display_device::detail::JsonTypeName<std::decay_t<T>>::m_name;
+          nlohmann_json_j["value"] = value;
+        },
+        nlohmann_json_t);
+    }
+
+    static void
+    from_json(const json &nlohmann_json_j, std::variant<Ts...> &nlohmann_json_t) {
+      // Call variant_from_json for all types, only one will succeed
+      const bool found { (display_device::detail::variantFromJson<Ts>(nlohmann_json_j, nlohmann_json_t) || ...) };
+      if (!found) {
+        const std::string error { "Could not parse variant from type " + nlohmann_json_j.at("type").get<std::string>() + "!" };
+        throw std::runtime_error(error);
+      }
+    }
+  };
 }  // namespace nlohmann
 #endif

src/common/include/displaydevice/types.h

@@ -3,6 +3,7 @@
 // system includes
 #include <optional>
 #include <string>
+#include <variant>
 #include <vector>
 
 namespace display_device {
@@ -42,6 +43,25 @@ namespace display_device {
     operator==(const Point &lhs, const Point &rhs);
   };
 
+  /**
+   * @brief Floating point stored in a "numerator/denominator" form.
+   */
+  struct Rational {
+    unsigned int m_numerator {};
+    unsigned int m_denominator {};
+
+    /**
+     * @brief Comparator for strict equality.
+     */
+    friend bool
+    operator==(const Rational &lhs, const Rational &rhs);
+  };
+
+  /**
+   * @brief Floating point type.
+   */
+  using FloatingPoint = std::variant<double, Rational>;
+
   /**
    * @brief Enumerated display device information.
    */
@@ -51,8 +71,8 @@ namespace display_device {
      */
     struct Info {
       Resolution m_resolution {}; /**< Resolution of an active device. */
-      double m_resolution_scale {}; /**< Resolution scaling of an active device. */
-      double m_refresh_rate {}; /**< Refresh rate of an active device. */
+      FloatingPoint m_resolution_scale {}; /**< Resolution scaling of an active device. */
+      FloatingPoint m_refresh_rate {}; /**< Refresh rate of an active device. */
       bool m_primary {}; /**< Indicates whether the device is a primary display. */
       Point m_origin_point {}; /**< A starting point of the display. */
       std::optional<HdrState> m_hdr_state {}; /**< HDR of an active device. */
@@ -101,7 +121,7 @@ namespace display_device {
     std::string m_device_id {}; /**< Device to perform configuration for (can be empty if primary device should be used). */
     DevicePreparation m_device_prep {}; /**< Instruction on how to prepare device. */
     std::optional<Resolution> m_resolution {}; /**< Resolution to configure. */
-    std::optional<double> m_refresh_rate {}; /**< Refresh rate to configure. */
+    std::optional<FloatingPoint> m_refresh_rate {}; /**< Refresh rate to configure. */
     std::optional<HdrState> m_hdr_state {}; /**< HDR state to configure (if supported by the display). */
 
     /**

src/common/jsonserializer.cpp

@@ -16,6 +16,7 @@ namespace display_device {
 
   // Structs
   DD_JSON_DEFINE_SERIALIZE_STRUCT(Resolution, width, height)
+  DD_JSON_DEFINE_SERIALIZE_STRUCT(Rational, numerator, denominator)
   DD_JSON_DEFINE_SERIALIZE_STRUCT(Point, x, y)
   DD_JSON_DEFINE_SERIALIZE_STRUCT(EnumeratedDevice::Info, resolution, resolution_scale, refresh_rate, primary, origin_point, hdr_state)
   DD_JSON_DEFINE_SERIALIZE_STRUCT(EnumeratedDevice, device_id, display_name, friendly_name, info)

src/common/types.cpp

@@ -1,14 +1,30 @@
-// local includes
+// header include
 #include "displaydevice/types.h"
 
 namespace {
   bool
   fuzzyCompare(const double lhs, const double rhs) {
     return std::abs(lhs - rhs) * 1000000000000. <= std::min(std::abs(lhs), std::abs(rhs));
   }
+
+  bool
+  fuzzyCompare(const display_device::FloatingPoint &lhs, const display_device::FloatingPoint &rhs) {
+    if (lhs.index() == rhs.index()) {
+      if (std::holds_alternative<double>(lhs)) {
+        return fuzzyCompare(std::get<double>(lhs), std::get<double>(rhs));
+      }
+      return lhs == rhs;
+    }
+    return false;
+  }
 }  // namespace
 
 namespace display_device {
+  bool
+  operator==(const Rational &lhs, const Rational &rhs) {
+    return lhs.m_numerator == rhs.m_numerator && lhs.m_denominator == rhs.m_denominator;
+  }
+
   bool
   operator==(const Point &lhs, const Point &rhs) {
     return lhs.m_x == rhs.m_x && lhs.m_y == rhs.m_y;

src/windows/include/displaydevice/windows/detail/jsonserializer.h

@@ -6,7 +6,6 @@
 #ifdef DD_JSON_DETAIL
 namespace display_device {
   // Structs
-  DD_JSON_DECLARE_SERIALIZE_TYPE(Rational)
   DD_JSON_DECLARE_SERIALIZE_TYPE(DisplayMode)
   DD_JSON_DECLARE_SERIALIZE_TYPE(SingleDisplayConfigState::Initial)
   DD_JSON_DECLARE_SERIALIZE_TYPE(SingleDisplayConfigState::Modified)

src/windows/include/displaydevice/windows/settingsutils.h

@@ -112,7 +112,7 @@ namespace display_device::win_utils {
    */
   DeviceDisplayModeMap
   computeNewDisplayModes(const std::optional<Resolution> &resolution,
-    const std::optional<double> &refresh_rate,
+    const std::optional<FloatingPoint> &refresh_rate,
     bool configuring_primary_devices,
     const std::string &device_to_configure,
     const std::set<std::string> &additional_devices_to_configure,

src/windows/include/displaydevice/windows/types.h

@@ -77,26 +77,6 @@ namespace display_device {
    */
   using ActiveTopology = std::vector<std::vector<std::string>>;
 
-  /**
-   * @brief Floating point stored in a "numerator/denominator" form.
-   */
-  struct Rational {
-    unsigned int m_numerator {};
-    unsigned int m_denominator {};
-
-    /**
-     * @brief Contruct rational struct from double precission floating point.
-     */
-    static Rational
-    fromFloatingPoint(double value);
-
-    /**
-     * @brief Comparator for strict equality.
-     */
-    friend bool
-    operator==(const Rational &lhs, const Rational &rhs);
-  };
-
   /**
    * @brief Display's mode (resolution + refresh rate).
    */

src/windows/include/displaydevice/windows/winapilayer.h

@@ -46,7 +46,7 @@ namespace display_device {
     setHdrState(const DISPLAYCONFIG_PATH_INFO &path, HdrState state) override;
 
     /** For details @see WinApiLayerInterface::getDisplayScale */
-    [[nodiscard]] std::optional<double>
+    [[nodiscard]] std::optional<Rational>
     getDisplayScale(const std::string &display_name, const DISPLAYCONFIG_SOURCE_MODE &source_mode) const override;
   };
 }  // namespace display_device

src/windows/include/displaydevice/windows/winapilayerinterface.h

@@ -212,7 +212,7 @@ namespace display_device {
      * const auto scale = iface->getDisplayScale(iface->getDisplayName(path), source_mode);
      * ```
      */
-    [[nodiscard]] virtual std::optional<double>
+    [[nodiscard]] virtual std::optional<Rational>
     getDisplayScale(const std::string &display_name, const DISPLAYCONFIG_SOURCE_MODE &source_mode) const = 0;
   };
 }  // namespace display_device

src/windows/jsonserializer.cpp

@@ -7,7 +7,6 @@
 
 namespace display_device {
   // Structs
-  DD_JSON_DEFINE_SERIALIZE_STRUCT(Rational, numerator, denominator)
   DD_JSON_DEFINE_SERIALIZE_STRUCT(DisplayMode, resolution, refresh_rate)
   DD_JSON_DEFINE_SERIALIZE_STRUCT(SingleDisplayConfigState::Initial, topology, primary_devices)
   DD_JSON_DEFINE_SERIALIZE_STRUCT(SingleDisplayConfigState::Modified, topology, original_modes, original_hdr_states, original_primary_device)

src/windows/settingsutils.cpp

@@ -3,6 +3,7 @@
 
 // system includes
 #include <algorithm>
+#include <cmath>
 #include <thread>
 
 // local includes
@@ -264,7 +265,7 @@ namespace display_device::win_utils {
   }
 
   DeviceDisplayModeMap
-  computeNewDisplayModes(const std::optional<Resolution> &resolution, const std::optional<double> &refresh_rate, const bool configuring_primary_devices, const std::string &device_to_configure, const std::set<std::string> &additional_devices_to_configure, const DeviceDisplayModeMap &original_modes) {
+  computeNewDisplayModes(const std::optional<Resolution> &resolution, const std::optional<FloatingPoint> &refresh_rate, const bool configuring_primary_devices, const std::string &device_to_configure, const std::set<std::string> &additional_devices_to_configure, const DeviceDisplayModeMap &original_modes) {
     DeviceDisplayModeMap new_modes { original_modes };
 
     if (resolution) {
@@ -278,19 +279,32 @@ namespace display_device::win_utils {
     }
 
     if (refresh_rate) {
+      const auto from_floating_point { [](const FloatingPoint &value) {
+        if (const auto *rational_value { std::get_if<Rational>(&value) }; rational_value) {
+          return *rational_value;
+        }
+
+        // It's hard to deal with floating values, so we just multiply it
+        // to keep 4 decimal places (if any) and let Windows deal with it!
+        // Genius idea if I'm being honest.
+        constexpr auto multiplier { static_cast<unsigned int>(std::pow(10, 4)) };
+        const double transformed_value { std::round(std::get<double>(value) * multiplier) };
+        return Rational { static_cast<unsigned int>(transformed_value), multiplier };
+      } };
+
       if (configuring_primary_devices) {
         // No device has been specified, so if they're all are primary devices
         // we need to apply the refresh rate change to all duplicates.
         const auto devices { joinConfigurableDevices(device_to_configure, additional_devices_to_configure) };
         for (const auto &device_id : devices) {
-          new_modes[device_id].m_refresh_rate = Rational::fromFloatingPoint(*refresh_rate);
+          new_modes[device_id].m_refresh_rate = from_floating_point(*refresh_rate);
         }
       }
       else {
         // Even if we have duplicate devices, their refresh rate may differ
         // and since the device was specified, let's apply the refresh
         // rate only to the specified device.
-        new_modes[device_to_configure].m_refresh_rate = Rational::fromFloatingPoint(*refresh_rate);
+        new_modes[device_to_configure].m_refresh_rate = from_floating_point(*refresh_rate);
       }
     }
 

src/windows/types.cpp

@@ -1,25 +1,7 @@
 // header include
 #include "displaydevice/windows/types.h"
 
-// system includes
-#include <cmath>
-
 namespace display_device {
-  Rational
-  Rational::fromFloatingPoint(const double value) {
-    // It's hard to deal with floating values, so we just multiply it
-    // to keep 4 decimal places (if any) and let Windows deal with it!
-    // Genius idea if I'm being honest.
-    constexpr auto multiplier { static_cast<unsigned int>(std::pow(10, 4)) };
-    const double transformed_value { std::round(value * multiplier) };
-    return Rational { static_cast<unsigned int>(transformed_value), multiplier };
-  }
-
-  bool
-  operator==(const Rational &lhs, const Rational &rhs) {
-    return lhs.m_numerator == rhs.m_numerator && lhs.m_denominator == rhs.m_denominator;
-  }
-
   bool
   operator==(const DisplayMode &lhs, const DisplayMode &rhs) {
     return lhs.m_refresh_rate == rhs.m_refresh_rate && lhs.m_resolution == rhs.m_resolution;

src/windows/winapilayer.cpp

@@ -7,6 +7,7 @@
 #include <boost/uuid/name_generator_sha1.hpp>
 #include <boost/uuid/uuid.hpp>
 #include <boost/uuid/uuid_io.hpp>
+#include <cmath>
 #include <cstdint>
 #include <iomanip>
 
@@ -575,7 +576,7 @@ namespace display_device {
     return true;
   }
 
-  std::optional<double>
+  std::optional<Rational>
   WinApiLayer::getDisplayScale(const std::string &display_name, const DISPLAYCONFIG_SOURCE_MODE &source_mode) const {
     // Note: implementation based on https://stackoverflow.com/a/74046173
     struct EnumData {
@@ -617,6 +618,6 @@ namespace display_device {
     }
 
     const auto width { static_cast<double>(*enum_data.m_width) / static_cast<double>(source_mode.width) };
-    return static_cast<double>(GetDpiForSystem()) / 96. / width;
+    return Rational { static_cast<unsigned int>(std::round((static_cast<double>(GetDpiForSystem()) / 96. / width) * 100)), 100 };
   }
 }  // namespace display_device

src/windows/windisplaydevicegeneral.cpp

@@ -59,15 +59,15 @@ namespace display_device {
         }
 
         const auto display_name { m_w_api->getDisplayName(best_path) };
-        const double refresh_rate { best_path.targetInfo.refreshRate.Denominator > 0 ?
-                                      static_cast<double>(best_path.targetInfo.refreshRate.Numerator) / static_cast<double>(best_path.targetInfo.refreshRate.Denominator) :
-                                      0. };
+        const Rational refresh_rate { best_path.targetInfo.refreshRate.Denominator > 0 ?
+                                        Rational { best_path.targetInfo.refreshRate.Numerator, best_path.targetInfo.refreshRate.Denominator } :
+                                        Rational { 0, 1 } };
         const EnumeratedDevice::Info info {
           { source_mode->width, source_mode->height },
-          m_w_api->getDisplayScale(display_name, *source_mode).value_or(0.),
+          m_w_api->getDisplayScale(display_name, *source_mode).value_or(Rational { 0, 1 }),
           refresh_rate,
           win_utils::isPrimary(*source_mode),
-          { source_mode->position.x, source_mode->position.y },
+          { static_cast<int>(source_mode->position.x), static_cast<int>(source_mode->position.y) },
           m_w_api->getHdrState(best_path)
         };
 

tests/fixtures/include/fixtures/jsonconvertertest.h

@@ -14,8 +14,11 @@ class JsonConverterTest: public BaseTest {
     EXPECT_TRUE(success);
     EXPECT_EQ(json_string, expected_string);
 
+    std::string error_message {};
     T defaulted_input {};
-    EXPECT_TRUE(display_device::fromJson(json_string, defaulted_input));
+    if (!display_device::fromJson(json_string, defaulted_input, &error_message)) {
+      GTEST_FAIL() << error_message;
+    }
     EXPECT_EQ(input, defaulted_input);
   }
 };