feat: externalize workarounds for Windows (#92)

FrogTheFrog · web-flow · commit b30c2e117e86 · 2024-08-08T21:48:27.000Z
diff --git a/src/common/include/display_device/detail/json_serializer_details.h b/src/common/include/display_device/detail/json_serializer_details.h
@@ -109,7 +109,7 @@ namespace nlohmann {
         nlohmann_json_t = std::nullopt;
       }
       else {
-        nlohmann_json_t = nlohmann_json_j.template get<T>();
+        nlohmann_json_t = nlohmann_json_j.get<T>();
       }
     }
   };
@@ -138,5 +138,23 @@ namespace nlohmann {
       }
     }
   };
+
+  // Specialization for chrono duration.
+  template <class Rep, class Period>
+  struct adl_serializer<std::chrono::duration<Rep, Period>> {
+    using NanoRep = decltype(std::chrono::nanoseconds {}.count());
+    static_assert(std::numeric_limits<Rep>::max() <= std::numeric_limits<NanoRep>::max(),
+      "Duration support above nanoseconds have not been tested/verified yet!");
+
+    static void
+    to_json(json &nlohmann_json_j, const std::chrono::duration<Rep, Period> &nlohmann_json_t) {
+      nlohmann_json_j = nlohmann_json_t.count();
+    }
+
+    static void
+    from_json(const json &nlohmann_json_j, std::chrono::duration<Rep, Period> &nlohmann_json_t) {
+      nlohmann_json_t = std::chrono::duration<Rep, Period> { nlohmann_json_j.get<Rep>() };
+    }
+  };
 }  // namespace nlohmann
 #endif
