From 48f12cad166ea10deffbda0a4736c000e5ffcea1 Mon Sep 17 00:00:00 2001 From: Dan Alec Date: Fri, 16 Oct 2026 23:23:51 -0300 Subject: [PATCH 1/4] 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/4] 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/4] 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/4] 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);