diff --git a/Virtual Display Driver (HDR)/MttVDD/Driver.cpp b/Virtual Display Driver (HDR)/MttVDD/Driver.cpp index 3145b04a..950df50b 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 @@ -151,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; @@ -169,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; @@ -177,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"}}, @@ -218,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"}}, @@ -226,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"}}, @@ -240,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"}}, @@ -265,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) { @@ -334,7 +499,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!"); @@ -344,89 +614,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) { @@ -439,7 +727,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); @@ -454,13 +741,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; } @@ -471,51 +771,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; @@ -531,68 +799,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""; - } - - 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; - } - } - } + std::wstring xmlValue; + if (!ReadSettingFromXml(xmlName, xmlValue)) { + LogQueries("d", xmlName + L" - not present in XML."); + return L""; } - return xmlLoggingValue; + LogQueries("i", xmlName + L" - Retrieved from XML: " + xmlValue); + return xmlValue; } double GetDoubleSetting(const std::wstring& settingKey) { @@ -605,18 +848,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; } @@ -629,51 +895,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."); + std::wstring xmlValue; + if (!ReadSettingFromXml(xmlName, xmlValue)) { + LogQueries("d", xmlName + L" - not present in XML."); return 0.0; } - CComPtr pReader; - hr = CreateXmlReader(__uuidof(IXmlReader), (void**)&pReader, nullptr); - if (FAILED(hr)) { - LogQueries("d", xmlName + L" - Failed to create XML reader."); - 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; @@ -715,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 @@ -866,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) @@ -907,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; @@ -937,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; @@ -999,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)); } } @@ -1011,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; @@ -1042,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; } @@ -1074,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; @@ -1089,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) { @@ -1217,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; @@ -1227,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)); @@ -1258,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 @@ -1270,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; @@ -1354,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 { @@ -1362,18 +1696,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; @@ -1385,7 +1734,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; } @@ -1417,7 +1766,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"); } @@ -1446,7 +1795,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"); } @@ -1465,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; @@ -1475,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); } } @@ -1491,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); } } @@ -1708,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: @@ -1726,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); @@ -1759,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; } @@ -1824,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: @@ -1842,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); @@ -1868,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; } @@ -1932,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: @@ -1950,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); @@ -1977,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; } @@ -2423,7 +2944,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 @@ -2433,11 +2954,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; @@ -2455,7 +2991,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"); @@ -2482,35 +3018,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); @@ -2597,10 +3159,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") { @@ -2617,7 +3189,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); @@ -2625,15 +3204,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; } @@ -2713,19 +3305,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(); @@ -2736,11 +3382,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; @@ -3248,7 +3892,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 +4482,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 = {}; @@ -4694,6 +5350,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++;