Skip to content

Commit

Permalink
feat: Add HDR state handling logic (#54)
Browse files Browse the repository at this point in the history
  • Loading branch information
FrogTheFrog authored Jul 11, 2024
1 parent 239f0cb commit 45edace
Show file tree
Hide file tree
Showing 7 changed files with 432 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace display_device {
DevicePrepFailed,
PrimaryDevicePrepFailed,
DisplayModePrepFailed,
HdrStatePrepFailed,
PersistenceSaveFailed,
};

Expand Down
12 changes: 12 additions & 0 deletions src/windows/include/displaydevice/windows/settingsmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ namespace display_device {
[[nodiscard]] bool
prepareDisplayModes(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set<std::string> &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state);

/**
* @brief Changes or restores the HDR states based on the cached state, new state and configuration.
* @param config Configuration to be used for preparing HDR states.
* @param device_to_configure The main device to be used for preparation.
* @param additional_devices_to_configure Additional devices that should be configured.
* @param guard_fn Reference to the guard function which will be set to restore original state (if needed) in case something else fails down the line.
* @param new_state Reference to the new state which is to be updated accordingly.
* @return True if no errors have occured, false otherwise.
*/
[[nodiscard]] bool
prepareHdrStates(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set<std::string> &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state);

/**
* @brief Try to revert the modified settings.
* @returns True on success, false otherwise.
Expand Down
33 changes: 33 additions & 0 deletions src/windows/include/displaydevice/windows/settingsutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,22 @@ namespace display_device::win_utils {
const std::set<std::string> &additional_devices_to_configure,
const DeviceDisplayModeMap &original_modes);

/**
* @brief Compute new HDR states from arbitrary data.
* @param hdr_state Specify state that should be used to override the original states.
* @param configuring_primary_devices Specify whether the `device_to_configure` was unspecified (primary device was selected).
* @param device_to_configure Main device to be configured.
* @param additional_devices_to_configure Additional devices that belong to the same group as `device_to_configure`.
* @param original_states HDR states to be used as a base onto which changes are made.
* @return New HDR states that should be set.
*/
HdrStateMap
computeNewHdrStates(const std::optional<HdrState> &hdr_state,
bool configuring_primary_devices,
const std::string &device_to_configure,
const std::set<std::string> &additional_devices_to_configure,
const HdrStateMap &original_states);

/**
* @brief Make guard function for the topology.
* @param win_dd Interface for interacting with the OS.
Expand Down Expand Up @@ -210,4 +226,21 @@ namespace display_device::win_utils {
*/
DdGuardFn
hdrStateGuardFn(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology);

/**
* @brief Make guard function for the HDR states.
* @param win_dd Interface for interacting with the OS.
* @param states HDR states to restore when the guard is executed.
* @return Function that once called will try to revert HDR states to the ones that were provided.
*
* EXAMPLES:
* ```cpp
* WinDisplayDeviceInterface* iface = getIface(...);
* const HdrStateMap states { };
*
* boost::scope::scope_exit guard { hdrStateGuardFn(*iface, states) };
* ```
*/
DdGuardFn
hdrStateGuardFn(WinDisplayDeviceInterface &win_dd, const HdrStateMap &states);
} // namespace display_device::win_utils
68 changes: 62 additions & 6 deletions src/windows/settingsmanagerapply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ namespace display_device {
return ApplyResult::DisplayModePrepFailed;
}

// TODO:
//
// Other device handling goes here that will use device_to_configure and additional_devices_to_configure:
//
// - handle HDR (need to replicate the HDR bug and find the best place for workaround)
//
DdGuardFn hdr_state_guard_fn { noopFn };
boost::scope::scope_exit<DdGuardFn &> hdr_state_guard { hdr_state_guard_fn };
if (!prepareHdrStates(config, device_to_configure, additional_devices_to_configure, hdr_state_guard_fn, new_state)) {
// Error already logged
return ApplyResult::HdrStatePrepFailed;
}

// We will always keep the new state persistently, even if there are no new meaningful changes, because
// we want to preserve the initial state for consistency.
Expand All @@ -105,6 +105,7 @@ namespace display_device {
topology_prep_guard.set_active(false);
primary_guard.set_active(false);
mode_guard.set_active(false);
hdr_state_guard.set_active(false);
return ApplyResult::Ok;
}

Expand Down Expand Up @@ -304,4 +305,59 @@ namespace display_device {

return true;
}

[[nodiscard]] bool
SettingsManager::prepareHdrStates(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set<std::string> &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state) {
const auto &cached_state { m_persistence_state->getState() };
const auto cached_hdr_states { cached_state ? cached_state->m_modified.m_original_hdr_states : HdrStateMap {} };
const bool change_required { config.m_hdr_state };
const bool might_need_to_restore { !cached_hdr_states.empty() };

HdrStateMap current_hdr_states;
if (change_required || might_need_to_restore) {
current_hdr_states = m_dd_api->getCurrentHdrStates(win_utils::flattenTopology(new_state.m_modified.m_topology));
if (current_hdr_states.empty()) {
DD_LOG(error) << "Failed to get current HDR states!";
return false;
}
}

const auto try_change { [&](const HdrStateMap &new_states, const auto info_preamble, const auto error_log) {
if (current_hdr_states != new_states) {
DD_LOG(info) << info_preamble << toJson(new_states);
if (!m_dd_api->setHdrStates(new_states)) {
DD_LOG(error) << error_log;
return false;
}

guard_fn = win_utils::hdrStateGuardFn(*m_dd_api, current_hdr_states);
}

return true;
} };

if (change_required) {
const bool configuring_primary_devices { config.m_device_id.empty() };
const auto original_hdr_states { cached_hdr_states.empty() ? current_hdr_states : cached_hdr_states };
const auto new_hdr_states { win_utils::computeNewHdrStates(config.m_hdr_state, configuring_primary_devices, device_to_configure, additional_devices_to_configure, original_hdr_states) };

if (!try_change(new_hdr_states, "Changing HDR states to: ", "Failed to apply new configuration, because new HDR states could not be set!")) {
// Error already logged
return false;
}

// Here we preserve the data from persistence (unless there's none) as in the end that is what we want to go back to.
new_state.m_modified.m_original_hdr_states = original_hdr_states;
return true;
}

if (might_need_to_restore) {
if (!try_change(cached_hdr_states, "Changing HDR states back to: ", "Failed to restore original HDR states!")) {
// Error already logged
return false;
}
}

return true;
}
} // namespace display_device
39 changes: 38 additions & 1 deletion src/windows/settingsutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,39 @@ namespace display_device::win_utils {
return new_modes;
}