diff --git a/src/common/include/display_device/json.h b/src/common/include/display_device/json.h
@@ -25,6 +25,7 @@
 namespace display_device {
   extern const std::optional<unsigned int> JSON_COMPACT;
 
+  DD_JSON_DECLARE_CONVERTER(EnumeratedDevice)
   DD_JSON_DECLARE_CONVERTER(EnumeratedDeviceList)
   DD_JSON_DECLARE_CONVERTER(SingleDisplayConfiguration)
   DD_JSON_DECLARE_CONVERTER(std::set<std::string>)
diff --git a/src/common/json.cpp b/src/common/json.cpp
@@ -13,6 +13,7 @@
 // clang-format on
 
 namespace display_device {
+  DD_JSON_DEFINE_CONVERTER(EnumeratedDevice)
   DD_JSON_DEFINE_CONVERTER(EnumeratedDeviceList)
   DD_JSON_DEFINE_CONVERTER(SingleDisplayConfiguration)
   DD_JSON_DEFINE_CONVERTER(std::set<std::string>)
diff --git a/src/windows/include/display_device/windows/detail/json_serializer.h b/src/windows/include/display_device/windows/detail/json_serializer.h
@@ -14,5 +14,6 @@ namespace display_device {
   DD_JSON_DECLARE_SERIALIZE_TYPE(SingleDisplayConfigState::Initial)
   DD_JSON_DECLARE_SERIALIZE_TYPE(SingleDisplayConfigState::Modified)
   DD_JSON_DECLARE_SERIALIZE_TYPE(SingleDisplayConfigState)
+  DD_JSON_DECLARE_SERIALIZE_TYPE(WinWorkarounds)
 }  // namespace display_device
 #endif
diff --git a/src/windows/include/display_device/windows/json.h b/src/windows/include/display_device/windows/json.h
@@ -14,4 +14,5 @@ namespace display_device {
   DD_JSON_DECLARE_CONVERTER(DeviceDisplayModeMap)
   DD_JSON_DECLARE_CONVERTER(HdrStateMap)
   DD_JSON_DECLARE_CONVERTER(SingleDisplayConfigState)
+  DD_JSON_DECLARE_CONVERTER(WinWorkarounds)
 }  // namespace display_device
diff --git a/src/windows/include/display_device/windows/settings_manager.h b/src/windows/include/display_device/windows/settings_manager.h
@@ -5,7 +5,6 @@
 #pragma once
 
 // system includes
-#include <chrono>
 #include <memory>
 
 // local includes
@@ -25,11 +24,13 @@ namespace display_device {
      * @param dd_api A pointer to the Windows Display Device interface. Will throw on nullptr!
      * @param audio_context_api [Optional] A pointer to the Audio Context interface.
      * @param persistent_state A pointer to a class for managing persistence.
+     * @param workarounds Workaround settings for the APIs.
      */
     explicit SettingsManager(
       std::shared_ptr<WinDisplayDeviceInterface> dd_api,
       std::shared_ptr<AudioContextInterface> audio_context_api,
-      std::unique_ptr<PersistentState> persistent_state);
+      std::unique_ptr<PersistentState> persistent_state,
+      WinWorkarounds workarounds);
 
     /** For details @see SettingsManagerInterface::enumAvailableDevices */
     [[nodiscard]] EnumeratedDeviceList
@@ -115,9 +116,6 @@ namespace display_device {
     std::shared_ptr<WinDisplayDeviceInterface> m_dd_api;
     std::shared_ptr<AudioContextInterface> m_audio_context_api;
     std::unique_ptr<PersistentState> m_persistence_state;
-
-  private:
-    /** @see win_utils::blankHdrStates for more details. */
-    std::chrono::milliseconds m_hdr_blank_delay { 500 };  // 500ms should be more than enough...
+    WinWorkarounds m_workarounds;
   };
 }  // namespace display_device
diff --git a/src/windows/include/display_device/windows/settings_utils.h b/src/windows/include/display_device/windows/settings_utils.h
@@ -165,9 +165,10 @@ namespace display_device::win_utils {
    *
    * @param win_dd Interface for interacting with the OS.
    * @param delay Delay between OFF and ON states (ON -> OFF -> DELAY -> ON).
+   *              If null optional is provided, the function does nothing.
    */
   void
-  blankHdrStates(WinDisplayDeviceInterface &win_dd, std::chrono::milliseconds delay);
+  blankHdrStates(WinDisplayDeviceInterface &win_dd, const std::optional<std::chrono::milliseconds> &delay);
 
   /**
    * @brief Make guard function for the topology.
diff --git a/src/windows/include/display_device/windows/types.h b/src/windows/include/display_device/windows/types.h
@@ -11,6 +11,7 @@
 #include <windows.h>
 
 // system includes
+#include <chrono>
 #include <functional>
 #include <map>
 #include <set>
@@ -171,4 +172,17 @@ namespace display_device {
    * @brief Default function type used for cleanup/guard functions.
    */
   using DdGuardFn = std::function<void()>;
+
+  /**
+   * @brief Settings for workarounds/hacks for Windows.
+   */
+  struct WinWorkarounds {
+    std::optional<std::chrono::milliseconds> m_hdr_blank_delay { std::nullopt };  ///< @seealso{win_utils::blankHdrStates for more details.}
+
+    /**
+     * @brief Comparator for strict equality.
+     */
+    friend bool
+    operator==(const WinWorkarounds &lhs, const WinWorkarounds &rhs);
+  };
 }  // namespace display_device
diff --git a/src/windows/json.cpp b/src/windows/json.cpp
@@ -19,4 +19,5 @@ namespace display_device {
   DD_JSON_DEFINE_CONVERTER(DeviceDisplayModeMap)
   DD_JSON_DEFINE_CONVERTER(HdrStateMap)
   DD_JSON_DEFINE_CONVERTER(SingleDisplayConfigState)
+  DD_JSON_DEFINE_CONVERTER(WinWorkarounds)
 }  // namespace display_device
diff --git a/src/windows/json_serializer.cpp b/src/windows/json_serializer.cpp
@@ -15,4 +15,5 @@ namespace display_device {
   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)
   DD_JSON_DEFINE_SERIALIZE_STRUCT(SingleDisplayConfigState, initial, modified)
+  DD_JSON_DEFINE_SERIALIZE_STRUCT(WinWorkarounds, hdr_blank_delay)
 }  // namespace display_device
