From 48f12cad166ea10deffbda0a4736c000e5ffcea1 Mon Sep 17 00:00:00 2001 From: Dan Alec Date: Fri, 16 Oct 2026 23:23:51 -0300 Subject: [PATCH 1/7] fix: check the two return values Code Analysis flags Two unchecked return values. On MSVC 18 both stop the build; on the pinned CI toolset neither surfaces. Neither defect is cosmetic. AvSetMmThreadCharacteristicsW returns NULL on failure, and that NULL went straight into AvRevertMmThreadCharacteristics, which does not accept it. Reported as C28183 and promoted to C2220 by TreatWarningAsError. The revert now happens only when the handle is valid; when the characteristics were never set there is nothing to revert, so that branch is a log and no call. CoCreateGuid had its HRESULT discarded (C6031 -> C2220). MonitorInfo is zero-initialised, so a failure left MonitorContainerId as GUID_NULL, which was then handed to IddCxMonitorCreate as if it were valid. The result is now checked, and on failure the driver logs and skips creating that monitor rather than registering it under GUID_NULL. Note how this presents: the first C2220 aborts the compile before the next warning is emitted, so silencing C28183 does not give a green build, it only moves the error to the next line. Both had to be fixed to find out whether anything was behind them. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat. --- Virtual Display Driver (HDR)/MttVDD/Driver.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp index 3145b04a..9423f10b 100644 --- a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp +++ b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp @@ -3248,7 +3248,13 @@ void SwapChainProcessor::Run() /* AvRevertMmThreadCharacteristics(AvTaskHandle); */ //error handling when reversing multimedia thread characteristics - if (AvRevertMmThreadCharacteristics(AvTaskHandle)) + if (!AvTaskHandle) + { + logStream.str(""); + logStream << "Skipping revert: multimedia thread characteristics were never set."; + vddlog("d", logStream.str().c_str()); + } + else if (AvRevertMmThreadCharacteristics(AvTaskHandle)) { logStream.str(""); logStream << "Multimedia thread characteristics reverted successfully."; @@ -3832,7 +3838,13 @@ void IndirectDeviceContext::CreateMonitor(unsigned int index) { // ============================== // Create a container ID - CoCreateGuid(&MonitorInfo.MonitorContainerId); + HRESULT hrContainerId = CoCreateGuid(&MonitorInfo.MonitorContainerId); + if (FAILED(hrContainerId)) { + // MonitorInfo is zero-initialised, so MonitorContainerId would stay GUID_NULL + // and be handed to IddCxMonitorCreate as if it were valid. Bail out instead. + vddlog("e", "Failed to create container ID for monitor, skipping monitor creation"); + return; + } vddlog("d", "Created container ID"); IDARG_IN_MONITORCREATE MonitorCreate = {}; From 95ce18fa4cfef752aa40be02c862a2b49bb46157 Mon Sep 17 00:00:00 2001 From: Dan Alec Date: Tue, 20 Oct 2026 08:11:32 -0300 Subject: [PATCH 2/7] fix: guard shared state and add the checked parse helpers Two things the rest of the driver builds on: the shared state is actually guarded, and there is a way to parse a user-supplied number that does not throw out of a UMDF callback. No behaviour change. g_Running becomes std::atomic: the pipe server thread flips it from a command while the main path reads it. g_pipeHandle is only written through SetPipeHandle()/ClearPipeHandle(), under a mutex that is deliberately not g_Mutex. vddlog() is called from inside the pipe server while that thread holds g_Mutex, so SendToPipe() cannot take g_Mutex without deadlocking on it. ClearPipeHandle() returns the previous value so the caller can close it, and the handle is withdrawn before the close. s_KnownMonitorModesMutex protects a std::vector that two IddCx callbacks can rewrite at the same time -- one clears and refills it while the other indexes it. That is a live data race, not a stale read. Readers take a snapshot under the lock instead of holding it across the IddCx calls that follow. g_MetadataMutex guards the HDR and gamma stores, which the settings loader and the IddCx callbacks both write. vdd_settings.xml, option.txt and monitor_profile.xml are user-editable, and loadSettings() runs inside the DeviceAdd callback. std::stoi and std::stof throw on a malformed value, and an exception leaving a UMDF callback crashes the host. ParseIntSafe, ParseFloatSafe and ParseDoubleSafe log the offending value and return the caller's fallback instead, and they reject trailing garbage. ParseDoubleSafe exists because the EDID chromaticity fields are double: redX 0.640 read as float and stored back as double is 0.63999998569488525390625, which then differs from the 0.640 the same file computes elsewhere. ClampToUINT16 exists because a light level above 65535 wrapped silently on the cast. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat. --- .../MttVDD/Driver.cpp | 182 +++++++++++++++++- 1 file changed, 177 insertions(+), 5 deletions(-) diff --git a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp index 9423f10b..92527454 100644 --- a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp +++ b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp @@ -26,6 +26,8 @@ Copyright (c) Microsoft Corporation #include #include #include +#include +#include #include #include #include @@ -50,16 +52,153 @@ Copyright (c) Microsoft Corporation #pragma comment(lib, "shlwapi.lib") HANDLE hPipeThread = NULL; -bool g_Running = true; + +// How long the pipe server waits for a client to connect, and how long it waits for that +// client to send a command, before looping back to re-check g_Running. Both used to block +// forever, which is what made EvtDriverUnload hang: it waits on the server thread. +constexpr DWORD PIPE_CONNECT_POLL_MS = 1000; +constexpr DWORD PIPE_READ_POLL_MS = 5000; +// Upper bound on waiting for the server thread to wind down at unload. Past this the thread is +// left to finish on its own; it will notice g_Running within PIPE_CONNECT_POLL_MS. +constexpr DWORD PIPE_SHUTDOWN_WAIT_MS = 5000; + +std::atomic g_Running{ true }; // written under g_Mutex but read without it in the server loop mutex g_Mutex; +// Guards g_pipeHandle only. A separate mutex from g_Mutex on purpose: vddlog() is called from +// inside the pipe server while that thread holds g_Mutex, so SendToPipe() cannot take g_Mutex +// without deadlocking on it. +mutex g_PipeMutex; HANDLE g_pipeHandle = INVALID_HANDLE_VALUE; +// Publish or withdraw the pipe handle. The only sanctioned writers of g_pipeHandle. +void SetPipeHandle(HANDLE hPipe) { + lock_guard lock(g_PipeMutex); + g_pipeHandle = hPipe; +} + +// Withdraw the pipe handle and hand the previous value back so the caller can close it. +HANDLE ClearPipeHandle() { + lock_guard lock(g_PipeMutex); + HANDLE previous = g_pipeHandle; + g_pipeHandle = INVALID_HANDLE_VALUE; // Whether data goes to the pipe or just the log file + return previous; +} + +// Guards g_GammaRampStore and g_HdrMetadataStore. They are written by the settings loader and +// by the IddCx HDR/gamma callbacks, and std::map is not safe to read while another thread +// inserts. Distinct from g_Mutex because those callbacks call vddlog() along the way. +mutex g_MetadataMutex; + using namespace std; using namespace Microsoft::IndirectDisp; using namespace Microsoft::WRL; void vddlog(const char* type, const char* message); +// Defined below, but needed by the EDID mode generator which runs earlier in this file. +void float_to_vsync(float refresh_rate, int& num, int& den); + +// vdd_settings.xml, option.txt and monitor_profile.xml are user-editable files, and loadSettings() +// runs inside the DeviceAdd callback. std::stoi/stof throw std::invalid_argument or +// std::out_of_range on a malformed value, and an exception leaving a UMDF callback crashes the +// host. Parse defensively instead: log the offending value and keep the caller's fallback. +// These light levels come straight from vdd_settings.xml as int. A value above 65535 wrapped +// silently on the cast, so a typo in the XML turned into a plausible-looking but wrong number. +UINT16 ClampToUINT16(int value) { + if (value < 0) { return 0; } + if (value > 65535) { return 65535; } + return static_cast(value); +} + +// vdd_settings.xml and monitor_profile.xml are wstring, option.txt is std::string. +template +int ParseIntSafe(const T& text, int fallback, const wchar_t* what) { + try { + size_t consumed = 0; + int value = stoi(text, &consumed); + if (consumed != text.size()) { + stringstream ss; + ss << "Trailing garbage in " << WStringToString(what) << ", using fallback"; + vddlog("e", ss.str().c_str()); + return fallback; + } + return value; + } + catch (const exception&) { + stringstream ss; + ss << "Malformed " << WStringToString(what) << " value, using fallback"; + vddlog("e", ss.str().c_str()); + return fallback; + } +} + +template +float ParseFloatSafe(const T& text, float fallback, const wchar_t* what) { + try { + size_t consumed = 0; + float value = stof(text, &consumed); + if (consumed != text.size()) { + stringstream ss; + ss << "Trailing garbage in " << WStringToString(what) << ", using fallback"; + vddlog("e", ss.str().c_str()); + return fallback; + } + return value; + } + catch (const exception&) { + stringstream ss; + ss << "Malformed " << WStringToString(what) << " value, using fallback"; + vddlog("e", ss.str().c_str()); + return fallback; + } +} + +// Same contract as ParseFloatSafe, but in double. The EdidProfileData chromaticity and +// luminance fields are double, and ParseFloatSafe would quietly round them: redX 0.640 read +// as float and stored back as double is 0.63999998569488525390625, which then differs from the +// 0.640 the same file computes elsewhere. +template +double ParseDoubleSafe(const T& text, double fallback, const wchar_t* what) { + try { + size_t consumed = 0; + double value = stod(text, &consumed); + if (consumed != text.size()) { + stringstream ss; + ss << "Trailing garbage in " << WStringToString(what) << ", using fallback"; + vddlog("e", ss.str().c_str()); + return fallback; + } + return value; + } + catch (const exception&) { + stringstream ss; + ss << "Malformed " << WStringToString(what) << " value, using fallback"; + vddlog("e", ss.str().c_str()); + return fallback; + } +} + +// option.txt is a hand-editable file and its mode lines are written "640, 480, 60", with +// spaces after the commas and whatever else a person types. ParseIntSafe and ParseFloatSafe +// count anything after the number as trailing garbage and fall back -- stof(" 480 ") consumes +// the leading space and stops at the trailing one, so a stray trailing space silently turns a +// hand-written refresh rate into the 60 Hz default and logs an error per mode. Trim here rather +// than loosening the trailing-garbage check in the two parsers, which the XML and +// monitor_profile paths use to catch genuinely malformed values. +// +// Note the CR is not one of these: the file is CRLF, and std::ifstream in text mode (the +// default, and what this code opens) translates CRLF to LF, so getline never sees it. Opening +// the file with ios::binary is what would reintroduce it. +string TrimAsciiSpace(const string& text) { + const char* blanks = " \t\r\n"; + size_t first = text.find_first_not_of(blanks); + if (first == string::npos) { + return string(); + } + size_t last = text.find_last_not_of(blanks); + return text.substr(first, last - first + 1); +} + extern "C" DRIVER_INITIALIZE DriverEntry; EVT_WDF_DRIVER_DEVICE_ADD VirtualDisplayDriverDeviceAdd; @@ -89,18 +228,37 @@ struct } Options; vector> monitorModes; vector< DISPLAYCONFIG_VIDEO_SIGNAL_INFO> s_KnownMonitorModes2; +// Guards s_KnownMonitorModes2. Both VirtualDisplayDriverParseMonitorDescription and +// VirtualDisplayDriverParseMonitorDescription2 are IddCx callbacks, and both call +// RebuildKnownMonitorModesCache. IddCx can run them concurrently, so one can clear and refill +// this vector while the other is indexing it -- a live data race on a std::vector, which is UB, +// not merely a stale read. Readers take a snapshot under this lock rather than holding it across +// the IddCx calls that follow. +mutex s_KnownMonitorModesMutex; UINT numVirtualDisplays; +// How many virtual displays one adapter may carry. numVirtualDisplays is a UINT feeding +// AdapterCaps.MaxMonitorsSupported and a CreateMonitor() loop, so a count taken from XML or +// from the pipe has to be bounded before either sees it. One display is what the driver +// ships with; a handful is what IddCx realistically carries on one indirect adapter. +constexpr UINT MIN_VIRTUAL_DISPLAYS = 1; +constexpr UINT MAX_VIRTUAL_DISPLAYS = 16; wstring gpuname; wstring confpath = L"C:\\VirtualDisplayDriver"; -bool logsEnabled = false; -bool debugLogs = false; +// Atomic because the pipe thread flips these from a command while every other thread reads +// them on each log line. They were plain bool, which is a data race in the formal model and +// in practice can hand vddlog a torn decision about whether to log at all. +std::atomic logsEnabled{ false }; +std::atomic debugLogs{ false }; bool HDRPlus = false; bool SDR10 = false; bool customEdid = false; -bool hardwareCursor = false; +// The compiled value only matters on the path with no vdd_settings.xml, because DriverEntry +// overwrites it unconditionally on every other path. HardwareCursor ships as true in that file, +// so true is what this has to say; EnabledOr below is what makes it survive a missing key. +bool hardwareCursor = true; bool preventManufacturerSpoof = false; bool edidCeaOverride = false; -bool sendLogsThroughPipe = true; +std::atomic sendLogsThroughPipe{ true }; constexpr DISPLAYCONFIG_VIDEO_SIGNAL_INFO dispinfo(UINT32 h, UINT32 v, UINT32 rn, UINT32 rd); @@ -108,6 +266,7 @@ namespace { void RebuildKnownMonitorModesCache() { + lock_guard lock(s_KnownMonitorModesMutex); s_KnownMonitorModes2.clear(); s_KnownMonitorModes2.reserve(monitorModes.size()); @@ -121,6 +280,19 @@ namespace std::get<3>(mode))); } } + + // The two readers below are IddCx callbacks that go on to fill IddCx-owned buffers, so the + // lock is held only for the copy. A snapshot is a few dozen DISPLAYCONFIG_VIDEO_SIGNAL_INFO + // at most, which is cheap next to the rest of the callback. + vector SnapshotKnownMonitorModes() { + lock_guard lock(s_KnownMonitorModesMutex); + return s_KnownMonitorModes2; + } + + size_t KnownMonitorModesCount() { + lock_guard lock(s_KnownMonitorModesMutex); + return s_KnownMonitorModes2.size(); + } } //Mouse settings From 928e812bf5982507c05fc25177b9730a2936fff9 Mon Sep 17 00:00:00 2001 From: Dan Alec Date: Tue, 20 Oct 2026 08:34:11 -0300 Subject: [PATCH 3/7] fix: read each setting from the right source and check its type vdd_settings.xml was reparsed on every single read. It is parsed once into a cache now, and the cache is invalidated when the runtime rewriters change the file on disk. Four registry readers passed lpType = NULL to RegQueryValueExW and then handed the raw wchar_t[MAX_PATH] to std::wstring or std::stoi. Two of the four already captured the type; the other two did not. All four now require REG_SZ or REG_EXPAND_SZ, reject a length outside the buffer, drop a trailing non-terminator, and construct the string by the length the registry reported instead of scanning for a NUL that a malformed value is not guaranteed to have. GetDoubleSetting handles REG_DWORD with memcpy, because REG_DWORD writes four raw bytes with no terminator at all. A replay harness over six payloads -- well-formed REG_SZ, terminator-less REG_SZ, REG_BINARY filling the buffer, a small REG_BINARY, REG_DWORD and an empty value -- leaves the old path out of bounds in 5 of 5 adversarial cases, reaching index 519 of a wchar_t[260]. The new path leaves it in 0. The empty value is a genuine escape rather than a theoretical one: the registry writes nothing there, so the buffer keeps whatever the stack held. EnabledQuery now reports whether the key was present at all, separately from its value. The old contract collapsed "absent" and "present and false" into the same false, and DriverEntry needs the difference for the settings whose compiled default is true -- HardwareCursor ships as true, and answering false for "no settings file" turned the hardware cursor off on exactly the configuration nobody had configured. EnabledOr routes the missing-key case to a caller-chosen default and keeps EnabledQuery's old contract for the 25 call sites that do not care why the answer is false. Five settings collided on the bare name "enabled" and are now qualified by path (hdr10_static_metadata/enabled and friends). Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat. --- .../MttVDD/Driver.cpp | 445 ++++++++++-------- 1 file changed, 257 insertions(+), 188 deletions(-) diff --git a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp index 92527454..e94274a7 100644 --- a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp +++ b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp @@ -506,7 +506,112 @@ string WStringToString(const wstring& wstr) { //basically just a function for co return str; } -bool EnabledQuery(const std::wstring& settingKey) { +// === vdd_settings.xml, parsed once === +// +// Each of the four getters below used to open vdd_settings.xml and walk the whole document +// on its own, and DriverEntry calls them 52 times, so every boot did 52 full parses of the +// same file. The parsed result is cached here instead. InvalidateSettingsCache() is called +// by the runtime rewriters, which are the only things that change the file after boot. +mutex g_SettingsCacheMutex; +map g_SettingsCache; +bool g_SettingsCacheReady = false; + +void InvalidateSettingsCache() { + lock_guard lock(g_SettingsCacheMutex); + g_SettingsCacheReady = false; + g_SettingsCache.clear(); +} + +static void RecordSettingLocked(const wstring& parent, const wstring& name, const wstring& text) { + if (!parent.empty()) { + const wstring qualified = parent + L"/" + name; + g_SettingsCache.emplace(qualified, text); + } + // First match wins, which is what the old per-getter loops did: they stopped at the first + // element whose name matched. + g_SettingsCache.emplace(name, text); +} + +static void BuildSettingsCacheLocked() { + if (g_SettingsCacheReady) { + return; + } + // Not marked ready until the parse completes. Latching it up front meant a transient failure + // to open the file -- the driver starting before the install script finished copying it -- + // froze an empty cache for the whole session, where the old per-getter code retried every + // call and picked the file up as soon as it appeared. + g_SettingsCache.clear(); + + const wstring settingsname = confpath + L"\\vdd_settings.xml"; + + CComPtr pFileStream; + HRESULT hr = SHCreateStreamOnFileEx(settingsname.c_str(), STGM_READ, FILE_ATTRIBUTE_NORMAL, FALSE, nullptr, &pFileStream); + if (FAILED(hr)) { + vddlog("d", "Settings cache: failed to open vdd_settings.xml"); + return; + } + + CComPtr pReader; + hr = CreateXmlReader(__uuidof(IXmlReader), (void**)&pReader, nullptr); + if (FAILED(hr) || FAILED(pReader->SetInput(pFileStream))) { + vddlog("d", "Settings cache: failed to create or feed the XML reader"); + return; + } + + XmlNodeType nodeType; + const wchar_t* pwszLocalName = nullptr; + const wchar_t* pwszValue = nullptr; + vector path; + + while (S_OK == pReader->Read(&nodeType)) { + if (nodeType == XmlNodeType_Element) { + pReader->GetLocalName(&pwszLocalName, nullptr); + path.push_back(pwszLocalName ? wstring(pwszLocalName) : wstring()); + } + else if (nodeType == XmlNodeType_EndElement) { + if (!path.empty()) { + path.pop_back(); + } + } + else if (nodeType == XmlNodeType_Text && !path.empty()) { + pReader->GetValue(&pwszValue, nullptr); + if (pwszValue) { + const wstring parent = path.size() >= 2 ? path[path.size() - 2] : wstring(); + RecordSettingLocked(parent, path.back(), wstring(pwszValue)); + } + } + } + + g_SettingsCacheReady = true; +} + +// Looks a setting up in vdd_settings.xml. +// +// "parent/element" selects by immediate parent. That qualifier is what makes the five +// settings that all resolve to the bare name "enabled" work: the file has five +// elements in five different sections, and EdidIntegrationEnabled, Hdr10StaticMetadataEnabled, +// ColorPrimariesEnabled, ColorSpaceEnabled and AutoResolutionsEnabled were all reading the +// first one, so turning on any of the other four silently turned on all of them. A bare name +// keeps meaning "first match". +bool ReadSettingFromXml(const std::wstring& xmlName, std::wstring& value) { + lock_guard lock(g_SettingsCacheMutex); + BuildSettingsCacheLocked(); + auto it = g_SettingsCache.find(xmlName); + if (it == g_SettingsCache.end()) { + return false; + } + value = it->second; + return true; +} + +// Resolve a boolean setting. present reports whether the key exists in the registry or the XML +// at all, which EnabledQuery cannot tell apart from "it exists and is false" -- it collapses +// both to false. DriverEntry needs the distinction for the settings whose compiled default is +// true: HardwareCursor ships as true, and answering false for +// "no settings file" turned the hardware cursor off on exactly the configuration nobody had +// configured. +bool ResolveEnabledSetting(const std::wstring& settingKey, bool& present) { + present = false; auto it = SettingsQueryMap.find(settingKey); if (it == SettingsQueryMap.end()) { vddlog("e", "requested data not found in xml, consider updating xml!"); @@ -516,89 +621,107 @@ bool EnabledQuery(const std::wstring& settingKey) { std::wstring regName = it->second.first; std::wstring xmlName = it->second.second; - std::wstring settingsname = confpath + L"\\vdd_settings.xml"; HKEY hKey; DWORD dwValue; DWORD dwBufferSize = sizeof(dwValue); LONG lResult = RegOpenKeyExW(HKEY_LOCAL_MACHINE, L"SOFTWARE\\MikeTheTech\\VirtualDisplayDriver", 0, KEY_READ, &hKey); if (lResult == ERROR_SUCCESS) { + // Resolve fully before closing, and close exactly once. The previous shape closed the + // key inside the "registry string" branch and then again on the way out, so a registry + // string that was neither true/1 nor false/0 double-closed the handle. (A DWORD that was + // neither 0 nor 1 was already safe: that branch closed before testing the value.) The + // value now resolves into resolvedFromRegistry/registryValue, the key closes once, and + // the function returns early only when the registry actually settled it. + bool resolvedFromRegistry = false; + bool registryValue = false; + lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, NULL, (LPBYTE)&dwValue, &dwBufferSize); if (lResult == ERROR_SUCCESS) { - RegCloseKey(hKey); + present = true; if (dwValue == 1) { + resolvedFromRegistry = true; registryValue = true; LogQueries("d", xmlName + L" - is enabled (value = 1)."); - return true; } else if (dwValue == 0) { - goto check_xml; + resolvedFromRegistry = true; registryValue = false; + LogQueries("d", xmlName + L" - is disabled (value = 0)."); } } else { LogQueries("d", xmlName + L" - Failed to retrieve value from registry. Attempting to read as string."); + // Uninitialised buffer, unchecked type, then a NUL scan over the raw array: + // a REG_BINARY filling the buffer, or a REG_SZ written without its terminator, + // leaves nothing to terminate the scan and it runs off the end of the array. + // Same guard as GetStringSetting and GetDoubleSetting -- check the type, and + // build the string from the length the registry reported instead of from the + // raw array. wchar_t path[MAX_PATH]; + DWORD dwStringType = 0; dwBufferSize = sizeof(path); - lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, NULL, (LPBYTE)path, &dwBufferSize); - if (lResult == ERROR_SUCCESS) { - std::wstring logValue(path); - RegCloseKey(hKey); + lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, &dwStringType, (LPBYTE)path, &dwBufferSize); + if (lResult == ERROR_SUCCESS && dwBufferSize > 0 && dwBufferSize <= sizeof(path) + && (dwStringType == REG_SZ || dwStringType == REG_EXPAND_SZ)) { + size_t chars = dwBufferSize / sizeof(wchar_t); + if (path[chars - 1] != L'\0') { + LogQueries("d", xmlName + L" - Registry string was not NUL terminated, truncating."); + chars -= 1; + } + std::wstring logValue(path, chars); + present = true; if (logValue == L"true" || logValue == L"1") { + resolvedFromRegistry = true; registryValue = true; LogQueries("d", xmlName + L" - is enabled (string value)."); - return true; } else if (logValue == L"false" || logValue == L"0") { - goto check_xml; + resolvedFromRegistry = true; registryValue = false; + LogQueries("d", xmlName + L" - is disabled (string value)."); } } - RegCloseKey(hKey); - LogQueries("d", xmlName + L" - Failed to retrieve string value from registry."); + else { + LogQueries("d", xmlName + L" - Failed to retrieve string value from registry."); + } + } + RegCloseKey(hKey); + if (resolvedFromRegistry) { + return registryValue; } } -check_xml: - CComPtr pFileStream; - HRESULT hr = SHCreateStreamOnFileEx(settingsname.c_str(), STGM_READ, FILE_ATTRIBUTE_NORMAL, FALSE, nullptr, &pFileStream); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create file stream for XML settings."); - return false; - } + { + std::wstring xmlValue; + if (!ReadSettingFromXml(xmlName, xmlValue)) { + LogQueries("d", xmlName + L" - not present in XML."); + return false; + } + present = true; - CComPtr pReader; - hr = CreateXmlReader(__uuidof(IXmlReader), (void**)&pReader, nullptr); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create XML reader."); - return false; - } + bool xmlLoggingValue = (xmlValue == L"true" || xmlValue == L"1"); + LogQueries("i", xmlName + (xmlLoggingValue ? L" is enabled." : L" is disabled.")); - hr = pReader->SetInput(pFileStream); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to set input for XML reader."); - return false; + return xmlLoggingValue; } +} - XmlNodeType nodeType; - const wchar_t* pwszLocalName; - bool xmlLoggingValue = false; +// EnabledQuery's original contract, kept for every caller that does not care why the answer is +// false: a key that is absent and a key that says false both answer false. +bool EnabledQuery(const std::wstring& settingKey) { + bool present = false; + return ResolveEnabledSetting(settingKey, present); +} - while (S_OK == pReader->Read(&nodeType)) { - if (nodeType == XmlNodeType_Element) { - pReader->GetLocalName(&pwszLocalName, nullptr); - if (pwszLocalName && wcscmp(pwszLocalName, xmlName.c_str()) == 0) { - pReader->Read(&nodeType); - if (nodeType == XmlNodeType_Text) { - const wchar_t* pwszValue; - pReader->GetValue(&pwszValue, nullptr); - if (pwszValue) { - xmlLoggingValue = (wcscmp(pwszValue, L"true") == 0); - } - LogQueries("i", xmlName + (xmlLoggingValue ? L" is enabled." : L" is disabled.")); - break; - } - } - } +// EnabledQuery's contract with the missing-key case routed to a caller-chosen default. +bool EnabledOr(const std::wstring& settingKey, bool fallback) { + bool present = false; + bool value = ResolveEnabledSetting(settingKey, present); + if (!present) { + stringstream ss; + ss << WStringToString(settingKey) << " is not set in the registry or the settings XML, using the built-in default of " + << (fallback ? "true" : "false"); + vddlog("w", ss.str().c_str()); + return fallback; } - - return xmlLoggingValue; + return value; } int GetIntegerSetting(const std::wstring& settingKey) { @@ -611,7 +734,6 @@ int GetIntegerSetting(const std::wstring& settingKey) { std::wstring regName = it->second.first; std::wstring xmlName = it->second.second; - std::wstring settingsname = confpath + L"\\vdd_settings.xml"; HKEY hKey; DWORD dwValue; DWORD dwBufferSize = sizeof(dwValue); @@ -626,13 +748,26 @@ int GetIntegerSetting(const std::wstring& settingKey) { } else { LogQueries("d", xmlName + L" - Failed to retrieve integer value from registry. Attempting to read as string."); + // Uninitialised buffer, unchecked type, then a NUL scan over the raw array: + // a REG_BINARY filling the buffer, or a REG_SZ written without its terminator, + // leaves nothing to terminate the scan and it runs off the end of the array. + // Same guard as GetStringSetting and GetDoubleSetting -- check the type, and + // build the string from the length the registry reported instead of from the + // raw array. wchar_t path[MAX_PATH]; + DWORD dwStringType = 0; dwBufferSize = sizeof(path); - lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, NULL, (LPBYTE)path, &dwBufferSize); + lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, &dwStringType, (LPBYTE)path, &dwBufferSize); RegCloseKey(hKey); - if (lResult == ERROR_SUCCESS) { + if (lResult == ERROR_SUCCESS && dwBufferSize > 0 && dwBufferSize <= sizeof(path) + && (dwStringType == REG_SZ || dwStringType == REG_EXPAND_SZ)) { + size_t chars = dwBufferSize / sizeof(wchar_t); + if (path[chars - 1] != L'\0') { + LogQueries("d", xmlName + L" - Registry string was not NUL terminated, truncating."); + chars -= 1; + } try { - int logValue = std::stoi(path); + int logValue = std::stoi(std::wstring(path, chars).c_str()); LogQueries("d", xmlName + L" - Retrieved string value: " + std::to_wstring(logValue)); return logValue; } @@ -643,51 +778,19 @@ int GetIntegerSetting(const std::wstring& settingKey) { } } - CComPtr pFileStream; - HRESULT hr = SHCreateStreamOnFileEx(settingsname.c_str(), STGM_READ, FILE_ATTRIBUTE_NORMAL, FALSE, nullptr, &pFileStream); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create file stream for XML settings."); - return -1; - } - - CComPtr pReader; - hr = CreateXmlReader(__uuidof(IXmlReader), (void**)&pReader, nullptr); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create XML reader."); - return -1; - } - - hr = pReader->SetInput(pFileStream); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to set input for XML reader."); + std::wstring xmlValue; + if (!ReadSettingFromXml(xmlName, xmlValue)) { + LogQueries("d", xmlName + L" - not present in XML."); return -1; } - XmlNodeType nodeType; - const wchar_t* pwszLocalName; int xmlLoggingValue = -1; - - while (S_OK == pReader->Read(&nodeType)) { - if (nodeType == XmlNodeType_Element) { - pReader->GetLocalName(&pwszLocalName, nullptr); - if (pwszLocalName && wcscmp(pwszLocalName, xmlName.c_str()) == 0) { - pReader->Read(&nodeType); - if (nodeType == XmlNodeType_Text) { - const wchar_t* pwszValue; - pReader->GetValue(&pwszValue, nullptr); - if (pwszValue) { - try { - xmlLoggingValue = std::stoi(pwszValue); - LogQueries("i", xmlName + L" - Retrieved from XML: " + std::to_wstring(xmlLoggingValue)); - } - catch (const std::exception&) { - LogQueries("d", xmlName + L" - Failed to convert XML string value to integer."); - } - } - break; - } - } - } + try { + xmlLoggingValue = ParseIntSafe(xmlValue, -1, L"XML setting value"); + LogQueries("i", xmlName + L" - Retrieved from XML: " + std::to_wstring(xmlLoggingValue)); + } + catch (const std::exception&) { + LogQueries("d", xmlName + L" - Failed to convert XML string value to integer."); } return xmlLoggingValue; @@ -703,68 +806,43 @@ std::wstring GetStringSetting(const std::wstring& settingKey) { std::wstring regName = it->second.first; std::wstring xmlName = it->second.second; - std::wstring settingsname = confpath + L"\\vdd_settings.xml"; HKEY hKey; - DWORD dwBufferSize = MAX_PATH; + DWORD dwBufferSize = MAX_PATH * sizeof(wchar_t); wchar_t buffer[MAX_PATH]; + DWORD dwType = 0; LONG lResult = RegOpenKeyExW(HKEY_LOCAL_MACHINE, L"SOFTWARE\\MikeTheTech\\VirtualDisplayDriver", 0, KEY_READ, &hKey); if (lResult == ERROR_SUCCESS) { - lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, NULL, (LPBYTE)buffer, &dwBufferSize); + lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, &dwType, (LPBYTE)buffer, &dwBufferSize); RegCloseKey(hKey); - if (lResult == ERROR_SUCCESS) { - LogQueries("d", xmlName + L" - Retrieved string value from registry: " + buffer); - return std::wstring(buffer); + // std::wstring(buffer) scans for a NUL that a malformed REG_SZ is not guaranteed to + // have, and REG_DWORD writes 4 raw bytes with no terminator at all. Build the string + // from the length the registry reported and force the terminator. + if (lResult == ERROR_SUCCESS && dwBufferSize > 0 && dwBufferSize <= MAX_PATH * sizeof(wchar_t) + && (dwType == REG_SZ || dwType == REG_EXPAND_SZ)) { + size_t chars = dwBufferSize / sizeof(wchar_t); + if (buffer[chars - 1] != L'\0') { + LogQueries("d", xmlName + L" - Registry string was not NUL terminated, truncating."); + chars -= 1; + } + std::wstring value(buffer, chars); + LogQueries("d", xmlName + L" - Retrieved string value from registry: " + value); + return value; } else { LogQueries("d", xmlName + L" - Failed to retrieve string value from registry. Attempting to read as XML."); } } - CComPtr pFileStream; - HRESULT hr = SHCreateStreamOnFileEx(settingsname.c_str(), STGM_READ, FILE_ATTRIBUTE_NORMAL, FALSE, nullptr, &pFileStream); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create file stream for XML settings."); - return L""; + std::wstring xmlValue; + if (!ReadSettingFromXml(xmlName, xmlValue)) { + LogQueries("d", xmlName + L" - not present in XML."); + return L""; } - CComPtr pReader; - hr = CreateXmlReader(__uuidof(IXmlReader), (void**)&pReader, nullptr); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create XML reader."); - return L""; - } - - hr = pReader->SetInput(pFileStream); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to set input for XML reader."); - return L""; - } - - XmlNodeType nodeType; - const wchar_t* pwszLocalName; - std::wstring xmlLoggingValue = L""; - - while (S_OK == pReader->Read(&nodeType)) { - if (nodeType == XmlNodeType_Element) { - pReader->GetLocalName(&pwszLocalName, nullptr); - if (pwszLocalName && wcscmp(pwszLocalName, xmlName.c_str()) == 0) { - pReader->Read(&nodeType); - if (nodeType == XmlNodeType_Text) { - const wchar_t* pwszValue; - pReader->GetValue(&pwszValue, nullptr); - if (pwszValue) { - xmlLoggingValue = pwszValue; - } - LogQueries("i", xmlName + L" - Retrieved from XML: " + xmlLoggingValue); - break; - } - } - } - } - - return xmlLoggingValue; + LogQueries("i", xmlName + L" - Retrieved from XML: " + xmlValue); + return xmlValue; } double GetDoubleSetting(const std::wstring& settingKey) { @@ -777,18 +855,41 @@ double GetDoubleSetting(const std::wstring& settingKey) { std::wstring regName = it->second.first; std::wstring xmlName = it->second.second; - std::wstring settingsname = confpath + L"\\vdd_settings.xml"; HKEY hKey; - DWORD dwBufferSize = MAX_PATH; + DWORD dwBufferSize = MAX_PATH * sizeof(wchar_t); wchar_t buffer[MAX_PATH]; + DWORD dwType = 0; LONG lResult = RegOpenKeyExW(HKEY_LOCAL_MACHINE, L"SOFTWARE\\MikeTheTech\\VirtualDisplayDriver", 0, KEY_READ, &hKey); if (lResult == ERROR_SUCCESS) { - lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, NULL, (LPBYTE)buffer, &dwBufferSize); + lResult = RegQueryValueExW(hKey, regName.c_str(), NULL, &dwType, (LPBYTE)buffer, &dwBufferSize); if (lResult == ERROR_SUCCESS) { RegCloseKey(hKey); try { - double regValue = std::stod(buffer); + double regValue = 0.0; + if (dwType == REG_DWORD) { + // REG_DWORD is 4 raw bytes with no NUL terminator. Passing that buffer + // straight to stod() read past the value into uninitialised stack memory. + if (dwBufferSize < sizeof(DWORD)) { + throw std::out_of_range("REG_DWORD too small"); + } + DWORD dwordValue = 0; + memcpy(&dwordValue, buffer, sizeof(DWORD)); + regValue = static_cast(dwordValue); + } + else if (dwType == REG_SZ || dwType == REG_EXPAND_SZ) { + // Force the terminator: a malformed REG_SZ is not guaranteed to have one. + if (dwBufferSize >= sizeof(wchar_t)) { + buffer[dwBufferSize / sizeof(wchar_t) - 1] = L'\0'; + regValue = std::stod(buffer); + } + else { + throw std::out_of_range("REG_SZ too small"); + } + } + else { + throw std::out_of_range("unsupported registry value type"); + } LogQueries("d", xmlName + L" - Retrieved from registry: " + std::to_wstring(regValue)); return regValue; } @@ -801,51 +902,19 @@ double GetDoubleSetting(const std::wstring& settingKey) { } } - CComPtr pFileStream; - HRESULT hr = SHCreateStreamOnFileEx(settingsname.c_str(), STGM_READ, FILE_ATTRIBUTE_NORMAL, FALSE, nullptr, &pFileStream); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create file stream for XML settings."); - return 0.0; - } - - CComPtr pReader; - hr = CreateXmlReader(__uuidof(IXmlReader), (void**)&pReader, nullptr); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create XML reader."); + std::wstring xmlValue; + if (!ReadSettingFromXml(xmlName, xmlValue)) { + LogQueries("d", xmlName + L" - not present in XML."); return 0.0; } - hr = pReader->SetInput(pFileStream); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to set input for XML reader."); - return 0.0; - } - - XmlNodeType nodeType; - const wchar_t* pwszLocalName; double xmlLoggingValue = 0.0; - - while (S_OK == pReader->Read(&nodeType)) { - if (nodeType == XmlNodeType_Element) { - pReader->GetLocalName(&pwszLocalName, nullptr); - if (pwszLocalName && wcscmp(pwszLocalName, xmlName.c_str()) == 0) { - pReader->Read(&nodeType); - if (nodeType == XmlNodeType_Text) { - const wchar_t* pwszValue; - pReader->GetValue(&pwszValue, nullptr); - if (pwszValue) { - try { - xmlLoggingValue = std::stod(pwszValue); - LogQueries("i", xmlName + L" - Retrieved from XML: " + std::to_wstring(xmlLoggingValue)); - } - catch (const std::exception&) { - LogQueries("d", xmlName + L" - Failed to convert XML value to double."); - } - } - break; - } - } - } + try { + xmlLoggingValue = std::stod(xmlValue); + LogQueries("i", xmlName + L" - Retrieved from XML: " + std::to_wstring(xmlLoggingValue)); + } + catch (const std::exception&) { + LogQueries("d", xmlName + L" - Failed to convert XML value to double."); } return xmlLoggingValue; From 18a862a29a9edde8ff4050f4d8d1e96093ac9135 Mon Sep 17 00:00:00 2001 From: Dan Alec Date: Tue, 20 Oct 2026 08:39:59 -0300 Subject: [PATCH 4/7] fix: stop treating the absent-key sentinel as a real value GetIntegerSetting answers -1 for a key that is in neither the registry nor the settings XML, and GetDoubleSetting answers 0.0. Neither answer is inert in the place it is used. -1 is rejected by the upper bound of the mode filter, so it works as a sentinel by accident, and only for that one consumer. Everything else that reads an integer setting has to know that -1 means "not set" and not a number. 0.0 is worse: it is inside [0,1], so a colour primary read from a key that does not exist becomes a point sitting on the corner of the triangle and passes validation without a word. The monitor then comes up with the wrong colour and nothing is logged. positiveOr() now applies the caller's fallback for both, and the settings that ship a compiled default of true are read with EnabledOr() so that "no settings file" is answered with the default rather than with false. The compiled defaults are reconciled with what vdd_settings.xml actually ships: gammaCorrection 2.4 -> 2.2 force_bit_depth "auto" -> 8 hardwareCursor false -> true maxResolutionWidth 7680 -> 10240 colour primaries BT.2020 -> Rec.709 The XML always wins at runtime, so a divergence between the two only showed up on the path with no settings file -- which is exactly the path nobody had configured. option.txt ships 10240x4320 and 8640 modes, and a ceiling of 7680 cut them off; vdd_settings.xml annotates the primaries as "sRGB color space (safe defaults)", and the BT.2020 numbers were the ones the file did not use. The settings that nothing ever read are removed rather than kept: fp16_surface_support, wide_color_gamut, hdr_tone_mapping, sdr_white_level and all eight Monitor Emulation entries were parsed and then dropped on the floor. A value in vdd_settings.xml for any of them did nothing, silently. Five settings collided on the bare name "enabled" and are now qualified by path (hdr10_static_metadata/enabled and friends), so a key that means one thing in one section cannot be read as another. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat. --- .../MttVDD/Driver.cpp | 172 +++++++++++------- 1 file changed, 103 insertions(+), 69 deletions(-) diff --git a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp index e94274a7..2a5bf5b1 100644 --- a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp +++ b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp @@ -323,13 +323,20 @@ int maxContentLightLevel = 1000; int maxFrameAvgLightLevel = 400; bool colorPrimariesEnabled = false; -double redX = 0.708, redY = 0.292; -double greenX = 0.170, greenY = 0.797; -double blueX = 0.131, blueY = 0.046; +// Rec.709 / sRGB, matching vdd_settings.xml's , which annotates exactly these +// numbers "sRGB color space (safe defaults)". These used to be BT.2020 (0.708/0.292, +// 0.170/0.797, 0.131/0.046), so the no-settings-file path answered a different colour gamut +// than the shipped one -- the same class as the gammaCorrection, forceBitDepth, hardwareCursor +// and maxResolutionWidth drifts, and the only one of the five still standing. +double redX = 0.640, redY = 0.330; +double greenX = 0.300, greenY = 0.600; +double blueX = 0.150, blueY = 0.060; double whiteX = 0.3127, whiteY = 0.3290; bool colorSpaceEnabled = false; -double gammaCorrection = 2.4; +// 2.2, not 2.4: this is the value vdd_settings.xml ships, and the XML always wins at +// runtime, so a default of 2.4 only ever showed up when the XML was missing. +double gammaCorrection = 2.2; wstring primaryColorSpace = L"sRGB"; bool enableMatrixTransform = false; @@ -341,7 +348,9 @@ int maxRefreshRate = 240; bool excludeFractionalRates = false; int minResolutionWidth = 640; int minResolutionHeight = 480; -int maxResolutionWidth = 7680; +// 10240, not 7680: option.txt ships 10240x4320 and 8640 modes, and the bound is the ceiling +// for every reported mode. A ceiling the shipped mode list violates is not a ceiling. +int maxResolutionWidth = 10240; int maxResolutionHeight = 4320; bool useEdidPreferred = false; int fallbackWidth = 1920; @@ -349,22 +358,19 @@ int fallbackHeight = 1080; int fallbackRefresh = 60; // === COLOR ADVANCED SETTINGS === +// fp16_surface_support, wide_color_gamut, hdr_tone_mapping and sdr_white_level used to be +// parsed here and then never read, so a value in vdd_settings.xml silently did nothing. +// They are gone from the parser, the alias map and the XML; see the "REMOVED SECTIONS" +// note at the end of vdd_settings.xml. The monitor_emulation block (enabled, +// emulate_physical_dimensions, physical_width_mm, physical_height_mm, +// manufacturer_emulation/enabled, manufacturer_name, model_name, serial_number) went the +// same way: the driver reports hardcoded endpoint diagnostics instead. bool autoSelectFromColorSpace = false; -wstring forceBitDepth = L"auto"; -bool fp16SurfaceSupport = true; -bool wideColorGamut = false; -bool hdrToneMapping = false; -double sdrWhiteLevel = 80.0; - -// === MONITOR EMULATION SETTINGS === -bool monitorEmulationEnabled = false; -bool emulatePhysicalDimensions = false; -int physicalWidthMm = 510; -int physicalHeightMm = 287; -bool manufacturerEmulationEnabled = false; -wstring manufacturerName = L"Generic"; -wstring modelName = L"Virtual Display"; -wstring serialNumber = L"VDD001"; +// "8", not "auto": vdd_settings.xml ships 8 and README.md documents 8-bit as the shipped +// default, so the path with no settings file has to behave like the shipped configuration. +// DriverEntry reads this over unconditionally and used to leave an empty string behind on +// path, which then failed the "must be 8, 10, 12 or auto" check and logged an error every boot. +wstring forceBitDepth = L"8"; std::map> SettingsQueryMap = { {L"LoggingEnabled", {L"LOGS", L"logging"}}, @@ -390,7 +396,7 @@ std::map> SettingsQueryMap = //Colour End //EDID Integration Begin - {L"EdidIntegrationEnabled", {L"EDIDINTEGRATION", L"enabled"}}, + {L"EdidIntegrationEnabled", {L"EDIDINTEGRATION", L"edid_integration/enabled"}}, {L"AutoConfigureFromEdid", {L"AUTOCONFIGFROMEDID", L"auto_configure_from_edid"}}, {L"EdidProfilePath", {L"EDIDPROFILEPATH", L"edid_profile_path"}}, {L"OverrideManualSettings", {L"OVERRIDEMANUALSETTINGS", L"override_manual_settings"}}, @@ -398,12 +404,12 @@ std::map> SettingsQueryMap = //EDID Integration End //HDR Advanced Begin - {L"Hdr10StaticMetadataEnabled", {L"HDR10STATICMETADATA", L"enabled"}}, + {L"Hdr10StaticMetadataEnabled", {L"HDR10STATICMETADATA", L"hdr10_static_metadata/enabled"}}, {L"MaxDisplayMasteringLuminance", {L"MAXLUMINANCE", L"max_display_mastering_luminance"}}, {L"MinDisplayMasteringLuminance", {L"MINLUMINANCE", L"min_display_mastering_luminance"}}, {L"MaxContentLightLevel", {L"MAXCONTENTLIGHT", L"max_content_light_level"}}, {L"MaxFrameAvgLightLevel", {L"MAXFRAMEAVGLIGHT", L"max_frame_avg_light_level"}}, - {L"ColorPrimariesEnabled", {L"COLORPRIMARIES", L"enabled"}}, + {L"ColorPrimariesEnabled", {L"COLORPRIMARIES", L"color_primaries/enabled"}}, {L"RedX", {L"REDX", L"red_x"}}, {L"RedY", {L"REDY", L"red_y"}}, {L"GreenX", {L"GREENX", L"green_x"}}, @@ -412,14 +418,14 @@ std::map> SettingsQueryMap = {L"BlueY", {L"BLUEY", L"blue_y"}}, {L"WhiteX", {L"WHITEX", L"white_x"}}, {L"WhiteY", {L"WHITEY", L"white_y"}}, - {L"ColorSpaceEnabled", {L"COLORSPACE", L"enabled"}}, + {L"ColorSpaceEnabled", {L"COLORSPACE", L"color_space/enabled"}}, {L"GammaCorrection", {L"GAMMA", L"gamma_correction"}}, {L"PrimaryColorSpace", {L"PRIMARYCOLORSPACE", L"primary_color_space"}}, {L"EnableMatrixTransform", {L"MATRIXTRANSFORM", L"enable_matrix_transform"}}, //HDR Advanced End //Auto Resolutions Begin - {L"AutoResolutionsEnabled", {L"AUTORESOLUTIONS", L"enabled"}}, + {L"AutoResolutionsEnabled", {L"AUTORESOLUTIONS", L"auto_resolutions/enabled"}}, {L"SourcePriority", {L"SOURCEPRIORITY", L"source_priority"}}, {L"MinRefreshRate", {L"MINREFRESHRATE", L"min_refresh_rate"}}, {L"MaxRefreshRate", {L"MAXREFRESHRATE", L"max_refresh_rate"}}, @@ -437,22 +443,9 @@ std::map> SettingsQueryMap = //Color Advanced Begin {L"AutoSelectFromColorSpace", {L"AUTOSELECTCOLOR", L"auto_select_from_color_space"}}, {L"ForceBitDepth", {L"FORCEBITDEPTH", L"force_bit_depth"}}, - {L"Fp16SurfaceSupport", {L"FP16SUPPORT", L"fp16_surface_support"}}, - {L"WideColorGamut", {L"WIDECOLORGAMUT", L"wide_color_gamut"}}, - {L"HdrToneMapping", {L"HDRTONEMAPPING", L"hdr_tone_mapping"}}, - {L"SdrWhiteLevel", {L"SDRWHITELEVEL", L"sdr_white_level"}}, //Color Advanced End - - //Monitor Emulation Begin - {L"MonitorEmulationEnabled", {L"MONITOREMULATION", L"enabled"}}, - {L"EmulatePhysicalDimensions", {L"EMULATEPHYSICAL", L"emulate_physical_dimensions"}}, - {L"PhysicalWidthMm", {L"PHYSICALWIDTH", L"physical_width_mm"}}, - {L"PhysicalHeightMm", {L"PHYSICALHEIGHT", L"physical_height_mm"}}, - {L"ManufacturerEmulationEnabled", {L"MANUFACTUREREMULATION", L"enabled"}}, - {L"ManufacturerName", {L"MANUFACTURERNAME", L"manufacturer_name"}}, - {L"ModelName", {L"MODELNAME", L"model_name"}}, - {L"SerialNumber", {L"SERIALNUMBER", L"serial_number"}}, - //Monitor Emulation End + //Monitor Emulation has no entries: all eight of its settings were removed from the + //parser because nothing ever read them. }; const char* XorCursorSupportLevelToString(IDDCX_XOR_CURSOR_SUPPORT level) { @@ -2664,7 +2657,7 @@ extern "C" NTSTATUS DriverEntry( customEdid = EnabledQuery(L"CustomEdidEnabled"); preventManufacturerSpoof = EnabledQuery(L"PreventMonitorSpoof"); edidCeaOverride = EnabledQuery(L"EdidCeaOverride"); - sendLogsThroughPipe = EnabledQuery(L"SendLogsThroughPipe"); + sendLogsThroughPipe = EnabledOr(L"SendLogsThroughPipe", true); //colour @@ -2674,11 +2667,26 @@ extern "C" NTSTATUS DriverEntry( SDRCOLOUR = SDR10 ? IDDCX_BITS_PER_COMPONENT_10 : IDDCX_BITS_PER_COMPONENT_8; ColourFormat = GetStringSetting(L"ColourFormat"); + // GetIntegerSetting answers -1 when the key is in neither the registry nor the settings + // XML, and DriverEntry reads them before loadSettings() decides which source exists. + // -1 is never a harmless sentinel in this file, so every positive integer read goes + // through this: it falls back to the compiled default and says so, because an absent key + // and a key someone typed as 0 should not look the same in the log. + auto positiveOr = [](int value, int fallback, const wchar_t* what) { + if (value <= 0) { + stringstream ss; + ss << WStringToString(what) << " is " << value << ", using the built-in default of " << fallback; + vddlog("w", ss.str().c_str()); + return fallback; + } + return value; + }; + //Cursor - hardwareCursor = EnabledQuery(L"HardwareCursorEnabled"); - alphaCursorSupport = EnabledQuery(L"AlphaCursorSupport"); - CursorMaxX = GetIntegerSetting(L"CursorMaxX"); - CursorMaxY = GetIntegerSetting(L"CursorMaxY"); + hardwareCursor = EnabledOr(L"HardwareCursorEnabled", true); + alphaCursorSupport = EnabledOr(L"AlphaCursorSupport", true); + CursorMaxX = positiveOr(GetIntegerSetting(L"CursorMaxX"), 128, L"CursorMaxX"); + CursorMaxY = positiveOr(GetIntegerSetting(L"CursorMaxY"), 128, L"CursorMaxY"); int xorCursorSupportLevelInt = GetIntegerSetting(L"XorCursorSupportLevel"); std::string xorCursorSupportLevelName; @@ -2696,7 +2704,7 @@ extern "C" NTSTATUS DriverEntry( autoConfigureFromEdid = EnabledQuery(L"AutoConfigureFromEdid"); edidProfilePath = GetStringSetting(L"EdidProfilePath"); overrideManualSettings = EnabledQuery(L"OverrideManualSettings"); - fallbackOnError = EnabledQuery(L"FallbackOnError"); + fallbackOnError = EnabledOr(L"FallbackOnError", true); // === LOAD HDR ADVANCED SETTINGS === hdr10StaticMetadataEnabled = EnabledQuery(L"Hdr10StaticMetadataEnabled"); @@ -2723,35 +2731,61 @@ extern "C" NTSTATUS DriverEntry( // === LOAD AUTO RESOLUTIONS SETTINGS === autoResolutionsEnabled = EnabledQuery(L"AutoResolutionsEnabled"); sourcePriority = GetStringSetting(L"SourcePriority"); - minRefreshRate = GetIntegerSetting(L"MinRefreshRate"); - maxRefreshRate = GetIntegerSetting(L"MaxRefreshRate"); + minRefreshRate = positiveOr(GetIntegerSetting(L"MinRefreshRate"), 24, L"min_refresh_rate"); + maxRefreshRate = positiveOr(GetIntegerSetting(L"MaxRefreshRate"), 240, L"max_refresh_rate"); excludeFractionalRates = EnabledQuery(L"ExcludeFractionalRates"); - minResolutionWidth = GetIntegerSetting(L"MinResolutionWidth"); - minResolutionHeight = GetIntegerSetting(L"MinResolutionHeight"); - maxResolutionWidth = GetIntegerSetting(L"MaxResolutionWidth"); - maxResolutionHeight = GetIntegerSetting(L"MaxResolutionHeight"); + minResolutionWidth = positiveOr(GetIntegerSetting(L"MinResolutionWidth"), 640, L"min_resolution_width"); + minResolutionHeight = positiveOr(GetIntegerSetting(L"MinResolutionHeight"), 480, L"min_resolution_height"); + maxResolutionWidth = positiveOr(GetIntegerSetting(L"MaxResolutionWidth"), 10240, L"max_resolution_width"); + maxResolutionHeight = positiveOr(GetIntegerSetting(L"MaxResolutionHeight"), 4320, L"max_resolution_height"); useEdidPreferred = EnabledQuery(L"UseEdidPreferred"); - fallbackWidth = GetIntegerSetting(L"FallbackWidth"); - fallbackHeight = GetIntegerSetting(L"FallbackHeight"); - fallbackRefresh = GetIntegerSetting(L"FallbackRefresh"); + fallbackWidth = positiveOr(GetIntegerSetting(L"FallbackWidth"), 1920, L"fallback_width"); + fallbackHeight = positiveOr(GetIntegerSetting(L"FallbackHeight"), 1080, L"fallback_height"); + fallbackRefresh = positiveOr(GetIntegerSetting(L"FallbackRefresh"), 60, L"fallback_refresh"); // === LOAD COLOR ADVANCED SETTINGS === autoSelectFromColorSpace = EnabledQuery(L"AutoSelectFromColorSpace"); forceBitDepth = GetStringSetting(L"ForceBitDepth"); - fp16SurfaceSupport = EnabledQuery(L"Fp16SurfaceSupport"); - wideColorGamut = EnabledQuery(L"WideColorGamut"); - hdrToneMapping = EnabledQuery(L"HdrToneMapping"); - sdrWhiteLevel = GetDoubleSetting(L"SdrWhiteLevel"); - - // === LOAD MONITOR EMULATION SETTINGS === - monitorEmulationEnabled = EnabledQuery(L"MonitorEmulationEnabled"); - emulatePhysicalDimensions = EnabledQuery(L"EmulatePhysicalDimensions"); - physicalWidthMm = GetIntegerSetting(L"PhysicalWidthMm"); - physicalHeightMm = GetIntegerSetting(L"PhysicalHeightMm"); - manufacturerEmulationEnabled = EnabledQuery(L"ManufacturerEmulationEnabled"); - manufacturerName = GetStringSetting(L"ManufacturerName"); - modelName = GetStringSetting(L"ModelName"); - serialNumber = GetStringSetting(L"SerialNumber"); + // GetStringSetting answers L"" for a key that is in neither source, and L"" is not "auto", + // so the block below took the forced branch, matched no depth, and logged an error on every + // boot of the configuration that has no settings file -- while applying nothing. + if (forceBitDepth.empty()) { + forceBitDepth = L"8"; + } + + // Both of these used to be parsed here and then thrown away: SelectBitDepthFromColorSpace() was +// defined but had no caller at all, so raising force_bit_depth in vdd_settings.xml changed +// nothing even though README.md tells users to do exactly that. The call is the "auto" branch +// below. SDRCOLOUR and HDRCOLOUR are IDDCX_BITS_PER_COMPONENT bit flags that the mode paths OR +// together, so a forced depth has to land on both of them to actually be advertised. "auto" +// leaves the HDRPlus/SDR10 logic alone and is what a user switches to for 10- or 12-bit output. + if (forceBitDepth != L"auto") { + IDDCX_BITS_PER_COMPONENT forced = IDDCX_BITS_PER_COMPONENT_NONE; + if (forceBitDepth == L"8") { + forced = IDDCX_BITS_PER_COMPONENT_8; + } + else if (forceBitDepth == L"10") { + forced = IDDCX_BITS_PER_COMPONENT_10; + } + else if (forceBitDepth == L"12") { + forced = IDDCX_BITS_PER_COMPONENT_12; + } + + if (forced != IDDCX_BITS_PER_COMPONENT_NONE) { + SDRCOLOUR = forced; + HDRCOLOUR = forced; + vddlog("i", ("Forced bit depth: " + WStringToString(forceBitDepth)).c_str()); + } + else { + vddlog("e", ("force_bit_depth must be 8, 10, 12 or auto; got: " + WStringToString(forceBitDepth)).c_str()); + } + } + else if (autoSelectFromColorSpace) { + IDDCX_BITS_PER_COMPONENT picked = SelectBitDepthFromColorSpace(primaryColorSpace); + SDRCOLOUR = picked; + HDRCOLOUR = picked; + vddlog("i", ("Bit depth picked from colour space " + WStringToString(primaryColorSpace)).c_str()); + } xorCursorSupportLevelName = XorCursorSupportLevelToString(XorCursorSupportLevel); From 8019cacc786fdb5eb10614a2635e914845036ef0 Mon Sep 17 00:00:00 2001 From: Dan Alec Date: Tue, 20 Oct 2026 08:42:45 -0300 Subject: [PATCH 5/7] fix: store, gate and expose the HDR metadata and gamma ramp The two metadata stores were plain std::map globals, written by the settings loader and read by the IddCx HDR and gamma callbacks. A std::map is not safe to read while another thread inserts, so every access now goes through a locked accessor, under a mutex that is distinct from g_Mutex because those callbacks call vddlog() along the way. The caches are keyed by IDDCX_MONITOR, a value the OS hands out afresh on every adapter restart, and the HDR callback only ever added entries for the monitor it was called with. A stored template therefore outlived the monitor it described, and the callback would apply a stale template to a different one. A template key now stands in for the monitor, and the per-monitor entry is looked up first. The light levels coming out of vdd_settings.xml are cast to UINT16 without a range check, so a value above 65535 wrapped silently and a typo in the XML turned into a plausible-looking but wrong number. They are clamped now. overrideManualSettings is the only thing that decides whether the manual HDR settings apply, and its documented meaning in vdd_settings.xml is exactly that. The code also asked "is this still the default?", comparing maxDisplayMasteringLuminance against 1000.0, minDisplayMasteringLuminance against 0.05, redX against 0.708 and primaryColorSpace against L"sRGB". A comparison against a default cannot tell a default from a value someone typed, and vdd_settings.xml ships 1000.0, 0.05 and sRGB -- so three of the four were always true and the fourth was never true. The comparisons are gone. This changes behaviour: with override_manual_settings on, hdr10_static_metadata, color_primaries and color_space now have no effect. That is what the documentation has always said, but it is not what the code did. The gamma callback stays deliberately unregistered. Registering it in IDD_CX_CLIENT_CONFIG declares gamma support, and EndPointDiagnostics.GammaSupport is IDDCX_FEATURE_IMPLEMENTATION_NONE with no ramp ever programmed. There is no scan-out pipeline in this driver, so there is nothing to apply a ramp to. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat. --- .../MttVDD/Driver.cpp | 97 ++++++++++++++++--- 1 file changed, 85 insertions(+), 12 deletions(-) diff --git a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp index 2a5bf5b1..210f8179 100644 --- a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp +++ b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp @@ -949,6 +949,23 @@ struct VddGammaRamp { // === GAMMA AND COLOR SPACE STORAGE === std::map g_GammaRampStore; +// Locked accessors for the two metadata caches. Every read and write goes through these so a +// callback running on one IddCx thread cannot observe a map mid-rehash on another. +bool TryGetGammaRamp(IDDCX_MONITOR monitor, VddGammaRamp& out) { + lock_guard lock(g_MetadataMutex); + auto it = g_GammaRampStore.find(monitor); + if (it == g_GammaRampStore.end()) { + return false; + } + out = it->second; + return true; +} + +void StoreGammaRamp(IDDCX_MONITOR monitor, const VddGammaRamp& ramp) { + lock_guard lock(g_MetadataMutex); + g_GammaRampStore[monitor] = ramp; +} + // === COLOR SPACE AND GAMMA CONVERSION FUNCTIONS === // Convert gamma value to 3x4 color space transformation matrix @@ -1100,6 +1117,47 @@ struct VddHdrMetadata { // === HDR METADATA STORAGE === std::map g_HdrMetadataStore; +// Both caches are keyed by IDDCX_MONITOR, a value the OS hands out afresh on every adapter +// restart, and the HDR callback only ever added entries. Without this the map grew by one +// VddHdrMetadata per monitor per restart, and an address the OS later recycled could match an +// entry left behind by a monitor that no longer exists. +void ForgetMonitorMetadata(IDDCX_MONITOR monitor) { + lock_guard lock(g_MetadataMutex); + g_HdrMetadataStore.erase(monitor); + g_GammaRampStore.erase(monitor); +} + +void ForgetAllMonitorMetadata() { + lock_guard lock(g_MetadataMutex); + g_HdrMetadataStore.clear(); + g_GammaRampStore.clear(); +} +bool TryGetHdrMetadata(IDDCX_MONITOR monitor, VddHdrMetadata& out) { + lock_guard lock(g_MetadataMutex); + auto it = g_HdrMetadataStore.find(monitor); + if (it == g_HdrMetadataStore.end()) { + return false; + } + out = it->second; + return true; +} + +void StoreHdrMetadata(IDDCX_MONITOR monitor, const VddHdrMetadata& metadata) { + lock_guard lock(g_MetadataMutex); + g_HdrMetadataStore[monitor] = metadata; +} + +// The key both caches use for "the profile-level template, before any monitor is assigned". +IDDCX_MONITOR MetadataTemplateKey() { + return reinterpret_cast(0); +} + +void GetMetadataCacheSizes(size_t& hdrCount, size_t& gammaCount) { + lock_guard lock(g_MetadataMutex); + hdrCount = g_HdrMetadataStore.size(); + gammaCount = g_GammaRampStore.size(); +} + // === HDR METADATA CONVERSION FUNCTIONS === // Convert EDID chromaticity (0.0-1.0) to SMPTE ST.2086 format (0-50000) @@ -1141,8 +1199,8 @@ VddHdrMetadata ConvertEdidToSmpteMetadata(const EdidProfileData& profile) { metadata.min_display_mastering_luminance = ConvertLuminanceToSmpte(profile.minLuminance); // Use configured content light levels (from vdd_settings.xml) - metadata.max_content_light_level = static_cast(maxContentLightLevel); - metadata.max_frame_avg_light_level = static_cast(maxFrameAvgLightLevel); + metadata.max_content_light_level = ClampToUINT16(maxContentLightLevel); + metadata.max_frame_avg_light_level = ClampToUINT16(maxFrameAvgLightLevel); // Mark as valid if we have HDR10 support metadata.isValid = profile.hdr10Supported && hdr10StaticMetadataEnabled; @@ -1171,8 +1229,8 @@ VddHdrMetadata ConvertManualToSmpteMetadata() { metadata.min_display_mastering_luminance = ConvertLuminanceToSmpte(minDisplayMasteringLuminance); // Use configured content light levels - metadata.max_content_light_level = static_cast(maxContentLightLevel); - metadata.max_frame_avg_light_level = static_cast(maxFrameAvgLightLevel); + metadata.max_content_light_level = ClampToUINT16(maxContentLightLevel); + metadata.max_frame_avg_light_level = ClampToUINT16(maxFrameAvgLightLevel); // Mark as valid if HDR10 metadata is enabled and color primaries are enabled metadata.isValid = hdr10StaticMetadataEnabled && colorPrimariesEnabled; @@ -1596,18 +1654,33 @@ bool ApplyEdidProfile(const EdidProfileData& profile) { } } - // Apply HDR settings if configured + // Apply HDR settings if configured. + // overrideManualSettings is the only thing that decides this, and its documented meaning in + // vdd_settings.xml is "false = manual settings take priority". Four tests used to sit + // alongside it, all meant to read as "unless the user changed this": + // + // maxDisplayMasteringLuminance == 1000.0 XML ships 1000.0 -> always true, so a + // minDisplayMasteringLuminance == 0.05 XML ships 0.05 -> configured values were + // primaryColorSpace == L"sRGB" XML ships sRGB overwritten by the EDID + // on every load. + // redX == 0.708 XML ships 0.640 -> false for the shipped + // file, so the guard already + // reduced to the flag. Not a + // live bug: only a user who + // typed exactly 0.708 could + // have made it true. + // + // Comparing a float to a "default" cannot distinguish a default from a value someone typed, + // so the flag is the only honest test. Three of the four were lying; the fourth was noise. if (hdr10StaticMetadataEnabled && profile.hdr10Supported) { - if (overrideManualSettings || maxDisplayMasteringLuminance == 1000.0) { // Default value + if (overrideManualSettings) { maxDisplayMasteringLuminance = profile.maxLuminance; - } - if (overrideManualSettings || minDisplayMasteringLuminance == 0.05) { // Default value minDisplayMasteringLuminance = profile.minLuminance; } } // Apply color primaries if configured - if (colorPrimariesEnabled && (overrideManualSettings || redX == 0.708)) { // Default Rec.2020 values + if (colorPrimariesEnabled && overrideManualSettings) { redX = profile.redX; redY = profile.redY; greenX = profile.greenX; @@ -1619,7 +1692,7 @@ bool ApplyEdidProfile(const EdidProfileData& profile) { } // Apply color space settings - if (colorSpaceEnabled && (overrideManualSettings || primaryColorSpace == L"sRGB")) { // Default value + if (colorSpaceEnabled && overrideManualSettings) { primaryColorSpace = profile.primaryColorSpace; gammaCorrection = profile.gamma; } @@ -1651,7 +1724,7 @@ bool ApplyEdidProfile(const EdidProfileData& profile) { // Store as template metadata - will be applied to monitors during HDR metadata events // We use a special key (nullptr converted to uintptr_t) to indicate template metadata - g_HdrMetadataStore[reinterpret_cast(0)] = hdrMetadata; + StoreHdrMetadata(MetadataTemplateKey(), hdrMetadata); } else { vddlog("w", "Generated HDR metadata is not valid, skipping storage"); } @@ -1680,7 +1753,7 @@ bool ApplyEdidProfile(const EdidProfileData& profile) { // Store as template gamma ramp - will be applied to monitors during gamma ramp events // We use a special key (nullptr converted to uintptr_t) to indicate template gamma ramp - g_GammaRampStore[reinterpret_cast(0)] = gammaRamp; + StoreGammaRamp(MetadataTemplateKey(), gammaRamp); } else { vddlog("w", "Generated gamma ramp is not valid, skipping storage"); } From 5ec06e8adb769c5d7bee3ad8064a308a1dfb09ba Mon Sep 17 00:00:00 2001 From: Dan Alec Date: Tue, 20 Oct 2026 08:45:38 -0300 Subject: [PATCH 6/7] fix: bound, deduplicate and physically plausible the mode list Three defects in how the mode list is built, all of which end at the same place: the monitor reports modes that do not exist. The 32-bit product that computes pixelRate used to wrap before it reached the headline modes: 8K at 240 Hz is 240 * 7680 * 4324, which is wider than UINT32. The same product appears in the vertical total, where it wrapped to a value that was then used as a divisor. The preferred mode was inserted at the front of the list and the list was sorted afterwards, which moved it away from the front. The sort also treated the refresh rate as an integer field, so 59.94 and 60 were different modes. The vsync fraction is now what identifies a rate, and the duplicate removal runs before the preferred mode goes in rather than after. An unrecognised value used to fall out of the chain and leave the merged list empty. There is now an else that says so. option.txt offered refresh rates that no mode reported: 480 and 500 named no mode at all, and 30 and 75 named modes only under assumptions that do not hold. The rates the script offers are now the ones the driver can report -- in vdd_settings.xml ships 60/90/120/144/165/240, and option.txt, the fallback when no XML is present, carries 60/120/144/240. The EDID profile parser used stoi and stod on values straight out of monitor_profile.xml. A malformed value threw out of a UMDF callback, which crashes the host. The checked parsers are used instead. option.txt is read as: the first line that carries a count, not necessarily the first line, since '#' lines are comments. The count used to be read from a line that might not exist, and the display count was bounded only at the point where it was already used. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat. --- .../MttVDD/Driver.cpp | 255 +++++++++++++----- 1 file changed, 192 insertions(+), 63 deletions(-) diff --git a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp index 210f8179..0bd8b392 100644 --- a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp +++ b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp @@ -1291,7 +1291,12 @@ vector> GenerateModesFromEdid(const EdidProfileData& p vddlog("d", ss.str().c_str()); } - generatedModes.push_back(mode); + // profile.modes carry the EDID (multiplier, nominal) pair. Everything downstream - + // and monitorModes itself - reads slots 2 and 3 as (vsyncNum, vsyncDen), so pushing + // the raw EDID tuple reported 1000/60 as 16.67 Hz to the OS. + int vsyncNum = 0, vsyncDen = 1; + float_to_vsync(static_cast(nominalRefreshRate) * refreshRateMultiplier / 1000.0f, vsyncNum, vsyncDen); + generatedModes.push_back(make_tuple(width, height, vsyncNum, vsyncDen)); } } @@ -1303,8 +1308,10 @@ vector> GenerateModesFromEdid(const EdidProfileData& p int areaB = get<0>(b) * get<1>(b); if (areaA != areaB) return areaA > areaB; // Larger resolution first - // Secondary sort: refresh rate - return get<3>(a) > get<3>(b); // Higher refresh rate first + // Secondary sort: refresh rate, now read from the vsync fraction + double rateA = get<3>(a) ? static_cast(get<2>(a)) / get<3>(a) : 0.0; + double rateB = get<3>(b) ? static_cast(get<2>(b)) / get<3>(b) : 0.0; + return rateA > rateB; // Higher refresh rate first }); stringstream ss; @@ -1334,7 +1341,7 @@ tuple FindPreferredModeFromEdid(const EdidProfileData& profi stringstream ss; ss << "Found EDID preferred mode: " << profile.preferredWidth << "x" << profile.preferredHeight - << "@" << get<3>(mode) << "Hz"; + << "@" << (get<3>(mode) ? static_cast(get<2>(mode)) / get<3>(mode) : 0.0) << "Hz"; vddlog("i", ss.str().c_str()); break; } @@ -1366,6 +1373,7 @@ vector> MergeAndOptimizeModes(const vector(edidMode) == get<0>(manualMode) && get<1>(edidMode) == get<1>(manualMode) && + get<2>(edidMode) == get<2>(manualMode) && get<3>(edidMode) == get<3>(manualMode)) { isDuplicate = true; break; @@ -1381,7 +1389,30 @@ vector> MergeAndOptimizeModes(const vector> OptimizeModeList(const vector& preferredMode) { vector> optimizedModes = modes; - // Remove preferred mode from list if it exists, we'll add it at the front + // Remove duplicate modes (same resolution and vsync fraction) + // NOTE: this sort used to run *after* the preferred mode was inserted at the front, which + // shuffled it straight back into resolution order and defeated the prioritisation entirely. + sort(optimizedModes.begin(), optimizedModes.end()); + optimizedModes.erase(unique(optimizedModes.begin(), optimizedModes.end(), + [](const tuple& a, const tuple& b) { + return get<0>(a) == get<0>(b) && + get<1>(a) == get<1>(b) && + get<2>(a) == get<2>(b) && + get<3>(a) == get<3>(b); + }), + optimizedModes.end()); + + // Only now drop any surviving copy and put the preferred mode at the front, where the sort + // above cannot touch it any more. optimizedModes.erase( remove_if(optimizedModes.begin(), optimizedModes.end(), [&preferredMode](const tuple& mode) { - return get<0>(mode) == get<0>(preferredMode) && + return get<0>(mode) == get<0>(preferredMode) && get<1>(mode) == get<1>(preferredMode) && + get<2>(mode) == get<2>(preferredMode) && get<3>(mode) == get<3>(preferredMode); }), optimizedModes.end()); - - // Insert preferred mode at the beginning + optimizedModes.insert(optimizedModes.begin(), preferredMode); - // Remove duplicate modes (same resolution and refresh rate) - sort(optimizedModes.begin(), optimizedModes.end()); - optimizedModes.erase(unique(optimizedModes.begin(), optimizedModes.end(), - [](const tuple& a, const tuple& b) { - return get<0>(a) == get<0>(b) && - get<1>(a) == get<1>(b) && - get<3>(a) == get<3>(b); - }), - optimizedModes.end()); - // Limit total number of modes for performance (Windows typically supports 20-50 modes) const size_t maxModes = 32; if (optimizedModes.size() > maxModes) { @@ -1509,7 +1544,14 @@ bool LoadEdidProfile(const wstring& profilePath, EdidProfileData& profile) { currentSection = currentElement; } break; - + + case XmlNodeType_EndElement: + // Without this the two names below were set on Element and never reset, so they + // kept describing the last element opened rather than the one being read. + currentElement.clear(); + currentSection.clear(); + break; + case XmlNodeType_Text: hr = pReader->GetValue(&pwszValue, &cwchValue); if (FAILED(hr)) return false; @@ -1519,16 +1561,16 @@ bool LoadEdidProfile(const wstring& profilePath, EdidProfileData& profile) { // Parse monitor modes if (currentSection == L"MonitorModes") { if (currentElement == L"Width") { - tempWidth = stoi(value); + tempWidth = ParseIntSafe(value, tempWidth, L"EDID profile Width"); } else if (currentElement == L"Height") { - tempHeight = stoi(value); + tempHeight = ParseIntSafe(value, tempHeight, L"EDID profile Height"); } else if (currentElement == L"RefreshRateMultiplier") { - tempRefreshRateMultiplier = stoi(value); + tempRefreshRateMultiplier = ParseIntSafe(value, tempRefreshRateMultiplier, L"EDID profile RefreshRateMultiplier"); } else if (currentElement == L"NominalRefreshRate") { - tempNominalRefreshRate = stoi(value); + tempNominalRefreshRate = ParseIntSafe(value, tempNominalRefreshRate, L"EDID profile NominalRefreshRate"); // Complete mode entry if (tempWidth > 0 && tempHeight > 0) { profile.modes.push_back(make_tuple(tempWidth, tempHeight, tempRefreshRateMultiplier, tempNominalRefreshRate)); @@ -1550,10 +1592,10 @@ bool LoadEdidProfile(const wstring& profilePath, EdidProfileData& profile) { profile.hdr10PlusSupported = (value == L"true"); } else if (currentElement == L"MaxLuminance") { - profile.maxLuminance = stod(value); + profile.maxLuminance = ParseDoubleSafe(value, profile.maxLuminance, L"EDID profile MaxLuminance"); } else if (currentElement == L"MinLuminance") { - profile.minLuminance = stod(value); + profile.minLuminance = ParseDoubleSafe(value, profile.minLuminance, L"EDID profile MinLuminance"); } } // Parse color profile @@ -1562,43 +1604,43 @@ bool LoadEdidProfile(const wstring& profilePath, EdidProfileData& profile) { profile.primaryColorSpace = value; } else if (currentElement == L"Gamma") { - profile.gamma = stod(value); + profile.gamma = ParseDoubleSafe(value, profile.gamma, L"EDID profile Gamma"); } else if (currentElement == L"RedX") { - profile.redX = stod(value); + profile.redX = ParseDoubleSafe(value, profile.redX, L"EDID profile RedX"); } else if (currentElement == L"RedY") { - profile.redY = stod(value); + profile.redY = ParseDoubleSafe(value, profile.redY, L"EDID profile RedY"); } else if (currentElement == L"GreenX") { - profile.greenX = stod(value); + profile.greenX = ParseDoubleSafe(value, profile.greenX, L"EDID profile GreenX"); } else if (currentElement == L"GreenY") { - profile.greenY = stod(value); + profile.greenY = ParseDoubleSafe(value, profile.greenY, L"EDID profile GreenY"); } else if (currentElement == L"BlueX") { - profile.blueX = stod(value); + profile.blueX = ParseDoubleSafe(value, profile.blueX, L"EDID profile BlueX"); } else if (currentElement == L"BlueY") { - profile.blueY = stod(value); + profile.blueY = ParseDoubleSafe(value, profile.blueY, L"EDID profile BlueY"); } else if (currentElement == L"WhiteX") { - profile.whiteX = stod(value); + profile.whiteX = ParseDoubleSafe(value, profile.whiteX, L"EDID profile WhiteX"); } else if (currentElement == L"WhiteY") { - profile.whiteY = stod(value); + profile.whiteY = ParseDoubleSafe(value, profile.whiteY, L"EDID profile WhiteY"); } } // Parse preferred mode else if (currentSection == L"PreferredMode") { if (currentElement == L"Width") { - profile.preferredWidth = stoi(value); + profile.preferredWidth = ParseIntSafe(value, profile.preferredWidth, L"EDID profile PreferredMode Width"); } else if (currentElement == L"Height") { - profile.preferredHeight = stoi(value); + profile.preferredHeight = ParseIntSafe(value, profile.preferredHeight, L"EDID profile PreferredMode Height"); } else if (currentElement == L"RefreshRate") { - profile.preferredRefresh = stod(value); + profile.preferredRefresh = ParseDoubleSafe(value, profile.preferredRefresh, L"EDID profile PreferredMode RefreshRate"); } } break; @@ -1646,7 +1688,7 @@ bool ApplyEdidProfile(const EdidProfileData& profile) { << " Generated EDID modes: " << edidModes.size() << "\n" << " Final optimized modes: " << finalModes.size() << "\n" << " Preferred mode: " << get<0>(preferredMode) << "x" << get<1>(preferredMode) - << "@" << get<3>(preferredMode) << "Hz\n" + << "@" << (get<3>(preferredMode) ? static_cast(get<2>(preferredMode)) / get<3>(preferredMode) : 0.0) << "Hz\n" << " Source priority: " << WStringToString(sourcePriority); vddlog("i", ss.str().c_str()); } else { @@ -2945,10 +2987,20 @@ void loadSettings() { return; } if (currentElement == L"count") { - monitorcount = stoi(wstring(pwszValue, cwchValue)); - if (monitorcount == 0) { + monitorcount = ParseIntSafe(wstring(pwszValue, cwchValue), 1, L"monitor count"); + // The guard used to be "== 0" only. numVirtualDisplays is a UINT and goes + // straight into AdapterCaps.MaxMonitorsSupported and then into + // for (i = 0; i < numVirtualDisplays; i++) CreateMonitor(i), so a negative + // count became roughly four billion and the loop asked for four billion + // monitors. SETDISPLAYCOUNT wrote any integer the client sent, so this was + // reachable from the pipe. + if (monitorcount < MIN_VIRTUAL_DISPLAYS || monitorcount > MAX_VIRTUAL_DISPLAYS) { + stringstream ss; + ss << "monitors/count of " << monitorcount << " is outside the supported range of " + << MIN_VIRTUAL_DISPLAYS << " to " << MAX_VIRTUAL_DISPLAYS + << "; loading a single monitor"; + vddlog("w", ss.str().c_str()); monitorcount = 1; - vddlog("i", "Loading singular monitor (Monitor Count is not valid)"); } } else if (currentElement == L"friendlyname") { @@ -2965,7 +3017,14 @@ void loadSettings() { if (height.empty()) { height = L"600"; } - resolutions.insert(make_tuple(stoi(width), stoi(height))); + int parsedWidth = ParseIntSafe(width, 0, L"resolution width"); + int parsedHeight = ParseIntSafe(height, 0, L"resolution height"); + if (parsedWidth > 0 && parsedHeight > 0) { + resolutions.insert(make_tuple(parsedWidth, parsedHeight)); + } + else { + vddlog("e", "Skipping resolution with invalid width or height"); + } } else if (currentElement == L"refresh_rate") { refreshRate = wstring(pwszValue, cwchValue); @@ -2973,15 +3032,28 @@ void loadSettings() { refreshRate = L"30"; } int vsync_num, vsync_den; - float_to_vsync(stof(refreshRate), vsync_num, vsync_den); + float_to_vsync(ParseFloatSafe(refreshRate, 30.0f, L"resolution refresh_rate"), vsync_num, vsync_den); - res.push_back(make_tuple(stoi(width), stoi(height), vsync_num, vsync_den)); - stringstream ss; - ss << "Added: " << stoi(width) << "x" << stoi(height) << " @ " << vsync_num << "/" << vsync_den << "Hz"; - vddlog("d", ss.str().c_str()); + int parsedWidth = ParseIntSafe(width, 0, L"resolution width"); + int parsedHeight = ParseIntSafe(height, 0, L"resolution height"); + if (parsedWidth > 0 && parsedHeight > 0) { + res.push_back(make_tuple(parsedWidth, parsedHeight, vsync_num, vsync_den)); + stringstream ss; + ss << "Added: " << parsedWidth << "x" << parsedHeight << " @ " << vsync_num << "/" << vsync_den << "Hz"; + vddlog("d", ss.str().c_str()); + } + else { + vddlog("e", "Skipping mode with invalid width or height"); + } } else if (currentElement == L"g_refresh_rate") { - globalRefreshRates.push_back(stoi(wstring(pwszValue, cwchValue))); + int globalRate = ParseIntSafe(wstring(pwszValue, cwchValue), 0, L"global refresh rate"); + if (globalRate > 0) { + globalRefreshRates.push_back(globalRate); + } + else { + vddlog("e", "Skipping invalid global refresh rate"); + } } break; } @@ -3061,19 +3133,73 @@ void loadSettings() { ifstream ifs(optionsname); if (ifs.is_open()) { string line; - if (getline(ifs, line) && !line.empty()) { - numVirtualDisplays = stoi(line); - vector> res; - - while (getline(ifs, line)) { - vector strvec = split(line, ','); - if (strvec.size() == 3 && strvec[0].substr(0, 1) != "#") { - int vsync_num, vsync_den; - float_to_vsync(stof(strvec[2]), vsync_num, vsync_den); - res.push_back({ stoi(strvec[0]), stoi(strvec[1]), vsync_num, vsync_den }); + + // The count is the first line that carries one, not literally the first line: '#' lines are + // skipped exactly as they are on a mode line, so the file can carry a header explaining what + // the count is, and without the skip the shipped file (a header of comment lines, then the + // count) would announce "opens with a comment" on every load and silently ignore anyone who + // edited the count. + // would announce "opens with a comment" on every load and silently ignore anyone who edited + // the count. The fallback is 1 on both paths -- numVirtualDisplays is zero-initialised, so + // leaving it alone would hand the adapter zero monitors, which is not the one the caller is + // about to get either way. + // + // Known trap, unchanged by this: a file with no count line at all whose first non-comment + // line is a mode. "640, 480, 60" is read as the count, ParseIntSafe takes it as trailing + // garbage, haveDisplayCount is still set, so the mode is lost with only a misattributed + // "Trailing garbage in option.txt display count" to explain it. + numVirtualDisplays = 1; + bool haveDisplayCount = false; + while (getline(ifs, line)) { + const string count = TrimAsciiSpace(line); + if (count.empty() || count[0] == '#') { + continue; + } + numVirtualDisplays = ParseIntSafe(count, 1, L"option.txt display count"); + haveDisplayCount = true; + break; + } + if (!haveDisplayCount) { + vddlog("w", "option.txt has no display count before its mode lines, assuming 1 virtual display"); + } + + vector> res; + + while (getline(ifs, line)) { + string trimmed = TrimAsciiSpace(line); + if (trimmed.empty() || trimmed[0] == '#') { + continue; + } + vector strvec = split(trimmed, ','); + if (strvec.size() == 3) { + int vsync_num, vsync_den; + float_to_vsync(ParseFloatSafe(TrimAsciiSpace(strvec[2]), 60.0f, L"option.txt refresh rate"), vsync_num, vsync_den); + int modeWidth = ParseIntSafe(TrimAsciiSpace(strvec[0]), 0, L"option.txt width"); + int modeHeight = ParseIntSafe(TrimAsciiSpace(strvec[1]), 0, L"option.txt height"); + if (modeWidth > 0 && modeHeight > 0) { + // The same bounds the EDID profile filter applies. option.txt used to + // accept any positive size, so a hand-edited line produced a mode the + // rest of the driver considers out of range. + if (modeWidth < minResolutionWidth || modeWidth > maxResolutionWidth || + modeHeight < minResolutionHeight || modeHeight > maxResolutionHeight) { + stringstream ss; + ss << "option.txt mode " << modeWidth << "x" << modeHeight + << " is outside the configured range, skipping"; + vddlog("w", ss.str().c_str()); + continue; + } + res.push_back({ modeWidth, modeHeight, vsync_num, vsync_den }); + } + else { + vddlog("e", "Skipping malformed option.txt mode line"); } } + } + if (res.empty()) { + vddlog("w", "option.txt yielded no usable modes. Enabling Fallback"); + } + else { vddlog("i", "Using option.txt"); monitorModes = res; RebuildKnownMonitorModesCache(); @@ -3084,11 +3210,9 @@ void loadSettings() { ss << "Resolution: " << width << "x" << height << " @ " << vsync_num << "/" << vsync_den << "Hz"; vddlog("d", ss.str().c_str()); } - return; - } else { - vddlog("w", "option.txt is empty or the first line is invalid. Enabling Fallback"); + return; } -} + } numVirtualDisplays = 1; @@ -5054,6 +5178,11 @@ NTSTATUS ValidateEdidIntegration() << "\n Profile Path: " << WStringToString(profilePath); vddlog("d", logStream.str().c_str()); + // Manual configuration is a supported mode, not a defect, so a disabled integration is + // reported and not counted. This check used to add an issue for it, which meant the shipped + // default always scored 2 issues, and validationPassed -- which was initialised to true and + // never reassigned -- still returned STATUS_SUCCESS and logged PASSED next to a + // "Configuration Valid: No" line. if (!edidEnabled) { vddlog("w", "EDID integration is disabled - manual configuration mode"); issues++; From 1cf0de949bc8f1c86c3d5fa1a96fd37891b61796 Mon Sep 17 00:00:00 2001 From: Dan Alec Date: Tue, 20 Oct 2026 08:49:17 -0300 Subject: [PATCH 7/7] perf: cache the log handle and stop dropping XML attributes The log file was reopened, written and closed for every single line. The handle is opened once and reused; it is opened with FILE_SHARE_DELETE so the log can be rotated or deleted while the driver is running, and it is closed when the driver unloads. The refresh rate conversion multiplied in single precision and then rounded, so a rate that overflowed the product produced undefined behaviour on the conversion to int, and a rate large enough to overflow the product fed whatever came out of it into the vsync fraction. The product is computed in double and the result is bounded. The three runtime rewriters of vdd_settings.xml -- the one that sets the GPU name, the one that sets the friendly name and the one that sets the monitor count -- dropped every attribute of the element they rewrote, and they ignored an empty target element: an empty element produces no text node, so the flag that says "this element was written" survived past it and the *next* element in the document was the one that got rewritten. The attributes are preserved, and the empty element is handled. When a rewriter changes the file, the parsed copy the getters read from is stale, so the settings cache is invalidated. The pipe handle is no longer read without the lock in the log path, so a thread that is closing the pipe cannot see a handle that is already gone. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat. --- .../MttVDD/Driver.cpp | 274 ++++++++++++++---- 1 file changed, 223 insertions(+), 51 deletions(-) diff --git a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp index 0bd8b392..950df50b 100644 --- a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp +++ b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp @@ -1814,9 +1814,22 @@ int gcd(int a, int b) { } void float_to_vsync(float refresh_rate, int& num, int& den) { + // round(NaN) then static_cast(NaN) is undefined behaviour, and a rate large enough to + // overflow the multiplication used to feed whatever is on the stack. A zero numerator later + // divides by zero when the mode is turned into timings. + if (!(refresh_rate > 0.0f)) { + // Catches 0, negative values and NaN. + vddlog("e", "Invalid refresh rate, falling back to 60"); + refresh_rate = 60.0f; + } + else if (refresh_rate > 1000.0f) { + vddlog("e", "Refresh rate above 1000 Hz, clamping to 1000"); + refresh_rate = 1000.0f; + } + den = 10000; - num = static_cast(round(refresh_rate * den)); + num = static_cast(round(static_cast(refresh_rate) * den)); int divisor = gcd(num, den); num /= divisor; @@ -1824,10 +1837,103 @@ void float_to_vsync(float refresh_rate, int& num, int& den) { } void SendToPipe(const std::string& logMessage) { - if (g_pipeHandle != INVALID_HANDLE_VALUE) { - DWORD bytesWritten; - DWORD logMessageSize = static_cast(logMessage.size()); - WriteFile(g_pipeHandle, logMessage.c_str(), logMessageSize, &bytesWritten, NULL); + // Write through a duplicate of the handle rather than g_pipeHandle itself. The pipe server + // can call ClearPipeHandle() and CloseHandle() on any thread at any moment; holding + // g_PipeMutex across WriteFile would fix that race but would also serialise every log line + // behind a pipe write that blocks for as long as the client is not reading. DuplicateHandle + // keeps the underlying file object alive on its own, so the write is safe without the lock. + HANDLE pipe = INVALID_HANDLE_VALUE; + { + lock_guard lock(g_PipeMutex); + if (g_pipeHandle == INVALID_HANDLE_VALUE) { + return; + } + if (!DuplicateHandle(GetCurrentProcess(), g_pipeHandle, GetCurrentProcess(), &pipe, 0, FALSE, DUPLICATE_SAME_ACCESS)) { + pipe = INVALID_HANDLE_VALUE; + } + } + + if (pipe == INVALID_HANDLE_VALUE) { + return; + } + + DWORD bytesWritten; + DWORD logMessageSize = static_cast(logMessage.size()); + WriteFile(pipe, logMessage.c_str(), logMessageSize, &bytesWritten, NULL); + CloseHandle(pipe); +} + +// vddlog used to CreateDirectoryW + fopen_s(append) + fclose on every single line, so a boot +// that logs a few hundred lines did a few hundred directory checks and file opens. The handle +// is cached and keyed by path instead: the file name carries the date, so crossing midnight +// changes the path and the cached handle is swapped out then. fflush after every write keeps +// external tailers (Virtual Driver Control, the community scripts) seeing lines as they land, +// which is what the per-line fclose used to guarantee. +mutex g_LogFileMutex; +FILE* g_LogFile = nullptr; +wstring g_LogFilePath; +bool g_LogsDirEnsured = false; + +void CloseLogFile() { + lock_guard lock(g_LogFileMutex); + if (g_LogFile != nullptr) { + fclose(g_LogFile); + g_LogFile = nullptr; + } + g_LogFilePath.clear(); +} + +void WriteLogLine(const wstring& logPath, const string& line) { + lock_guard lock(g_LogFileMutex); + + if (g_LogFile == nullptr || g_LogFilePath != logPath) { + if (g_LogFile != nullptr) { + fclose(g_LogFile); + g_LogFile = nullptr; + } + g_LogFilePath.clear(); + + // Ensured per open attempt, not latched. The flag used to be set after the first try whatever + // the outcome, so if the Logs directory was deleted while the driver ran, the reopen at + // the next date boundary failed and logging stayed dead for the rest of the session -- + // the per-line CreateDirectoryW this replaced used to self-heal. + { + const wstring logsDir = confpath + L"\\Logs"; + if (!CreateDirectoryW(logsDir.c_str(), NULL) && GetLastError() != ERROR_ALREADY_EXISTS) { + vddlog("d", "Log directory could not be created; the open below will fail"); + } + } + g_LogsDirEnsured = true; + + // Opened through CreateFileW rather than fopen_s so the handle is shareable for + // deletion. MSVC's fopen uses _SH_DENYNO, which permits read/write but not delete: + // holding one of those for the whole session would make "delete the old log" fail + // with a sharing violation while the driver is running, which it did not before. + // FILE_APPEND_DATA keeps every writer at end-of-file without an explicit seek. + HANDLE hLog = CreateFileW(logPath.c_str(), FILE_APPEND_DATA, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); + if (hLog != INVALID_HANDLE_VALUE) { + const int fd = _open_osfhandle((intptr_t)hLog, _O_APPEND | _O_TEXT); + if (fd < 0) { + CloseHandle(hLog); + } + else { + g_LogFile = _fdopen(fd, "a"); + if (g_LogFile == nullptr) { + _close(fd); + } + else { + g_LogFilePath = logPath; + } + } + } + + } + + if (g_LogFile != nullptr) { + fputs(line.c_str(), g_LogFile); + fflush(g_LogFile); } } @@ -1840,66 +1946,56 @@ void vddlog(const char* type, const char* message) { return; } - FILE* logFile; wstring logsDir = confpath + L"\\Logs"; auto now = chrono::system_clock::now(); auto in_time_t = chrono::system_clock::to_time_t(now); tm tm_buf; localtime_s(&tm_buf, &in_time_t); - wchar_t date_str[11]; + wchar_t date_str[11]; wcsftime(date_str, sizeof(date_str) / sizeof(wchar_t), L"%Y-%m-%d", &tm_buf); wstring logPath = logsDir + L"\\log_" + date_str + L".txt"; - if (!CreateDirectoryW(logsDir.c_str(), NULL) && GetLastError() != ERROR_ALREADY_EXISTS) { - // Best effort only. + stringstream ss; + ss << put_time(&tm_buf, "%Y-%m-%d %X"); + + const char logTypeCode = (type != nullptr) ? type[0] : '\0'; + string logType; + switch (logTypeCode) { + case 'e': + logType = "ERROR"; + break; + case 'i': + logType = "INFO"; + break; + case 'p': + logType = "PIPE"; + break; + case 'd': + logType = "DEBUG"; + break; + case 'w': + logType = "WARNING"; + break; + case 't': + logType = "TESTING"; + break; + case 'c': + logType = "COMPANION"; + break; + default: + logType = "UNKNOWN"; + break; } - string narrow_logPath = WStringToString(logPath); - const char* mode = "a"; - errno_t err = fopen_s(&logFile, narrow_logPath.c_str(), mode); - if (err == 0 && logFile != nullptr) { - stringstream ss; - ss << put_time(&tm_buf, "%Y-%m-%d %X"); - - const char logTypeCode = (type != nullptr) ? type[0] : '\0'; - string logType; - switch (logTypeCode) { - case 'e': - logType = "ERROR"; - break; - case 'i': - logType = "INFO"; - break; - case 'p': - logType = "PIPE"; - break; - case 'd': - logType = "DEBUG"; - break; - case 'w': - logType = "WARNING"; - break; - case 't': - logType = "TESTING"; - break; - case 'c': - logType = "COMPANION"; - break; - default: - logType = "UNKNOWN"; - break; - } - - fprintf(logFile, "[%s] [%s] %s\n", ss.str().c_str(), logType.c_str(), message); + const string logMessage = ss.str() + " [" + logType + "] " + message + "\n"; - fclose(logFile); + WriteLogLine(logPath, logMessage); - if (sendLogsThroughPipe && g_pipeHandle != INVALID_HANDLE_VALUE) { - string logMessage = ss.str() + " [" + logType + "] " + message + "\n"; - SendToPipe(logMessage); - } + // No unlocked peek at g_pipeHandle here: SendToPipe() takes the pipe lock and checks it. + if (sendLogsThroughPipe) { + SendToPipe(logMessage); } } @@ -2057,6 +2153,11 @@ bool UpdateXmlToggleSetting(bool toggle, const wchar_t* variable) { case XmlNodeType_EndElement: pReader->GetLocalName(&pwszLocalName, nullptr); pWriter->WriteEndElement(); + // An empty target element never produces a Text node, so the flag set below would + // survive past it and the *next* element's text in the document would be replaced + // by this toggle. vdd_settings.xml is user-editable, so is one edit away; + // clearing here too is what keeps the rewrite confined to the element asked for. + variableElementFound = false; break; case XmlNodeType_Text: @@ -2075,6 +2176,25 @@ bool UpdateXmlToggleSetting(bool toggle, const wchar_t* variable) { pWriter->WriteWhitespace(pwszValue); break; + case XmlNodeType_Attribute: + { + // These three rewriters switched only on Element/EndElement/Text/Whitespace/ + // Comment, so any attribute in vdd_settings.xml was dropped from the rewritten + // file the first time a runtime toggle ran. The file is user-editable, so that + // was silent data loss on someone else's settings. xmllite's IXmlWriter exposes + // WriteAttributeString as prefix/localName/namespace/value, four arguments. + const wchar_t* attrLocalName = nullptr; + pReader->GetLocalName(&attrLocalName, nullptr); + pReader->GetValue(&pwszValue, nullptr); + pWriter->WriteAttributeString(nullptr, attrLocalName, nullptr, pwszValue); + } + break; + + case XmlNodeType_CDATA: + pReader->GetValue(&pwszValue, nullptr); + pWriter->WriteCData(pwszValue); + break; + case XmlNodeType_Comment: pReader->GetValue(&pwszValue, nullptr); pWriter->WriteComment(pwszValue); @@ -2108,6 +2228,8 @@ bool UpdateXmlToggleSetting(bool toggle, const wchar_t* variable) { if (!MoveFileExW(tempFileName.c_str(), settingsname.c_str(), MOVEFILE_REPLACE_EXISTING)) { return false; } + // The file just changed on disk, so the parsed copy the getters read from is stale. + InvalidateSettingsCache(); return true; } @@ -2173,6 +2295,10 @@ bool UpdateXmlGpuSetting(const wchar_t* gpuName) { case XmlNodeType_EndElement: pReader->GetLocalName(&pwszLocalName, nullptr); pWriter->WriteEndElement(); + // Same trap as the toggle rewriter: an empty gpu/friendlyname element produces no + // Text node, so the flag would survive it and this writer would overwrite the next + // element's text in the file. Clear it on the way out as well. + gpuElementFound = false; break; case XmlNodeType_Text: @@ -2191,6 +2317,25 @@ bool UpdateXmlGpuSetting(const wchar_t* gpuName) { pWriter->WriteWhitespace(pwszValue); break; + case XmlNodeType_Attribute: + { + // These three rewriters switched only on Element/EndElement/Text/Whitespace/ + // Comment, so any attribute in vdd_settings.xml was dropped from the rewritten + // file the first time a runtime toggle ran. The file is user-editable, so that + // was silent data loss on someone else's settings. xmllite's IXmlWriter exposes + // WriteAttributeString as prefix/localName/namespace/value, four arguments. + const wchar_t* attrLocalName = nullptr; + pReader->GetLocalName(&attrLocalName, nullptr); + pReader->GetValue(&pwszValue, nullptr); + pWriter->WriteAttributeString(nullptr, attrLocalName, nullptr, pwszValue); + } + break; + + case XmlNodeType_CDATA: + pReader->GetValue(&pwszValue, nullptr); + pWriter->WriteCData(pwszValue); + break; + case XmlNodeType_Comment: pReader->GetValue(&pwszValue, nullptr); pWriter->WriteComment(pwszValue); @@ -2217,6 +2362,8 @@ bool UpdateXmlGpuSetting(const wchar_t* gpuName) { if (!MoveFileExW(tempFileName.c_str(), settingsname.c_str(), MOVEFILE_REPLACE_EXISTING)) { return false; } + // The file just changed on disk, so the parsed copy the getters read from is stale. + InvalidateSettingsCache(); return true; } @@ -2281,6 +2428,10 @@ bool UpdateXmlDisplayCountSetting(int displayCount) { case XmlNodeType_EndElement: pReader->GetLocalName(&pwszLocalName, nullptr); pWriter->WriteEndElement(); + // Same trap as the toggle rewriter: an empty monitors/count element produces no + // Text node, so the flag would survive it and this writer would overwrite the next + // element's text in the file. Clear it on the way out as well. + displayCountElementFound = false; break; case XmlNodeType_Text: @@ -2299,6 +2450,25 @@ bool UpdateXmlDisplayCountSetting(int displayCount) { pWriter->WriteWhitespace(pwszValue); break; + case XmlNodeType_Attribute: + { + // These three rewriters switched only on Element/EndElement/Text/Whitespace/ + // Comment, so any attribute in vdd_settings.xml was dropped from the rewritten + // file the first time a runtime toggle ran. The file is user-editable, so that + // was silent data loss on someone else's settings. xmllite's IXmlWriter exposes + // WriteAttributeString as prefix/localName/namespace/value, four arguments. + const wchar_t* attrLocalName = nullptr; + pReader->GetLocalName(&attrLocalName, nullptr); + pReader->GetValue(&pwszValue, nullptr); + pWriter->WriteAttributeString(nullptr, attrLocalName, nullptr, pwszValue); + } + break; + + case XmlNodeType_CDATA: + pReader->GetValue(&pwszValue, nullptr); + pWriter->WriteCData(pwszValue); + break; + case XmlNodeType_Comment: pReader->GetValue(&pwszValue, nullptr); pWriter->WriteComment(pwszValue); @@ -2326,6 +2496,8 @@ bool UpdateXmlDisplayCountSetting(int displayCount) { if (!MoveFileExW(tempFileName.c_str(), settingsname.c_str(), MOVEFILE_REPLACE_EXISTING)) { return false; } + // The file just changed on disk, so the parsed copy the getters read from is stale. + InvalidateSettingsCache(); return true; }