HdrStateMap
computeNewHdrStates(const std::optional<HdrState> &hdr_state, bool configuring_primary_devices, const std::string &device_to_configure, const std::set<std::string> &additional_devices_to_configure, const HdrStateMap &original_states) {
HdrStateMap new_states { original_states };

if (hdr_state) {
const auto try_update_new_state = [&new_states, &hdr_state](const std::string &device_id) {
const auto current_state { new_states[device_id] };
if (!current_state) {
return;
}

new_states[device_id] = *hdr_state;
};

if (configuring_primary_devices) {
// No device has been specified, so if they're all are primary devices
// we need to update state for all duplicates.
const auto devices { joinConfigurableDevices(device_to_configure, additional_devices_to_configure) };
for (const auto &device_id : devices) {
try_update_new_state(device_id);
}
}
else {
// Even if we have duplicate devices, their HDR states may differ
// and since the device was specified, let's apply the HDR state
// only to the specified device.
try_update_new_state(device_to_configure);
}
}

return new_states;
}

DdGuardFn
topologyGuardFn(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology) {
DD_LOG(debug) << "Got topology in topologyGuardFn:\n"
Expand Down Expand Up @@ -344,7 +377,11 @@ namespace display_device::win_utils {

DdGuardFn
hdrStateGuardFn(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology) {
const auto states = win_dd.getCurrentHdrStates(flattenTopology(topology));
return hdrStateGuardFn(win_dd, win_dd.getCurrentHdrStates(flattenTopology(topology)));
}

DdGuardFn
hdrStateGuardFn(WinDisplayDeviceInterface &win_dd, const HdrStateMap &states) {
DD_LOG(debug) << "Got states in hdrStateGuardFn:\n"
<< toJson(states);
return [&win_dd, states]() {
Expand Down
Loading

0 comments on commit 45edace

Please sign in to comment.