diff --git a/src/windows/settings_manager_apply.cpp b/src/windows/settings_manager_apply.cpp
@@ -47,7 +47,7 @@ namespace display_device {
     bool system_settings_touched { false };
     boost::scope::scope_exit hdr_blank_always_executed_guard { [this, &system_settings_touched]() {
       if (system_settings_touched) {
-        win_utils::blankHdrStates(*m_dd_api, m_hdr_blank_delay);
+        win_utils::blankHdrStates(*m_dd_api, m_workarounds.m_hdr_blank_delay);
       }
     } };
 
diff --git a/src/windows/settings_manager_general.cpp b/src/windows/settings_manager_general.cpp
@@ -8,15 +8,18 @@
 // local includes
 #include "display_device/logging.h"
 #include "display_device/noop_audio_context.h"
+#include "display_device/windows/json.h"
 
 namespace display_device {
   SettingsManager::SettingsManager(
     std::shared_ptr<WinDisplayDeviceInterface> dd_api,
     std::shared_ptr<AudioContextInterface> audio_context_api,
-    std::unique_ptr<PersistentState> persistent_state):
+    std::unique_ptr<PersistentState> persistent_state,
+    WinWorkarounds workarounds):
       m_dd_api { std::move(dd_api) },
       m_audio_context_api { std::move(audio_context_api) },
-      m_persistence_state { std::move(persistent_state) } {
+      m_persistence_state { std::move(persistent_state) },
+      m_workarounds { std::move(workarounds) } {
     if (!m_dd_api) {
       throw std::logic_error { "Nullptr provided for WinDisplayDeviceInterface in SettingsManager!" };
     }
@@ -28,6 +31,9 @@ namespace display_device {
     if (!m_persistence_state) {
       throw std::logic_error { "Nullptr provided for PersistentState in SettingsManager!" };
     }
+
+    DD_LOG(info) << "Provided workaround settings for SettingsManager:\n"
+                 << toJson(m_workarounds);
   }
 
   EnumeratedDeviceList
diff --git a/src/windows/settings_manager_revert.cpp b/src/windows/settings_manager_revert.cpp
@@ -47,7 +47,7 @@ namespace display_device {
     bool system_settings_touched { false };
     boost::scope::scope_exit hdr_blank_always_executed_guard { [this, &system_settings_touched]() {
       if (system_settings_touched) {
-        win_utils::blankHdrStates(*m_dd_api, m_hdr_blank_delay);
+        win_utils::blankHdrStates(*m_dd_api, m_workarounds.m_hdr_blank_delay);
       }
     } };
     boost::scope::scope_exit topology_prep_guard { [this, &cached_state, &current_topology, &system_settings_touched]() {
diff --git a/src/windows/settings_utils.cpp b/src/windows/settings_utils.cpp
@@ -358,7 +358,11 @@ namespace display_device::win_utils {
   }
 
   void
-  blankHdrStates(WinDisplayDeviceInterface &win_dd, const std::chrono::milliseconds delay) {
+  blankHdrStates(WinDisplayDeviceInterface &win_dd, const std::optional<std::chrono::milliseconds> &delay) {
+    if (!delay) {
+      return;
+    }
+
     const auto topology { win_dd.getCurrentTopology() };
     if (!win_dd.isTopologyValid(topology)) {
       DD_LOG(error) << "Got an invalid topology while trying to blank HDR states!";
@@ -390,13 +394,13 @@ namespace display_device::win_utils {
       return;
     }
 
-    DD_LOG(info) << "Applying HDR state \"blank\" workaround (" << delay.count() << "ms) to devices: " << toJson(device_ids, JSON_COMPACT);
+    DD_LOG(info) << "Applying HDR state \"blank\" workaround (" << delay->count() << "ms) to devices: " << toJson(device_ids, JSON_COMPACT);
     if (!win_dd.setHdrStates(inverse_states)) {
       DD_LOG(error) << "Failed to apply inverse HDR states during \"blank\"!";
       return;
     }
 
-    std::this_thread::sleep_for(delay);
+    std::this_thread::sleep_for(*delay);
     if (!win_dd.setHdrStates(original_states)) {
       DD_LOG(error) << "Failed to apply original HDR states during \"blank\"!";
     }
diff --git a/src/windows/types.cpp b/src/windows/types.cpp
@@ -30,4 +30,9 @@ namespace display_device {
   operator==(const SingleDisplayConfigState &lhs, const SingleDisplayConfigState &rhs) {
     return lhs.m_initial == rhs.m_initial && lhs.m_modified == rhs.m_modified;
   }
+
+  bool
+  operator==(const WinWorkarounds &lhs, const WinWorkarounds &rhs) {
+    return lhs.m_hdr_blank_delay == rhs.m_hdr_blank_delay;
+  }
 }  // namespace display_device
diff --git a/tests/unit/general/test_json.cpp b/tests/unit/general/test_json.cpp
@@ -44,11 +44,25 @@ namespace display_device {
   DD_JSON_DEFINE_CONVERTER(TestEnum)
   DD_JSON_DEFINE_CONVERTER(TestStruct)
   DD_JSON_DEFINE_CONVERTER(TestVariant)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::nanoseconds)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::microseconds)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::milliseconds)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::seconds)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::minutes)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::hours)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::days)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::weeks)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::months)
+  DD_JSON_DEFINE_CONVERTER(std::chrono::years)
 }  // namespace display_device
 
 namespace {
   // Specialized TEST macro(s) for this test file
 #define TEST_S(...) DD_MAKE_TEST(TEST, JsonTest, __VA_ARGS__)
+
+  // Additional convenience global const(s)
+  constexpr auto MIN_NANO_VAL { std::numeric_limits<decltype(std::chrono::nanoseconds {}.count())>::min() };
+  constexpr auto MAX_NANO_VAL { std::numeric_limits<decltype(std::chrono::nanoseconds {}.count())>::max() };
 }  // namespace
 
 TEST_S(ToJson, NoError, WithSuccessParam) {
@@ -192,3 +206,55 @@ TEST_S(FromJson, TestVariant, UnknownVariantType) {
   EXPECT_FALSE(display_device::fromJson(R"({"type":"SomeUnknownType","value":123.0})", variant, &error_message));
   EXPECT_EQ(error_message, "Could not parse variant from type SomeUnknownType!");
 }
+
+TEST_S(ToJson, ChronoDuration) {
+  using namespace std::chrono;
+
+  EXPECT_EQ(display_device::toJson(nanoseconds { 2000000000 }, std::nullopt, nullptr), R"(2000000000)");
+  EXPECT_EQ(display_device::toJson(microseconds { 2000000 }, std::nullopt, nullptr), R"(2000000)");
+  EXPECT_EQ(display_device::toJson(milliseconds { 2000 }, std::nullopt, nullptr), R"(2000)");
+  EXPECT_EQ(display_device::toJson(seconds { 2 }, std::nullopt, nullptr), R"(2)");
+  EXPECT_EQ(display_device::toJson(minutes { 20 }, std::nullopt, nullptr), R"(20)");
+  EXPECT_EQ(display_device::toJson(hours { 20 }, std::nullopt, nullptr), R"(20)");
+  EXPECT_EQ(display_device::toJson(days { 20 }, std::nullopt, nullptr), R"(20)");
+  EXPECT_EQ(display_device::toJson(weeks { 20 }, std::nullopt, nullptr), R"(20)");
+  EXPECT_EQ(display_device::toJson(months { 20 }, std::nullopt, nullptr), R"(20)");
+  EXPECT_EQ(display_device::toJson(years { 20 }, std::nullopt, nullptr), R"(20)");
+}
+
+TEST_S(FromJson, ChronoDuration) {
+  const auto doTest { []<class T>(const std::string &string_input, T expected_value) {
+    T value {};
+
+    EXPECT_TRUE(display_device::fromJson(string_input, value, nullptr));
+    EXPECT_EQ(value, expected_value);
+  } };
+
+  using namespace std::chrono;
+
+  doTest(R"(2000000000)", nanoseconds { 2000000000 });
+  doTest(R"(2000000)", microseconds { 2000000 });
+  doTest(R"(2000)", milliseconds { 2000 });
+  doTest(R"(2)", seconds { 2 });
+  doTest(R"(20)", minutes { 20 });
+  doTest(R"(20)", hours { 20 });
+  doTest(R"(20)", days { 20 });
+  doTest(R"(20)", weeks { 20 });
+  doTest(R"(20)", months { 20 });
+  doTest(R"(20)", years { 20 });
+}
+
+TEST_S(ToJson, ChronoDuration, Ranges) {
+  EXPECT_EQ(display_device::toJson(std::chrono::nanoseconds { MIN_NANO_VAL }, std::nullopt, nullptr), std::to_string(MIN_NANO_VAL));
+  EXPECT_EQ(display_device::toJson(std::chrono::nanoseconds { MAX_NANO_VAL }, std::nullopt, nullptr), std::to_string(MAX_NANO_VAL));
+}
+
+TEST_S(FromJson, ChronoDuration, Ranges) {
+  std::chrono::nanoseconds value {};
+
+  EXPECT_TRUE(display_device::fromJson(std::to_string(MIN_NANO_VAL), value, nullptr));
+  EXPECT_EQ(value, std::chrono::nanoseconds { MIN_NANO_VAL });
+
+  EXPECT_TRUE(display_device::fromJson(std::to_string(MAX_NANO_VAL), value, nullptr));
+  EXPECT_EQ(value, std::chrono::nanoseconds { MAX_NANO_VAL });
+}
diff --git a/tests/unit/general/test_json_converter.cpp b/tests/unit/general/test_json_converter.cpp
@@ -6,6 +6,37 @@ namespace {
 #define TEST_F_S(...) DD_MAKE_TEST(TEST_F, JsonConverterTest, __VA_ARGS__)
 }  // namespace
 
+TEST_F_S(EnumeratedDevice) {
+  display_device::EnumeratedDevice item_1 {
+    "ID_1",
+    "NAME_2",
+    "FU_NAME_3",
+    display_device::EnumeratedDevice::Info {
+      { 1920, 1080 },
+      display_device::Rational { 175, 100 },
+      119.9554,
+      false,
+      { 1, 2 },
+      display_device::HdrState::Enabled }
+  };
+  display_device::EnumeratedDevice item_2 {
+    "ID_2",
+    "NAME_2",
+    "FU_NAME_2",
+    display_device::EnumeratedDevice::Info {
+      { 1920, 1080 },
+      1.75,
+      display_device::Rational { 1199554, 10000 },
+      true,
+      { 0, 0 },
+      display_device::HdrState::Disabled }
+  };
+
+  executeTestCase(display_device::EnumeratedDevice {}, R"({"device_id":"","display_name":"","friendly_name":"","info":null})");
+  executeTestCase(item_1, R"({"device_id":"ID_1","display_name":"NAME_2","friendly_name":"FU_NAME_3","info":{"hdr_state":"Enabled","origin_point":{"x":1,"y":2},"primary":false,"refresh_rate":{"type":"double","value":119.9554},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"rational","value":{"denominator":100,"numerator":175}}}})");
+  executeTestCase(item_2, R"({"device_id":"ID_2","display_name":"NAME_2","friendly_name":"FU_NAME_2","info":{"hdr_state":"Disabled","origin_point":{"x":0,"y":0},"primary":true,"refresh_rate":{"type":"rational","value":{"denominator":10000,"numerator":1199554}},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"double","value":1.75}}})");
+}
+
 TEST_F_S(EnumeratedDeviceList) {
   display_device::EnumeratedDevice item_1 {
     "ID_1",
diff --git a/tests/unit/windows/test_json_converter.cpp b/tests/unit/windows/test_json_converter.cpp
@@ -40,3 +40,12 @@ TEST_F_S(SingleDisplayConfigState) {
   executeTestCase(display_device::SingleDisplayConfigState {}, R"({"initial":{"primary_devices":[],"topology":[]},"modified":{"original_hdr_states":{},"original_modes":{},"original_primary_device":"","topology":[]}})");
   executeTestCase(valid_input, R"({"initial":{"primary_devices":["DeviceId1"],"topology":[["DeviceId1"]]},"modified":{"original_hdr_states":{"DeviceId2":"Disabled"},"original_modes":{"DeviceId2":{"refresh_rate":{"denominator":1,"numerator":120},"resolution":{"height":1080,"width":1920}}},"original_primary_device":"DeviceId2","topology":[["DeviceId2"]]}})");
 }
+
+TEST_F_S(WinWorkarounds) {
+  display_device::WinWorkarounds input {
+    std::chrono::milliseconds { 500 }
+  };
+
+  executeTestCase(display_device::WinWorkarounds {}, R"({"hdr_blank_delay":null})");
+  executeTestCase(input, R"({"hdr_blank_delay":500})");
+}
diff --git a/tests/unit/windows/test_settings_manager_apply.cpp b/tests/unit/windows/test_settings_manager_apply.cpp
@@ -47,7 +47,10 @@ namespace {
     display_device::SettingsManager &
     getImpl() {
       if (!m_impl) {
-        m_impl = std::make_unique<display_device::SettingsManager>(m_dd_api, m_audio_context_api, std::make_unique<display_device::PersistentState>(m_settings_persistence_api));
+        m_impl = std::make_unique<display_device::SettingsManager>(m_dd_api, m_audio_context_api, std::make_unique<display_device::PersistentState>(m_settings_persistence_api),
+          display_device::WinWorkarounds {
+            .m_hdr_blank_delay = std::chrono::milliseconds { 123 }  // Value is irrelevant for the tests
+          });
       }
 
       return *m_impl;
diff --git a/tests/unit/windows/test_settings_manager_general.cpp b/tests/unit/windows/test_settings_manager_general.cpp
diff --git a/tests/unit/windows/test_settings_manager_revert.cpp b/tests/unit/windows/test_settings_manager_revert.cpp
diff --git a/tests/unit/windows/test_settings_utils.cpp b/tests/unit/windows/test_settings_utils.cpp
diff --git a/tests/unit/windows/test_win_playground.cpp b/tests/unit/windows/test_win_playground.cpp
