diff --git a/vibrance.GUI/AMD/AmdDynamicVibranceProxy.cs b/vibrance.GUI/AMD/AmdDynamicVibranceProxy.cs index 7e98d3d..bac5137 100644 --- a/vibrance.GUI/AMD/AmdDynamicVibranceProxy.cs +++ b/vibrance.GUI/AMD/AmdDynamicVibranceProxy.cs @@ -114,14 +114,21 @@ private void OnWinEventHook(object sender, WinEventHookEventArgs e) { //test if a resolution change is needed Screen screen = Screen.FromHandle(e.Handle); - if (_vibranceInfo.neverChangeResolution == false && - applicationSetting.IsResolutionChangeNeeded && - IsResolutionChangeNeeded(screen, applicationSetting.ResolutionSettings) && + if (_vibranceInfo.neverChangeResolution == false && + applicationSetting.IsResolutionChangeNeeded && + ResolutionHelper.IsResolutionChangeNeeded(screen.DeviceName, applicationSetting.ResolutionSettings) && _windowsResolutionSettings.ContainsKey(screen.DeviceName) && _windowsResolutionSettings[screen.DeviceName].Item2.Contains(applicationSetting.ResolutionSettings)) { _gameScreen = screen; - PerformResolutionChange(screen, applicationSetting.ResolutionSettings); + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx( + applicationSetting.ResolutionSettings, screen.DeviceName, false); + // AppliedUnverified means CDS_UPDATEREGISTRY itself reported success but the + // post-apply readback did not confirm it - see the matching comment in + // NvidiaDynamicVibranceProxy's OnWinEventHook. + _vibranceInfo.isResolutionChangeApplied = + result == ResolutionHelper.ResolutionChangeResult.Applied || + result == ResolutionHelper.ResolutionChangeResult.AppliedUnverified; } _amdAdapter.SetSaturationOnAllDisplays(_vibranceInfo.userVibranceSettingDefault); @@ -142,32 +149,25 @@ private void OnWinEventHook(object sender, WinEventHookEventArgs e) //test if a resolution change is needed Screen screen = Screen.FromHandle(processHandle); - if (_vibranceInfo.neverChangeResolution == false && - _gameScreen != null && _gameScreen.Equals(screen) && + if (_vibranceInfo.neverChangeResolution == false && + _gameScreen != null && _gameScreen.Equals(screen) && _windowsResolutionSettings.ContainsKey(screen.DeviceName) && - IsResolutionChangeNeeded(screen, _windowsResolutionSettings[screen.DeviceName].Item1)) + ResolutionHelper.IsResolutionChangeNeeded(screen.DeviceName, _windowsResolutionSettings[screen.DeviceName].Item1)) { - PerformResolutionChange(screen, _windowsResolutionSettings[screen.DeviceName].Item1); + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx( + _windowsResolutionSettings[screen.DeviceName].Item1, screen.DeviceName, true); + // A failed (or unverified) revert must leave the flag true so the next + // foreground event retries it; Suppressed (the give-up state) deliberately + // still clears it - see the matching comment in NvidiaDynamicVibranceProxy's + // OnWinEventHook. + if (result != ResolutionHelper.ResolutionChangeResult.Failed && + result != ResolutionHelper.ResolutionChangeResult.AppliedUnverified) + _vibranceInfo.isResolutionChangeApplied = false; } _amdAdapter.SetSaturationOnAllDisplays(_vibranceInfo.userVibranceSettingDefault); } } } - - private static bool IsResolutionChangeNeeded(Screen screen, ResolutionModeWrapper resolutionSettings) - { - Devmode mode; - if (resolutionSettings != null && ResolutionHelper.GetCurrentResolutionSettings(out mode, screen.DeviceName) && !resolutionSettings.Equals(mode)) - { - return true; - } - return false; - } - - private static void PerformResolutionChange(Screen screen, ResolutionModeWrapper resolutionSettings) - { - ResolutionHelper.ChangeResolutionEx(resolutionSettings, screen.DeviceName); - } } } \ No newline at end of file diff --git a/vibrance.GUI/NVIDIA/NvidiaDynamicVibranceProxy.cs b/vibrance.GUI/NVIDIA/NvidiaDynamicVibranceProxy.cs index 8f2a5b3..2ec44fe 100644 --- a/vibrance.GUI/NVIDIA/NvidiaDynamicVibranceProxy.cs +++ b/vibrance.GUI/NVIDIA/NvidiaDynamicVibranceProxy.cs @@ -219,13 +219,23 @@ private static void OnWinEventHook(object sender, WinEventHookEventArgs e) { //test if a resolution change is needed Screen screen = Screen.FromHandle(e.Handle); - if (_vibranceInfo.neverChangeResolution == false && - applicationSetting.IsResolutionChangeNeeded && - IsResolutionChangeNeeded(screen, applicationSetting.ResolutionSettings) && + if (_vibranceInfo.neverChangeResolution == false && + applicationSetting.IsResolutionChangeNeeded && + ResolutionHelper.IsResolutionChangeNeeded(screen.DeviceName, applicationSetting.ResolutionSettings) && _windowsResolutionSettings.ContainsKey(screen.DeviceName) && _windowsResolutionSettings[screen.DeviceName].Item2.Contains(applicationSetting.ResolutionSettings)) { - PerformResolutionChange(screen, applicationSetting.ResolutionSettings); + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx( + applicationSetting.ResolutionSettings, screen.DeviceName, false); + // AppliedUnverified means CDS_UPDATEREGISTRY itself reported success but + // the post-apply readback did not confirm it - the mode most likely DID + // change, so this still counts as applied for the purpose of a later + // revert attempt. Treating it as "not applied" would both strand the + // desktop at whatever this change actually produced AND tell the user the + // opposite of what happened. + _vibranceInfo.isResolutionChangeApplied = + result == ResolutionHelper.ResolutionChangeResult.Applied || + result == ResolutionHelper.ResolutionChangeResult.AppliedUnverified; } _gameScreen = screen; _vibranceInfo.defaultHandle = displayHandle; @@ -241,13 +251,25 @@ private static void OnWinEventHook(object sender, WinEventHookEventArgs e) //test if a resolution change is needed Screen currentScreen = Screen.FromHandle(processHandle); - if (_vibranceInfo.neverChangeResolution == false && - _gameScreen != null && - _gameScreen.Equals(currentScreen) && + if (_vibranceInfo.neverChangeResolution == false && + _gameScreen != null && + _gameScreen.Equals(currentScreen) && _windowsResolutionSettings.ContainsKey(currentScreen.DeviceName) && - IsResolutionChangeNeeded(currentScreen, _windowsResolutionSettings[currentScreen.DeviceName].Item1)) + ResolutionHelper.IsResolutionChangeNeeded(currentScreen.DeviceName, _windowsResolutionSettings[currentScreen.DeviceName].Item1)) { - PerformResolutionChange(currentScreen, _windowsResolutionSettings[currentScreen.DeviceName].Item1); + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx( + _windowsResolutionSettings[currentScreen.DeviceName].Item1, currentScreen.DeviceName, true); + // A failed (or unverified) revert must leave the flag true, or the next + // foreground event would never retry it - AppliedUnverified here means the + // revert's own CDS_UPDATEREGISTRY reported success but the readback did not + // confirm the desktop is really back, so it is treated the same as Failed: + // still worth another attempt. Suppressed (the give-up state) deliberately + // still clears it: once ChangeResolutionEx has stopped calling the driver at + // all, holding this true would retry forever with the device call skipped + // every time. + if (result != ResolutionHelper.ResolutionChangeResult.Failed && + result != ResolutionHelper.ResolutionChangeResult.AppliedUnverified) + _vibranceInfo.isResolutionChangeApplied = false; } //test if changing the vibrance value is needed @@ -268,21 +290,6 @@ private static void OnWinEventHook(object sender, WinEventHookEventArgs e) } } - private static bool IsResolutionChangeNeeded(Screen screen, ResolutionModeWrapper resolutionSettings) - { - Devmode mode; - if (resolutionSettings != null && ResolutionHelper.GetCurrentResolutionSettings(out mode, screen.DeviceName) && !resolutionSettings.Equals(mode)) - { - return true; - } - return false; - } - - private static void PerformResolutionChange(Screen screen, ResolutionModeWrapper resolutionSettings) - { - ResolutionHelper.ChangeResolutionEx(resolutionSettings, screen.DeviceName); - } - private void EnumerateDisplayHandles() { for (int i = 0, displayHandle = 0; displayHandle != -1; i++) diff --git a/vibrance.GUI/Program.cs b/vibrance.GUI/Program.cs index 472db5b..e5c3b2c 100644 --- a/vibrance.GUI/Program.cs +++ b/vibrance.GUI/Program.cs @@ -19,6 +19,7 @@ static class Program private const string ErrorGraphicsAdapterUnknown = "Failed to determine your Graphic GraphicsAdapter type (NVIDIA/AMD). Make sure you have installed a proper GPU driver. Intel laptops are not supported as stated on the website. When installing your GPU driver did not work, please contact @juvlarN at twitter. Press Yes to open twitter in your browser now. Error: "; private const string ErrorGraphicsAdapterAmbiguous = "Both NVIDIA and AMD graphic drivers have been found on your system. This can happen when you recently switched your graphic card and did not uninstall the old drivers. Make sure to uninstall unused graphic drivers to keep your system safe and stable. Use the program \"Display Driver Uninstaller\" to uninstall your old drivers!\n\nPress Yes to open \"Display Driver Uninstaller\" download website now.\nPress No to quit vibranceGUI."; private const string MessageBoxCaption = "vibranceGUI Error"; + private const string ResolutionSelfTestMessageBoxCaption = "vibranceGUI resolution change self test"; [STAThread] static void Main(string[] args) @@ -33,6 +34,21 @@ static void Main(string[] args) Application.EnableVisualStyles(); Application.SetCompatibleTextRenderingDefault(false); + + // Runs before the GPU vendor detection below on purpose: ResolutionChangeFixture only + // ever drives ChangeResolutionEx through a fake IDisplayModeDevice, so it needs no + // driver and stays runnable on a machine GetAdapter() cannot resolve and would exit + // from. There is deliberately no hardware variant of this self test, and there must + // never be one - a display mode has no guaranteed undo, and a mode the panel cannot + // show would leave a user unable to even see a dialog asking them to confirm it, which + // is literally what issue #114 reports. + if (args.Contains("--selftest-resolution")) + { + MessageBox.Show(string.Join(Environment.NewLine, ResolutionChangeFixture.Run().ToArray()), + ResolutionSelfTestMessageBoxCaption, MessageBoxButtons.OK, MessageBoxIcon.Information); + return; + } + NativeMethods.SetDllDirectory(CommonUtils.GetVibrance_GUI_AppDataPath()); GraphicsAdapter adapter = GraphicsAdapterHelper.GetAdapter(); @@ -97,5 +113,20 @@ static void Main(string[] args) GC.KeepAlive(mutex); } + + // Internal rather than private: ResolutionHelper's WinEvent-reachable failure-recording + // path (ChangeResolutionEx) reuses this so a broken log write (e.g. File.AppendText + // failing) cannot itself throw an exception across the native WinEvent callback frame. + internal static void LogSafely(string message) + { + try + { + VibranceGUI.Log(message); + } + catch (Exception) + { + // Logging must never be the reason a resolution change fails to complete. + } + } } } diff --git a/vibrance.GUI/common/Definitions.cs b/vibrance.GUI/common/Definitions.cs index 5bda3f2..6306fde 100644 --- a/vibrance.GUI/common/Definitions.cs +++ b/vibrance.GUI/common/Definitions.cs @@ -18,5 +18,10 @@ public struct VibranceInfo public List displayHandles; public bool affectPrimaryMonitorOnly; public bool neverChangeResolution; + // Tracks whether ResolutionHelper.ChangeResolutionEx last reported the game's resolution as + // applied (Applied or AppliedUnverified) for the current foreground game, so + // VibranceGUI.RebuildWindowsResolutionSettings knows not to overwrite the captured Windows + // mode with a live read while one of the proxies' own changes is in effect. + public bool isResolutionChangeApplied; } } diff --git a/vibrance.GUI/common/ResolutionChangeFixture.cs b/vibrance.GUI/common/ResolutionChangeFixture.cs new file mode 100644 index 0000000..61f31da --- /dev/null +++ b/vibrance.GUI/common/ResolutionChangeFixture.cs @@ -0,0 +1,857 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; + +namespace vibrance.GUI.common +{ + /// + /// Regression coverage for the resolution-change fix (#114/#132, both + /// "Changing the resolution failed: DispChangeBadFlags"): a modal MessageBox raised from + /// inside the foreground-change callback (ResolutionHelper.cs no longer has a + /// "using System.Windows.Forms" or any MessageBox call site), a "success" read from the wrong + /// ChangeDisplaySettingsEx call, dmFields never + /// declaring the fields actually being changed, and a DmDisplayFixedOutput-inclusive equality + /// guard that could re-fire a real mode set and registry write on every single foreground + /// event forever. Everything here runs through IDisplayModeDevice against + /// FakeDisplayModeDevice below - no GUI, no live display, ever. Run by vibrance.GUI.exe + /// --selftest-resolution. + /// + public static class ResolutionChangeFixture + { + public static List Run() + { + Checklist checklist = new Checklist(); + checklist.Lines.Add("vibranceGUI resolution change self test"); + checklist.Lines.Add(string.Empty); + + CheckNoDeferredApply(checklist); + CheckDmFieldsDeclared(checklist); + CheckTestGatesRegistryWrite(checklist); + CheckAuthoritativeResult(checklist); + CheckPostApplyVerification(checklist); + CheckFixedOutputFallback(checklist); + CheckNoRepeatNotification(checklist); + CheckApplyBound(checklist); + CheckRevertBound(checklist); + CheckFullRevertCycle(checklist); + CheckFailedRevertDoesNotStrandSilently(checklist); + CheckUnachievableFixedOutputLoopIsBounded(checklist); + CheckAlreadyMatchingShortCircuits(checklist); + CheckClearFailureStateDoesNotStraddleDeviceNames(checklist); + CheckUnreadableCurrentModeNeverCountsTowardGiveUp(checklist); + + checklist.Lines.Add(string.Empty); + checklist.Lines.Add(string.Format("PASSED {0}/{1}", checklist.Passed, checklist.Total)); + return checklist.Lines; + } + + // Regression test for the old CDS_UPDATEREGISTRY|CDS_NORESET staged-commit pattern: a + // success must be exactly CDS_TEST then CDS_UPDATEREGISTRY, both against the real device + // name, with CDS_NORESET never appearing anywhere. + private static void CheckNoDeferredApply(Checklist checklist) + { + checklist.Lines.Add("A successful change records exactly CDS_TEST then CDS_UPDATEREGISTRY, never CDS_NORESET, never a null device name (regression test for the old staged-commit pattern):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-NODEFER"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + + checklist.Check(result == ResolutionHelper.ResolutionChangeResult.Applied, + string.Format("ChangeResolutionEx returns Applied, got {0}", result)); + + checklist.Check(device.CallLog.Count == 2, + string.Format("exactly two ChangeMode calls are recorded, got {0}", device.CallLog.Count)); + + if (device.CallLog.Count == 2) + { + checklist.Check(device.CallLog[0].Flags == ChangeDisplaySettingsFlags.CdsTest, + "the first call uses CDS_TEST"); + checklist.Check(device.CallLog[1].Flags == ChangeDisplaySettingsFlags.CdsUpdateregistry, + "the second call uses CDS_UPDATEREGISTRY"); + } + + checklist.Check(!device.CallLog.Any(call => (call.Flags & ChangeDisplaySettingsFlags.CdsNoreset) != 0), + "CDS_NORESET never appears in any recorded call"); + checklist.Check(!device.CallLog.Any(call => call.DeviceName == null), + "no recorded call has a null device name"); + checklist.Check(device.CallLog.All(call => call.DeviceName == deviceName), + "every recorded call carries the real device name"); + + checklist.Lines.Add(string.Empty); + } + + // Regression test for "dmFields is never updated": the Devmode ChangeMode is called with + // must declare the four fields actually being changed, and must keep declaring whatever + // EnumDisplaySettings already reported (DM_POSITION above all - the only candidate + // mechanism on file for issue #134, a multi-monitor desktop rearranging itself). + private static void CheckDmFieldsDeclared(Checklist checklist) + { + checklist.Lines.Add("The Devmode passed to ChangeMode declares the four fields being changed and keeps DM_POSITION untouched (regression test for the old 'dmFields never updated' defect):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-DMFIELDS"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + + uint requiredFields = (uint)(DevmodeFields.DmPelsWidth | DevmodeFields.DmPelsHeight | + DevmodeFields.DmBitsPerPel | DevmodeFields.DmDisplayFrequency); + + checklist.Check(device.CallLog.Count > 0 && device.CallLog.All(call => (call.Mode.dmFields & requiredFields) == requiredFields), + "DmPelsWidth|DmPelsHeight|DmBitsPerPel|DmDisplayFrequency are set in dmFields on every recorded call"); + checklist.Check(device.CallLog.Count > 0 && device.CallLog.All(call => (call.Mode.dmFields & (uint)DevmodeFields.DmPosition) != 0), + "the DM_POSITION bit the current mode already carried survives into every recorded call, unmodified"); + + checklist.Lines.Add(string.Empty); + } + + // Regression test for the old unconditional commit call: CDS_TEST failing must gate + // CDS_UPDATEREGISTRY completely, for every DispChange failure code, not just some of them. + private static void CheckTestGatesRegistryWrite(Checklist checklist) + { + checklist.Lines.Add("CDS_TEST failing gates CDS_UPDATEREGISTRY - never called after a rejected test, for any DispChange failure code (regression test for the old unconditional commit call):"); + + DispChange[] failureCodes = + { + DispChange.DispChangeRestart, + DispChange.DispChangeFailed, + DispChange.DispChangeBadmode, + DispChange.DispChangeNotupdated, + DispChange.DispChangeBadflags, + DispChange.DispChangeBadparam + }; + + bool allGated = true; + string firstFailureDescription = null; + foreach (DispChange code in failureCodes) + { + ResolutionHelper.ResetForTests(); + string deviceName = "FAKE-RES-GATE-" + code; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + // Queued twice - ApplyTargetFields always ORs DM_DISPLAYFIXEDOUTPUT into dmFields + // (see ResolutionHelper.OwnedFields), so every CDS_TEST failure triggers + // ChangeResolutionEx's one fixed-output fallback retry. Both attempts have to fail + // for this check to prove CDS_UPDATEREGISTRY really is unreachable, not merely + // skipped on the first try. + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, code); + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, code); + + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + + bool noRegistryCall = !device.CallLog.Any(call => call.Flags == ChangeDisplaySettingsFlags.CdsUpdateregistry); + bool returnedFailed = result == ResolutionHelper.ResolutionChangeResult.Failed; + + if (!noRegistryCall || !returnedFailed) + { + allGated = false; + firstFailureDescription = string.Format("{0}: returned {1}, CDS_UPDATEREGISTRY called = {2}", code, result, !noRegistryCall); + break; + } + } + + checklist.Check(allGated, allGated + ? "all six DispChange failure codes from CDS_TEST return Failed with zero CDS_UPDATEREGISTRY calls" + : "all six DispChange failure codes from CDS_TEST return Failed with zero CDS_UPDATEREGISTRY calls - " + firstFailureDescription); + + checklist.Lines.Add(string.Empty); + } + + // Regression test for the old discarded second-call return value: CDS_UPDATEREGISTRY's own + // result is authoritative. Failed propagates as Failed; Notupdated is not treated as a + // failure - the mode is verified live regardless - but still returns Applied only once the + // post-apply readback confirms it, and logs exactly once. + private static void CheckAuthoritativeResult(Checklist checklist) + { + checklist.Lines.Add("CDS_UPDATEREGISTRY's own return code is authoritative - Failed propagates as Failed, Notupdated still verifies and returns Applied (regression test for the old discarded second-call result):"); + + ResolutionHelper.ResetForTests(); + const string failDevice = "FAKE-RES-AUTHORITATIVE-FAIL"; + FakeDisplayModeDevice failFakeDevice = new FakeDisplayModeDevice(); + failFakeDevice.SetCurrentMode(failDevice, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper failTarget = BuildTarget(2560, 1440, 32, 144, 0); + failFakeDevice.QueueResult(failDevice, ChangeDisplaySettingsFlags.CdsUpdateregistry, DispChange.DispChangeFailed); + + ResolutionHelper.ResolutionChangeResult failResult = ResolutionHelper.ChangeResolutionEx(failFakeDevice, failTarget, failDevice, false); + checklist.Check(failResult == ResolutionHelper.ResolutionChangeResult.Failed, + string.Format("CDS_UPDATEREGISTRY returning DispChangeFailed makes ChangeResolutionEx return Failed, got {0}", failResult)); + + ResolutionHelper.ResetForTests(); + const string notupdatedDevice = "FAKE-RES-AUTHORITATIVE-NOTUPDATED"; + FakeDisplayModeDevice notupdatedFakeDevice = new FakeDisplayModeDevice(); + notupdatedFakeDevice.SetCurrentMode(notupdatedDevice, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper notupdatedTarget = BuildTarget(2560, 1440, 32, 144, 0); + notupdatedFakeDevice.QueueResult(notupdatedDevice, ChangeDisplaySettingsFlags.CdsUpdateregistry, DispChange.DispChangeNotupdated); + + ResolutionHelper.ResolutionChangeResult notupdatedResult = ResolutionHelper.ChangeResolutionEx(notupdatedFakeDevice, notupdatedTarget, notupdatedDevice, false); + checklist.Check(notupdatedResult == ResolutionHelper.ResolutionChangeResult.Applied, + string.Format("CDS_UPDATEREGISTRY returning DispChangeNotupdated still returns Applied once the readback confirms it, got {0}", notupdatedResult)); + checklist.Check(ResolutionHelper.LoggedLineCountForTests == 1, + string.Format("exactly one log line is written for the Notupdated case, got {0}", ResolutionHelper.LoggedLineCountForTests)); + + checklist.Lines.Add(string.Empty); + } + + // Regression test for trusting the API's own return code alone: a CDS_UPDATEREGISTRY call + // that reports success but did not actually change the fake's current mode must not come + // back as a confirmed Applied - caught by the post-apply readback, which returns the + // distinct AppliedUnverified rather than a plain Failed (a genuine driver rejection already + // returns Failed at step 4/5, before ever reaching this readback - conflating the two would + // make the proxies wrongly discard a change that most likely did land). + private static void CheckPostApplyVerification(Checklist checklist) + { + checklist.Lines.Add("A CDS_UPDATEREGISTRY call that reports success is still verified by reading the mode back - returning AppliedUnverified, not Applied, and not a plain Failed either (regression test for trusting the API's own return code alone):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-VERIFY"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + // Reports DispChangeSuccessful (the default) but is told not to actually mutate its + // stored mode for that one call - standing in for a driver that lies about having + // applied it. + device.SuppressNextApply(deviceName); + + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + checklist.Check(result == ResolutionHelper.ResolutionChangeResult.AppliedUnverified, + string.Format("a reported-successful apply that did not actually change the device's mode returns AppliedUnverified, not Applied and not a plain Failed, got {0}", result)); + + checklist.Lines.Add(string.Empty); + } + + // A CDS_TEST rejection with DM_DISPLAYFIXEDOUTPUT declared retries exactly once, with the + // bit cleared and the device's own current value restored, then goes on to apply. + private static void CheckFixedOutputFallback(Checklist checklist) + { + checklist.Lines.Add("A CDS_TEST rejection with DM_DISPLAYFIXEDOUTPUT declared retries exactly once with that field dropped, then applies:"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-FIXEDOUTPUT-FALLBACK"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + Devmode current = BuildDevmode(1920, 1080, 32, 60, 0); // device's own DmDisplayFixedOutput is 0 (Default) + device.SetCurrentMode(deviceName, current); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, (uint)Dmdfo.Center); + + // Only the first CDS_TEST call is queued to fail - the retry, with the bit dropped, is + // left to default to Successful. + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + checklist.Check(result == ResolutionHelper.ResolutionChangeResult.Applied, + string.Format("the retried, successful CDS_TEST is followed through to Applied, got {0}", result)); + + List testCalls = device.CallLog.Where(call => call.Flags == ChangeDisplaySettingsFlags.CdsTest).ToList(); + checklist.Check(testCalls.Count == 2, + string.Format("exactly one retry - two CDS_TEST calls total, got {0}", testCalls.Count)); + + if (testCalls.Count == 2) + { + checklist.Check((testCalls[0].Mode.dmFields & (uint)DevmodeFields.DmDisplayFixedOutput) != 0 && + testCalls[0].Mode.dmDisplayFixedOutput == (uint)Dmdfo.Center, + "the first CDS_TEST call declares DM_DISPLAYFIXEDOUTPUT with the requested (Center) value"); + checklist.Check((testCalls[1].Mode.dmFields & (uint)DevmodeFields.DmDisplayFixedOutput) == 0, + "the retried CDS_TEST call has the DM_DISPLAYFIXEDOUTPUT bit cleared from dmFields"); + checklist.Check(testCalls[1].Mode.dmDisplayFixedOutput == current.dmDisplayFixedOutput, + "the retried CDS_TEST call's dmDisplayFixedOutput is restored to the device's own current value"); + } + + checklist.Check(device.CallLog.Count(call => call.Flags == ChangeDisplaySettingsFlags.CdsUpdateregistry) == 1, + "exactly one CDS_UPDATEREGISTRY call follows the successful retry"); + + checklist.Lines.Add(string.Empty); + } + + // Regression test for notification/log spam: a run of identical failures logs and notifies + // exactly once each, not once per attempt; a REAL success (never a ResetForTests() escape + // hatch - that would make every one of these assertions pass no matter what the code under + // test does) re-opens both, so a different failure code logs again, and even a REPEAT of an + // already-seen code logs again too, proving the reopening is not a one-shot "first new code + // after a success" special case. + private static void CheckNoRepeatNotification(Checklist checklist) + { + checklist.Lines.Add("A run of identical failures logs and notifies exactly once each, not once per attempt - a real success (not a test reset) is what reopens both, for a new failure code or a repeated one (regression test for notification/log spam):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-NOREPEAT"; + Devmode mismatched = BuildDevmode(1920, 1080, 32, 60, 0); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + // Matches `mismatched` on the four controllable fields - driving ChangeResolutionEx + // with this as the target, while the fake's current mode is still `mismatched`, is a + // real AlreadyMatching success, usable at any point to clear this device's state. + ResolutionModeWrapper clearingTarget = BuildTarget(mismatched.dmPelsWidth, mismatched.dmPelsHeight, mismatched.dmBitsPerPel, mismatched.dmDisplayFrequency, 0); + + List raised = new List(); + EventHandler handler = delegate(object sender, ResolutionFailureEventArgs e) { raised.Add(e); }; + ResolutionHelper.ResolutionChangeFailed += handler; + try + { + // Phase A: 20 consecutive attempts, all rejected by CDS_TEST with the same code. + // The revert direction's bound (10) is exceeded partway through, after which every + // further attempt is Suppressed and never reaches the device at all - 20 is + // deliberately more than the bound, to prove going further adds neither a second + // log line nor a second notification. + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, mismatched); + for (int i = 0; i < 20; i++) + { + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + ResolutionHelper.ChangeResolutionEx(device, target, deviceName, true); + } + checklist.Check(raised.Count == 1, + string.Format("20 consecutive identical failures raise ResolutionChangeFailed exactly once, got {0}", raised.Count)); + checklist.Check(ResolutionHelper.LoggedLineCountForTests == 1, + string.Format("20 consecutive identical failures write exactly one log line, got {0}", ResolutionHelper.LoggedLineCountForTests)); + + // Phase B: a REAL success (AlreadyMatching) clears every suppression this device + // was carrying - the fake's current mode never actually changes for this call, only + // the target handed to it does, so this exercises exactly the same "success clears + // the device" path a real recovered driver would take. Then a DIFFERENT failure + // code logs one more line, without a second notification (this fresh streak is only + // 1 failure deep, nowhere near the bound of 10). + ResolutionHelper.ResolutionChangeResult clearingResult1 = ResolutionHelper.ChangeResolutionEx(device, clearingTarget, deviceName, false); + checklist.Check(clearingResult1 == ResolutionHelper.ResolutionChangeResult.AlreadyMatching, + "the first clearing call is itself a real success (AlreadyMatching), not a test-only reset"); + + int loggedLinesBeforePhaseB = ResolutionHelper.LoggedLineCountForTests; + raised.Clear(); + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadmode); + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadmode); + ResolutionHelper.ChangeResolutionEx(device, target, deviceName, true); + checklist.Check(ResolutionHelper.LoggedLineCountForTests == loggedLinesBeforePhaseB + 1, + string.Format("a different failure code, after a real success reopened the device, logs one more line - had {0} log lines before, {1} after", + loggedLinesBeforePhaseB, ResolutionHelper.LoggedLineCountForTests)); + checklist.Check(raised.Count == 0, + "a single new failure, below the give-up bound, does not raise a second notification"); + + // Phase C: another real success, then a REPEAT of phase B's own code (DispChangeBadmode) + // - still logs again, proving the reopening is not limited to "only a brand new code + // works once". + ResolutionHelper.ResolutionChangeResult clearingResult2 = ResolutionHelper.ChangeResolutionEx(device, clearingTarget, deviceName, false); + checklist.Check(clearingResult2 == ResolutionHelper.ResolutionChangeResult.AlreadyMatching, + "the second clearing call is itself a real success (AlreadyMatching), not a test-only reset"); + + int loggedLinesBeforePhaseC = ResolutionHelper.LoggedLineCountForTests; + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadmode); + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadmode); + ResolutionHelper.ChangeResolutionEx(device, target, deviceName, true); + checklist.Check(ResolutionHelper.LoggedLineCountForTests == loggedLinesBeforePhaseC + 1, + string.Format("a repeated failure code, after another real success, logs again instead of staying silent - had {0} log lines before, {1} after", + loggedLinesBeforePhaseC, ResolutionHelper.LoggedLineCountForTests)); + } + finally + { + ResolutionHelper.ResolutionChangeFailed -= handler; + } + + checklist.Lines.Add(string.Empty); + } + + // The apply direction gives up after 3 consecutive failures - after that, the device is + // never called again for this (device, target, direction) until something clears it. + private static void CheckApplyBound(Checklist checklist) + { + checklist.Lines.Add("The apply direction gives up after 3 consecutive failures and stops touching the driver entirely (regression test for the give-up bound):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-APPLYBOUND"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + List raised = new List(); + EventHandler handler = delegate(object sender, ResolutionFailureEventArgs e) { raised.Add(e); }; + ResolutionHelper.ResolutionChangeFailed += handler; + try + { + int callsAfterAttempt3 = -1; + for (int attempt = 1; attempt <= 20; attempt++) + { + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + if (attempt == 3) + { + callsAfterAttempt3 = device.CallLog.Count; + } + } + + checklist.Check(callsAfterAttempt3 > 0 && device.CallLog.Count == callsAfterAttempt3, + string.Format("attempts 4-20 record zero further calls (had {0} after attempt 3, {1} after attempt 20)", + callsAfterAttempt3, device.CallLog.Count)); + checklist.Check(raised.Count == 1 && raised[0].DeviceName == deviceName, + string.Format("exactly one notification is raised, for this device - count={0}", raised.Count)); + } + finally + { + ResolutionHelper.ResolutionChangeFailed -= handler; + } + + checklist.Lines.Add(string.Empty); + } + + // The revert direction is bounded at 10, not 3 - pins the apply/revert asymmetry so it + // cannot be "simplified" away to a single shared constant. + private static void CheckRevertBound(Checklist checklist) + { + checklist.Lines.Add("The revert direction tolerates 10 consecutive failures, not 3 - pins the apply/revert asymmetry (regression test to stop it being simplified away):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-REVERTBOUND"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + int callsAfterAttempt3 = -1; + int callsAfterAttempt9 = -1; + int callsAfterAttempt10 = -1; + for (int attempt = 1; attempt <= 20; attempt++) + { + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + ResolutionHelper.ChangeResolutionEx(device, target, deviceName, true); + if (attempt == 3) callsAfterAttempt3 = device.CallLog.Count; + if (attempt == 9) callsAfterAttempt9 = device.CallLog.Count; + if (attempt == 10) callsAfterAttempt10 = device.CallLog.Count; + } + + checklist.Check(callsAfterAttempt9 > callsAfterAttempt3, + string.Format("the device is still being called past attempt 3 on the revert direction (had {0} calls after attempt 3, {1} after attempt 9)", + callsAfterAttempt3, callsAfterAttempt9)); + checklist.Check(callsAfterAttempt10 > callsAfterAttempt9, + string.Format("attempt 10 itself still reaches the device (had {0} calls after attempt 9, {1} after attempt 10)", + callsAfterAttempt9, callsAfterAttempt10)); + checklist.Check(device.CallLog.Count == callsAfterAttempt10, + string.Format("attempts 11-20 record zero further calls ({0} after attempt 10, {1} after attempt 20)", + callsAfterAttempt10, device.CallLog.Count)); + + checklist.Lines.Add(string.Empty); + } + + // A full apply-then-revert cycle against a fake device returns it to the original mode, via + // the same CDS_TEST-then-CDS_UPDATEREGISTRY sequence both ways. + private static void CheckFullRevertCycle(Checklist checklist) + { + checklist.Lines.Add("A full apply-then-revert cycle against a fake device returns it to the original mode, via the same CDS_TEST-then-CDS_UPDATEREGISTRY sequence both ways:"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-FULLCYCLE"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + Devmode original = BuildDevmode(1920, 1080, 32, 60, 0); + device.SetCurrentMode(deviceName, original); + ResolutionModeWrapper gameTarget = BuildTarget(2560, 1440, 32, 144, 0); + ResolutionModeWrapper originalTarget = BuildTarget(original.dmPelsWidth, original.dmPelsHeight, original.dmBitsPerPel, original.dmDisplayFrequency, original.dmDisplayFixedOutput); + + ResolutionHelper.ResolutionChangeResult applyResult = ResolutionHelper.ChangeResolutionEx(device, gameTarget, deviceName, false); + checklist.Check(applyResult == ResolutionHelper.ResolutionChangeResult.Applied, + string.Format("the apply half returns Applied, got {0}", applyResult)); + checklist.Check(gameTarget.MatchesAchievedMode(device.GetCurrentMode(deviceName)), + "the fake is now at the game's mode"); + + ResolutionHelper.ResolutionChangeResult revertResult = ResolutionHelper.ChangeResolutionEx(device, originalTarget, deviceName, true); + checklist.Check(revertResult == ResolutionHelper.ResolutionChangeResult.Applied, + string.Format("the revert half returns Applied, got {0}", revertResult)); + checklist.Check(originalTarget.MatchesAchievedMode(device.GetCurrentMode(deviceName)), + "the fake's current mode is back to the original on all four controllable fields"); + + checklist.Check( + device.CallLog.Count(call => call.Flags == ChangeDisplaySettingsFlags.CdsTest) == 2 && + device.CallLog.Count(call => call.Flags == ChangeDisplaySettingsFlags.CdsUpdateregistry) == 2, + "both halves went through exactly one CDS_TEST and one CDS_UPDATEREGISTRY call each"); + + checklist.Lines.Add(string.Empty); + } + + // A revert that fails every single time must never strand the user silently: every attempt + // still returns Failed (never Suppressed, since 10 attempts never exceeds the revert + // bound), exactly one notification fires, and it names the device. + private static void CheckFailedRevertDoesNotStrandSilently(Checklist checklist) + { + checklist.Lines.Add("A revert that fails every single time still returns Failed on every attempt, notifies exactly once, and names the device (regression test for a silently stranded desktop resolution):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-STRANDED"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + List raised = new List(); + EventHandler handler = delegate(object sender, ResolutionFailureEventArgs e) { raised.Add(e); }; + ResolutionHelper.ResolutionChangeFailed += handler; + try + { + bool allFailed = true; + for (int attempt = 1; attempt <= 10; attempt++) + { + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + device.QueueResult(deviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx(device, target, deviceName, true); + allFailed = allFailed && result == ResolutionHelper.ResolutionChangeResult.Failed; + } + + checklist.Check(allFailed, + "every one of the 10 attempts returns Failed, not Suppressed - the revert direction never gives up silently within its own bound"); + checklist.Check(raised.Count == 1 && raised[0].IsRevert && raised[0].DeviceName == deviceName, + string.Format("exactly one notification is raised, for a revert, naming device {0}", deviceName)); + checklist.Check(ResolutionHelper.LoggedLineCountForTests >= 1, + "at least one log line documents the failure"); + } + finally + { + ResolutionHelper.ResolutionChangeFailed -= handler; + } + + checklist.Lines.Add(string.Empty); + } + + // Regression test for #132's "it keeps on saying that": a driver that silently pins + // DmDisplayFixedOutput away from whatever was requested must not re-fire a real mode set + // and registry write forever - IsResolutionChangeNeeded has to settle to false once the + // four controllable fields genuinely converge, and the total number of driver calls across + // many simulated foreground events has to stay small. + private static void CheckUnachievableFixedOutputLoopIsBounded(Checklist checklist) + { + checklist.Lines.Add("A driver that silently pins DmDisplayFixedOutput away from the requested value does not re-fire a mode change forever (regression test for #132's 'it keeps on saying that'):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-PINNEDFIXEDOUTPUT"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + device.PinFixedOutputTo(deviceName, 0); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, (uint)Dmdfo.Center); + + bool everFalse = false; + for (int i = 0; i < 20; i++) + { + if (ResolutionHelper.IsResolutionChangeNeeded(device, deviceName, target)) + { + ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + } + else + { + everFalse = true; + } + } + + checklist.Check(everFalse, + "IsResolutionChangeNeeded returns false once the four controllable fields match, even though DmDisplayFixedOutput never reaches the requested value"); + checklist.Check(target.MatchesAchievedMode(device.GetCurrentMode(deviceName)), + "the four controllable fields did genuinely converge to the target"); + checklist.Check(device.CallLog.Count <= 3, + string.Format("total ChangeMode calls across all 20 simulated events is at most 3, got {0}", device.CallLog.Count)); + + checklist.Lines.Add(string.Empty); + } + + // AlreadyMatching must short-circuit before ever touching the driver. + private static void CheckAlreadyMatchingShortCircuits(Checklist checklist) + { + checklist.Lines.Add("A target that already matches the current mode returns AlreadyMatching without calling ChangeMode at all:"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-ALREADYMATCHING"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(deviceName, BuildDevmode(2560, 1440, 32, 144, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + + checklist.Check(result == ResolutionHelper.ResolutionChangeResult.AlreadyMatching, + string.Format("ChangeResolutionEx returns AlreadyMatching, got {0}", result)); + checklist.Check(device.CallLog.Count == 0, + string.Format("zero ChangeMode calls are recorded, got {0}", device.CallLog.Count)); + + checklist.Lines.Add(string.Empty); + } + + // Regression test for ClearFailureState's separator-guarded prefix match: without a + // separator character that cannot appear in a device name, a success on + // "FAKE-RES-PREFIX1" would ALSO match every stored key that actually belongs to + // "FAKE-RES-PREFIX10" - a real string prefix, just not the intended one - and wipe that + // other device's failure state too. + private static void CheckClearFailureStateDoesNotStraddleDeviceNames(Checklist checklist) + { + checklist.Lines.Add("A success on one device does not clear failure state belonging to a different device whose name is a string-prefix of it (regression test for ClearFailureState's key separator):"); + ResolutionHelper.ResetForTests(); + + const string shortDeviceName = "FAKE-RES-PREFIX1"; + const string longDeviceName = "FAKE-RES-PREFIX10"; // shortDeviceName is a literal string prefix of this + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + device.SetCurrentMode(shortDeviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + device.SetCurrentMode(longDeviceName, BuildDevmode(1920, 1080, 32, 60, 0)); + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + List raised = new List(); + EventHandler handler = delegate(object sender, ResolutionFailureEventArgs e) { raised.Add(e); }; + ResolutionHelper.ResolutionChangeFailed += handler; + try + { + // The long device name fails twice (the apply bound is 3) - one more failure would + // give up. + for (int i = 0; i < 2; i++) + { + device.QueueResult(longDeviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + device.QueueResult(longDeviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + ResolutionHelper.ChangeResolutionEx(device, target, longDeviceName, false); + } + + // The short device name succeeds (AlreadyMatching) - must clear only its own state. + ResolutionModeWrapper shortMatchingTarget = BuildTarget(1920, 1080, 32, 60, 0); + ResolutionHelper.ResolutionChangeResult shortResult = ResolutionHelper.ChangeResolutionEx(device, shortMatchingTarget, shortDeviceName, false); + checklist.Check(shortResult == ResolutionHelper.ResolutionChangeResult.AlreadyMatching, + "the short device name's clearing call is itself a real success"); + + // If the long device name's count had been wiped by the short device name's + // success, it would take 3 MORE failures (a fresh streak) to give up - not 1. + device.QueueResult(longDeviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + device.QueueResult(longDeviceName, ChangeDisplaySettingsFlags.CdsTest, DispChange.DispChangeBadflags); + ResolutionHelper.ChangeResolutionEx(device, target, longDeviceName, false); + + checklist.Check(raised.Count == 1 && raised[0].DeviceName == longDeviceName, + string.Format("the long device name's failure streak survived the short device name's success - one more failure (its 3rd) reaches the apply bound and gives up, got {0} notification(s)", raised.Count)); + } + finally + { + ResolutionHelper.ResolutionChangeFailed -= handler; + } + + checklist.Lines.Add(string.Empty); + } + + // Regression test for step 1 (an unreadable current mode) being its own failure category, + // separate from a rejected mode change: it must return Failed every time and never be + // counted toward the give-up bound, however many times it recurs - a subtle distinction to + // preserve, since routing it through the same accounting as a real rejection would silently + // start giving up on (and Suppressing) a device this class never even tried to change. + private static void CheckUnreadableCurrentModeNeverCountsTowardGiveUp(Checklist checklist) + { + checklist.Lines.Add("An unreadable current mode returns Failed every time and never counts toward the give-up bound (regression test for step 1's separate, uncounted failure category):"); + ResolutionHelper.ResetForTests(); + + const string deviceName = "FAKE-RES-UNREADABLE"; + FakeDisplayModeDevice device = new FakeDisplayModeDevice(); + // Deliberately never calls SetCurrentMode - TryGetCurrentMode fails for every call. + ResolutionModeWrapper target = BuildTarget(2560, 1440, 32, 144, 0); + + List raised = new List(); + EventHandler handler = delegate(object sender, ResolutionFailureEventArgs e) { raised.Add(e); }; + ResolutionHelper.ResolutionChangeFailed += handler; + try + { + bool allFailed = true; + for (int attempt = 1; attempt <= 20; attempt++) + { + ResolutionHelper.ResolutionChangeResult result = ResolutionHelper.ChangeResolutionEx(device, target, deviceName, false); + allFailed = allFailed && result == ResolutionHelper.ResolutionChangeResult.Failed; + } + + checklist.Check(allFailed, + "all 20 attempts return Failed, never Suppressed - an unreadable current mode is never counted toward the apply bound (3), however many times it recurs"); + checklist.Check(device.CallLog.Count == 0, + "TryGetCurrentMode failing means ChangeMode (CDS_TEST/CDS_UPDATEREGISTRY) is never even reached"); + checklist.Check(raised.Count == 0, + "an unreadable current mode never raises ResolutionChangeFailed - it is a different failure category from a rejected mode change"); + checklist.Check(ResolutionHelper.LoggedLineCountForTests == 1, + string.Format("still deduped to exactly one log line across all 20 attempts, got {0}", ResolutionHelper.LoggedLineCountForTests)); + } + finally + { + ResolutionHelper.ResolutionChangeFailed -= handler; + } + + checklist.Lines.Add(string.Empty); + } + + // ------------------------------------------------------------------ + // Shared helpers. + // ------------------------------------------------------------------ + + // Stands in for what a real EnumDisplaySettings call returns for the current mode. + // dmPosition is set to a non-zero, distinctive value and DM_POSITION is the only bit + // declared here - deliberately NOT the four fields ChangeResolutionEx itself controls, so + // CheckDmFieldsDeclared can tell "ApplyTargetFields actually declared these" from "they + // happened to already be set on the current mode" (a real EnumDisplaySettings result would + // typically carry both, but folding them into this baseline too would make that check + // unable to fail no matter what ApplyTargetFields does - see CheckDmFieldsDeclared). + private static Devmode BuildDevmode(uint width, uint height, uint bpp, uint freq, uint fixedOutput) + { + Devmode mode = new Devmode(); + mode.dmSize = (ushort)Marshal.SizeOf(mode); + mode.dmPosition = new Pointl(); + mode.dmPosition.x = 100; + mode.dmPosition.y = 200; + mode.dmPelsWidth = width; + mode.dmPelsHeight = height; + mode.dmBitsPerPel = bpp; + mode.dmDisplayFrequency = freq; + mode.dmDisplayFixedOutput = fixedOutput; + mode.dmFields = (uint)DevmodeFields.DmPosition; + return mode; + } + + private static ResolutionModeWrapper BuildTarget(uint width, uint height, uint bpp, uint freq, uint fixedOutput) + { + ResolutionModeWrapper target = new ResolutionModeWrapper(); + target.DmPelsWidth = width; + target.DmPelsHeight = height; + target.DmBitsPerPel = bpp; + target.DmDisplayFrequency = freq; + target.DmDisplayFixedOutput = fixedOutput; + return target; + } + + // Stands in for a real display: an in-memory current Devmode per fake device name, plus a + // per-(device,flags) one-shot queue of forced ChangeMode results and a full call log in + // order. Device names used by the checks above are chosen to never collide with a real + // Screen.DeviceName ("\\.\DISPLAYn"), since ResolutionHelper's own failure-tracking + // dictionaries are static and would otherwise be shared with any real hardware use in the + // same process. + private class FakeDisplayModeDevice : IDisplayModeDevice + { + public struct RecordedCall + { + public readonly string DeviceName; + public readonly ChangeDisplaySettingsFlags Flags; + public readonly Devmode Mode; + + public RecordedCall(string deviceName, ChangeDisplaySettingsFlags flags, Devmode mode) + { + DeviceName = deviceName; + Flags = flags; + Mode = mode; + } + } + + private readonly Dictionary _currentModes = new Dictionary(); + private readonly Dictionary> _modeTables = new Dictionary>(); + private readonly Dictionary> _queuedResults = new Dictionary>(); + private readonly Dictionary _pinnedFixedOutput = new Dictionary(); + private readonly HashSet _suppressNextApply = new HashSet(); + + public readonly List CallLog = new List(); + + public void SetCurrentMode(string deviceName, Devmode mode) + { + _currentModes[deviceName] = mode; + } + + public Devmode GetCurrentMode(string deviceName) + { + return _currentModes[deviceName]; + } + + // Forces the NEXT ChangeMode call for (deviceName, flags) to return result instead of + // the default DispChangeSuccessful - one-shot, first in first out, so a check that + // needs the same (deviceName, flags) pair to fail more than once queues it that many + // times. + public void QueueResult(string deviceName, ChangeDisplaySettingsFlags flags, DispChange result) + { + string key = QueueKey(deviceName, flags); + Queue queue; + if (!_queuedResults.TryGetValue(key, out queue)) + { + queue = new Queue(); + _queuedResults[key] = queue; + } + queue.Enqueue(result); + } + + // Stands in for a driver that silently ignores the requested DmDisplayFixedOutput and + // always reports back its own value instead - CheckUnachievableFixedOutputLoopIsBounded's + // scenario. Applies to every CDS_UPDATEREGISTRY call that actually lands, not a + // one-shot. + public void PinFixedOutputTo(string deviceName, uint value) + { + _pinnedFixedOutput[deviceName] = value; + } + + // One-shot: the next applying CDS_UPDATEREGISTRY call for deviceName is still recorded + // and still returns whatever result was queued (or Successful by default), but does + // NOT mutate the stored current mode - standing in for a driver that reports success + // without the mode actually having changed, for CheckPostApplyVerification. + public void SuppressNextApply(string deviceName) + { + _suppressNextApply.Add(deviceName); + } + + private static string QueueKey(string deviceName, ChangeDisplaySettingsFlags flags) + { + return deviceName + "|" + flags; + } + + public bool TryGetCurrentMode(string deviceName, out Devmode mode) + { + return _currentModes.TryGetValue(deviceName, out mode); + } + + public bool TryEnumerateMode(string deviceName, int modeNum, out Devmode mode) + { + List table; + if (_modeTables.TryGetValue(deviceName, out table) && modeNum >= 0 && modeNum < table.Count) + { + mode = table[modeNum]; + return true; + } + mode = new Devmode(); + return false; + } + + public DispChange ChangeMode(string deviceName, Devmode mode, ChangeDisplaySettingsFlags flags) + { + CallLog.Add(new RecordedCall(deviceName, flags, mode)); + + DispChange result = DispChange.DispChangeSuccessful; + string key = QueueKey(deviceName, flags); + Queue queue; + if (_queuedResults.TryGetValue(key, out queue) && queue.Count > 0) + { + result = queue.Dequeue(); + } + + bool wouldApply = (flags & ChangeDisplaySettingsFlags.CdsUpdateregistry) != 0 && + (result == DispChange.DispChangeSuccessful || result == DispChange.DispChangeNotupdated); + + if (wouldApply && !_suppressNextApply.Remove(deviceName)) + { + Devmode applied = mode; + uint pinned; + if (_pinnedFixedOutput.TryGetValue(deviceName, out pinned)) + { + applied.dmDisplayFixedOutput = pinned; + } + _currentModes[deviceName] = applied; + } + + return result; + } + } + + private class Checklist + { + public readonly List Lines = new List(); + public int Passed; + public int Total; + + // No Skip() here. This fixture has no hardware half, by design, and must never + // gain one - a gamma ramp has a guaranteed undo and a display mode does not - so + // there is no path that would ever need to skip a check instead of running it. + public void Check(bool condition, string description) + { + Total++; + if (condition) + Passed++; + Lines.Add(string.Format("[{0}] {1}", condition ? "PASS" : "FAIL", description)); + } + } + } +} diff --git a/vibrance.GUI/common/ResolutionHelper.cs b/vibrance.GUI/common/ResolutionHelper.cs index d26240f..8afd7b6 100644 --- a/vibrance.GUI/common/ResolutionHelper.cs +++ b/vibrance.GUI/common/ResolutionHelper.cs @@ -3,25 +3,99 @@ using System.Linq; using System.Runtime.InteropServices; using System.Text; -using System.Windows.Forms; namespace vibrance.GUI.common { + // The seam between the state/failure-handling logic below and the actual display driver. + // RealDisplayModeDevice (bottom of this file) is the only production implementation; + // ResolutionChangeFixture supplies a fake so the retry/bound/notification state a + // foreground-change storm can drive through it is exercised via real apply/revert cycles, + // including forced failures at every step, without ever touching a real display. + internal interface IDisplayModeDevice + { + bool TryGetCurrentMode(string deviceName, out Devmode mode); + bool TryEnumerateMode(string deviceName, int modeNum, out Devmode mode); + + // Devmode is a value type and is taken here BY VALUE, deliberately not "ref" - a fake + // implementation has to be able to record exactly what it was handed (dmFields above all) + // without the caller's own copy changing underneath it afterward. RealDisplayModeDevice + // copies to a local before taking "ref" of THAT, for the P/Invoke, which needs "ref". + DispChange ChangeMode(string deviceName, Devmode mode, ChangeDisplaySettingsFlags flags); + } + + // Raised at most once per (device, target) while a failure streak is ongoing - specifically at + // the moment ChangeResolutionEx gives up and starts returning Suppressed, not on every + // individual failed attempt. A user does not need a balloon for a single transient + // ChangeDisplaySettingsEx failure that resolves itself on the next foreground switch; they do + // need one the moment vibranceGUI has stopped trying, especially on the revert side, where + // giving up means the desktop is stuck at the game's resolution until they act. There is + // deliberately no "IsGivingUp" flag here - a give-up is the ONLY reason this event is ever + // raised (see RecordFailure below), so a field that would always read true carries no + // information; if a future change makes this event fire on a non-give-up failure too, add the + // flag back then, with a real false case to go with it. + public class ResolutionFailureEventArgs : EventArgs + { + public string DeviceName { get; private set; } + public ResolutionModeWrapper Target { get; private set; } + public DispChange FailureCode { get; private set; } + public bool IsRevert { get; private set; } + + internal ResolutionFailureEventArgs(string deviceName, ResolutionModeWrapper target, DispChange failureCode, bool isRevert) + { + DeviceName = deviceName; + Target = target; + FailureCode = failureCode; + IsRevert = isRevert; + } + } + class ResolutionHelper { - private const int EnumCurrentSettings = -1; + // What a single ChangeResolutionEx call actually did - deliberately not a bool. + // AlreadyMatching and Suppressed both mean "nothing was sent to the driver this time", for + // two different reasons a caller needs to tell apart: AlreadyMatching means the mode is + // already right, Suppressed means this (device, target, direction) has failed too many + // times in a row and is being left alone until something (a success, or ResetForTests) + // clears it. AppliedUnverified is its own case, not a flavour of Failed: it means + // CDS_UPDATEREGISTRY itself reported success (or Notupdated) but the post-apply readback + // did not confirm it - the mode most likely DID change, so a caller (the proxies) should + // keep treating it as applied for the purpose of a later revert attempt, rather than + // writing off a change that plausibly landed. Nested (not a sibling top-level type) so + // callers write ResolutionHelper.ResolutionChangeResult.Applied, matching + // ResolutionChangeResult's own callers throughout the proxies and the fixture. + public enum ResolutionChangeResult + { + Applied, + AppliedUnverified, + AlreadyMatching, + Failed, + Suppressed + } - [DllImport("user32.dll")] - public static extern bool EnumDisplaySettings(string deviceName, int modeNum, ref Devmode devMode); + private const int EnumCurrentSettings = -1; + // How many consecutive failures ChangeResolutionEx tolerates for one (device, target, + // direction) before it stops calling the driver at all and starts returning Suppressed. + // Apply and revert use different bounds on purpose. Giving up on an apply only leaves the + // user at their own Windows resolution - the side it is safe to fail toward. Giving up on + // a revert leaves the desktop stuck at the GAME's resolution, with no other code path that + // will ever try again, so it is worth trying far longer before accepting that outcome. + private const int ApplyFailureBound = 3; + private const int RevertFailureBound = 10; + + // Bits ChangeResolutionEx is willing to declare in dmFields on top of whatever + // EnumDisplaySettings already reported for the current mode - see ApplyTargetFields, which + // ORs this in rather than overwriting dmFields outright. Dropping bits EnumDisplaySettings + // already set - DM_POSITION above all - would risk a multi-monitor desktop rearranging + // itself the next time a mode change runs. Not a confirmed mechanism for any specific + // upstream report (the pre-fix code never touched dmFields at all, so this was never a + // live cause of anything filed) - just the correctness this class owes every DEVMODE it + // hands to ChangeDisplaySettingsEx, going forward. + private const DevmodeFields OwnedFields = DevmodeFields.DmPelsWidth | DevmodeFields.DmPelsHeight | + DevmodeFields.DmBitsPerPel | DevmodeFields.DmDisplayFrequency | DevmodeFields.DmDisplayFixedOutput; - [DllImport("User32.dll")] - [return: MarshalAs(UnmanagedType.I4)] - private static extern int ChangeDisplaySettings( - [In, Out] - ref Devmode lpDevMode, - [param: MarshalAs(UnmanagedType.U4)] - uint dwflags); + [DllImport("user32.dll")] + private static extern bool EnumDisplaySettings(string deviceName, int modeNum, ref Devmode devMode); [DllImport("user32.dll")] private static extern DispChange ChangeDisplaySettingsEx( @@ -31,27 +105,45 @@ private static extern DispChange ChangeDisplaySettingsEx( ChangeDisplaySettingsFlags dwflags, IntPtr lParam); - [DllImport("user32.dll")] - public static extern DispChange ChangeDisplaySettingsEx( - string lpszDeviceName, - IntPtr lpDevMode, - IntPtr hwnd, - ChangeDisplaySettingsFlags dwflags, - IntPtr lParam); - - + // The only production IDisplayModeDevice. Everything below is exercised against this by + // the public, hardware-touching overloads; ResolutionChangeFixture drives the internal + // overloads with its own fake instead. + private static readonly IDisplayModeDevice _realDevice = new RealDisplayModeDevice(); + + public static event EventHandler ResolutionChangeFailed; + + // Consecutive-failure counts, keyed by device + target + direction (BuildFailureKey below) + // - direction is part of the key because apply and revert are bounded differently (see + // ApplyFailureBound/RevertFailureBound) and a device can legitimately be mid-streak on one + // direction while the other has never failed at all. + private static readonly Dictionary _consecutiveFailures = new Dictionary(); + + // Suppresses repeat log lines for a (device, DispChange) pair that keeps failing the same + // way - without this, a foreground-change storm against a broken device would write a line + // to the log on every single event. Deliberately keyed by failure code too, not just + // device: a device that starts failing with a DIFFERENT code is new diagnostic information + // and still gets its own line, not just a repeat of the first one. + private static readonly HashSet _loggedFailures = new HashSet(); + + // One ResolutionChangeFailed raise per (device, target) while a give-up streak is ongoing - + // see the class-level comment on ResolutionFailureEventArgs for why the raise itself is + // deferred to the moment of giving up rather than fired on the first failure. + private static readonly HashSet _notifiedFailures = new HashSet(); + + // Test-only counter of distinct log lines this class has actually written (i.e. every time + // the _loggedFailures dedup check below passes). A real log write has no return value or + // other observable signal, and ResolutionChangeFixture's design asks it to assert exact + // log-line counts - this is the seam that makes that possible without reading the real, + // shared log file. Reset by ResetForTests(). + internal static int LoggedLineCountForTests; + + /// + /// Reads the current mode for lpszDeviceName. False (no exception) is EnumDisplaySettings's + /// own way of reporting failure - see RealDisplayModeDevice. + /// public static bool GetCurrentResolutionSettings(out Devmode mode, string lpszDeviceName) { - mode = new Devmode(); - mode.dmSize = (ushort)Marshal.SizeOf(mode); - mode.dmDriverExtra = 0; - - if (EnumDisplaySettings(lpszDeviceName, EnumCurrentSettings, ref mode) == true) - { - return true; - } - - return false; + return _realDevice.TryGetCurrentMode(lpszDeviceName, out mode); } public static List EnumerateSupportedResolutionModes() @@ -62,67 +154,367 @@ public static List EnumerateSupportedResolutionModes() public static List EnumerateSupportedResolutionModes(string deviceName) { List resolutionList = new List(); - Devmode mode = new Devmode(); - mode.dmSize = (ushort)Marshal.SizeOf(mode); - + Devmode mode; int index = 0; - while (EnumDisplaySettings(deviceName, index++, ref mode) == true) + while (_realDevice.TryEnumerateMode(deviceName, index++, out mode)) { resolutionList.Add(new ResolutionModeWrapper(mode)); } - return resolutionList; } - public static bool ChangeResolution(ResolutionModeWrapper resolutionMode) + /// + /// True when deviceName's current mode does not yet match target on the four fields + /// ChangeResolutionEx actually controls and verifies - see + /// ResolutionModeWrapper.MatchesAchievedMode for why DmDisplayFixedOutput is deliberately + /// excluded from that comparison, and why that is what stops a "(Center)" mode selection + /// from re-firing a real mode change on every single foreground event forever. + /// + public static bool IsResolutionChangeNeeded(string deviceName, ResolutionModeWrapper target) + { + return IsResolutionChangeNeeded(_realDevice, deviceName, target); + } + + internal static bool IsResolutionChangeNeeded(IDisplayModeDevice device, string deviceName, ResolutionModeWrapper target) + { + if (target == null) + { + return false; + } + Devmode currentMode; + if (!device.TryGetCurrentMode(deviceName, out currentMode)) + { + // Nothing to compare against - matches the pre-fix callers, which treated an + // unreadable current mode as "no change needed" rather than forcing one blind. + return false; + } + return !target.MatchesAchievedMode(currentMode); + } + + /// + /// Clears every recorded failure, log-suppression and notification-suppression entry, and + /// the log-line counter - for test isolation only. Deliberately does not touch + /// ResolutionChangeFailed's subscriber list: production code (VibranceGUI) subscribes once + /// for the life of the form, and ResolutionChangeFixture's own checks install and remove + /// their own handler around each check instead of relying on this to detach one for them. + /// + internal static void ResetForTests() + { + _consecutiveFailures.Clear(); + _loggedFailures.Clear(); + _notifiedFailures.Clear(); + LoggedLineCountForTests = 0; + } + + /// + /// Drives a real display mode change (or revert) against deviceName toward target. Never + /// shows a MessageBox and never blocks - see the internal overload below for the exact + /// sequence and for why CDS_TEST-then-CDS_UPDATEREGISTRY replaced the old + /// CDS_UPDATEREGISTRY|CDS_NORESET staged-commit pattern. + /// + public static ResolutionChangeResult ChangeResolutionEx(ResolutionModeWrapper target, string deviceName, bool isRevert) + { + return ChangeResolutionEx(_realDevice, target, deviceName, isRevert); + } + + // The seam ResolutionChangeFixture drives directly - see ChangeResolutionEx's public + // overload above for why. + // + // CDS_TEST (validate only) is tried first and CDS_UPDATEREGISTRY (apply AND persist) is + // tried only once that passes - never CDS_UPDATEREGISTRY|CDS_NORESET followed by a second, + // separate commit call. That old two-call pattern left a reachable state where the first + // call could fail (or write a mode that never actually gets confirmed) while the second, + // unconditional commit call ran anyway with its own return value discarded - there is no + // window here where the registry can hold a mode that was never actually applied, because + // CDS_UPDATEREGISTRY both applies AND persists in one authoritative call. The trade-off: + // CDS_UPDATEREGISTRY gets no 15-second revert-if-unconfirmed safety net the way an + // interactive Windows Settings resolution change does, which is exactly why CDS_TEST runs + // first - a mode the driver would reject is caught before anything is written at all. + internal static ResolutionChangeResult ChangeResolutionEx(IDisplayModeDevice device, ResolutionModeWrapper target, string deviceName, bool isRevert) { - Devmode mode = new Devmode(); - if (GetCurrentResolutionSettings(out mode, null)) + if (target == null) { - mode.dmPelsWidth = resolutionMode.DmPelsWidth; - mode.dmPelsHeight = resolutionMode.DmPelsHeight; - mode.dmBitsPerPel = resolutionMode.DmBitsPerPel; - mode.dmDisplayFrequency = resolutionMode.DmDisplayFrequency; - mode.dmDisplayFixedOutput = resolutionMode.DmDisplayFixedOutput; - - DispChange returnValue = (DispChange)ChangeDisplaySettings(ref mode, 0); - if (DispChange.DispChangeSuccessful == returnValue) + // Defensive only - every real call site already gates on IsResolutionChangeNeeded, + // which itself returns false for a null target, so this is never reached through + // NvidiaDynamicVibranceProxy/AmdDynamicVibranceProxy. + return ResolutionChangeResult.Failed; + } + + int bound = isRevert ? RevertFailureBound : ApplyFailureBound; + string failureKey = BuildFailureKey(deviceName, target, isRevert); + + int priorFailures; + if (_consecutiveFailures.TryGetValue(failureKey, out priorFailures) && priorFailures >= bound) + { + // Already given up on this exact (device, target, direction) - do not touch the + // driver again until something clears it. In practice that means a success on a + // DIFFERENT target/direction for the same device (ClearFailureState clears the + // whole device, see below) - once THIS key is suppressed, the driver is never + // called for it again through the normal path above, so it can no longer produce a + // success of its own; the only other way out is ResetForTests(). This is what keeps + // a persistently failing revert from re-running the same doomed mode set on every + // single foreground event forever. + return ResolutionChangeResult.Suppressed; + } + + // Step 1. + Devmode currentMode; + if (!device.TryGetCurrentMode(deviceName, out currentMode)) + { + // Deliberately does not touch _consecutiveFailures/_notifiedFailures - an unreadable + // current mode is a different failure category from a rejected mode change, and is + // not counted toward the give-up bound. Still deduped so a persistently unreadable + // device logs once, not on every foreground event. + if (_loggedFailures.Add(DeviceKey(deviceName, "read-current-mode"))) { - return true; + LoggedLineCountForTests++; + Program.LogSafely(string.Format("Failed to read the current resolution for screen {0}, refusing to change it", deviceName)); } - else + return ResolutionChangeResult.Failed; + } + + // Step 2. + if (target.MatchesAchievedMode(currentMode)) + { + ClearFailureState(deviceName); + return ResolutionChangeResult.AlreadyMatching; + } + + // Step 3. Never mutates currentMode - it is still needed below (both as "the device's + // own value" for the fixed-output fallback, and to build the failure key it already + // contributed to above). + Devmode desiredMode = currentMode; + ApplyTargetFields(ref desiredMode, target); + + // Step 4. + DispChange testResult = device.ChangeMode(deviceName, desiredMode, ChangeDisplaySettingsFlags.CdsTest); + if (testResult != DispChange.DispChangeSuccessful && + (desiredMode.dmFields & (uint)DevmodeFields.DmDisplayFixedOutput) != 0 && + target.DmDisplayFixedOutput != currentMode.dmDisplayFixedOutput) + { + // A driver that cannot honour the requested scaling/centering behaviour may reject + // CDS_TEST outright rather than silently ignoring the field - retried exactly once, + // with the bit dropped and the device's OWN current value restored, so a scaling + // preference the driver cannot honour does not also block the four fields it can. + // OwnedFields declares DM_DISPLAYFIXEDOUTPUT unconditionally (see its own comment), + // so without the value-differs check above this retry would fire on every single + // rejection, including the common case where the requested value already matches + // the device's own - dropping the bit there changes nothing observable, so it is + // never worth a second driver call. + desiredMode.dmFields &= ~(uint)DevmodeFields.DmDisplayFixedOutput; + desiredMode.dmDisplayFixedOutput = currentMode.dmDisplayFixedOutput; + testResult = device.ChangeMode(deviceName, desiredMode, ChangeDisplaySettingsFlags.CdsTest); + } + + if (testResult != DispChange.DispChangeSuccessful) + { + return RecordFailure(deviceName, target, isRevert, bound, testResult); + } + + // Step 5. + DispChange applyResult = device.ChangeMode(deviceName, desiredMode, ChangeDisplaySettingsFlags.CdsUpdateregistry); + if (applyResult == DispChange.DispChangeNotupdated) + { + // Nothing was written to the registry, but the mode is live regardless - not a + // failure, just worth a diagnostic line the first time it happens for this device. + if (_loggedFailures.Add(DeviceKey(deviceName, applyResult.ToString()))) { - MessageBox.Show("Changing the resolution failed: " + Enum.GetName(typeof(DispChange), returnValue)); + LoggedLineCountForTests++; + Program.LogSafely(string.Format( + "Resolution change for screen {0} reported no update was written to the registry; treating the mode as live and verifying it", deviceName)); } } - return false; + else if (applyResult != DispChange.DispChangeSuccessful) + { + return RecordFailure(deviceName, target, isRevert, bound, applyResult); + } + + // Step 6. CDS_TEST passing is not proof the mode actually took - confirmed by reading + // it back, the same way a caller would have to if this class offered no confirmation + // of its own. + Devmode achievedMode; + bool readAchievedMode = device.TryGetCurrentMode(deviceName, out achievedMode); + if (!readAchievedMode || !target.MatchesAchievedMode(achievedMode)) + { + // CDS_UPDATEREGISTRY itself reported success (or Notupdated) to get here - a + // genuine driver rejection already returned Failed above, at step 4 or step 5, and + // never reaches this point. So this is NOT the same failure category: the mode most + // likely did change, and a caller that treated it as a plain Failed (clearing + // isResolutionChangeApplied) would strand the desktop while telling the user the + // opposite of what happened. AppliedUnverified keeps the same failure-count/log/ + // notify accounting - a driver that never confirms is still worth eventually giving + // up on - but returns a result the proxies keep treating as applied. See + // RecordUnverifiedApply for the log line, which reports the achieved-vs-target + // values instead of a synthetic DispChange code that would misrepresent this as a + // driver rejection. + return RecordUnverifiedApply(deviceName, target, isRevert, bound, readAchievedMode, achievedMode); + } + + ClearFailureState(deviceName); + return ResolutionChangeResult.Applied; } - public static bool ChangeResolutionEx(ResolutionModeWrapper resolutionMode, string lpszDeviceName) + // OR's target's four controllable fields' bits into whatever dmFields EnumDisplaySettings + // already returned for the current mode - never overwrites dmFields outright. See + // OwnedFields for why DM_POSITION (and anything else already set) must survive untouched. + private static void ApplyTargetFields(ref Devmode mode, ResolutionModeWrapper target) { - Devmode mode = new Devmode(); - if (GetCurrentResolutionSettings(out mode, lpszDeviceName)) - { - mode.dmPelsWidth = resolutionMode.DmPelsWidth; - mode.dmPelsHeight = resolutionMode.DmPelsHeight; - mode.dmBitsPerPel = resolutionMode.DmBitsPerPel; - mode.dmDisplayFrequency = resolutionMode.DmDisplayFrequency; - mode.dmDisplayFixedOutput = resolutionMode.DmDisplayFixedOutput; + mode.dmPelsWidth = target.DmPelsWidth; + mode.dmPelsHeight = target.DmPelsHeight; + mode.dmBitsPerPel = target.DmBitsPerPel; + mode.dmDisplayFrequency = target.DmDisplayFrequency; + mode.dmDisplayFixedOutput = target.DmDisplayFixedOutput; + mode.dmFields |= (uint)OwnedFields; + } + // Step 7 from the design: increments the consecutive-failure count for this exact + // (device, target, direction), logs at most once per (device, dedup key), and raises + // ResolutionChangeFailed at most once per (device, target) - specifically at the attempt + // where the count first reaches the bound. Below the bound, the failure is logged (if new) + // but nothing is raised - a single transient rejection is not worth a balloon tip; only + // genuinely giving up is (which is also why ResolutionFailureEventArgs carries no + // "IsGivingUp" flag - a give-up is the only reason this is ever raised). + // + // Shared by RecordFailure (a genuine driver rejection, step 4/5) and RecordUnverifiedApply + // (step 6 - CDS_UPDATEREGISTRY itself reported success, but the readback did not confirm + // it), which differ only in the dedup key, the log message and the event's FailureCode - + // and, at the call sites, in the ResolutionChangeResult returned around this accounting. + private static void RecordFailureAccounting(string deviceName, ResolutionModeWrapper target, bool isRevert, int bound, + string logDedupSuffix, string logMessage, DispChange eventFailureCode) + { + string failureKey = BuildFailureKey(deviceName, target, isRevert); + int count; + _consecutiveFailures.TryGetValue(failureKey, out count); + count++; + _consecutiveFailures[failureKey] = count; - DispChange returnValue = (DispChange)ChangeDisplaySettingsEx(lpszDeviceName, ref mode, IntPtr.Zero, (ChangeDisplaySettingsFlags.CdsUpdateregistry | ChangeDisplaySettingsFlags.CdsNoreset), IntPtr.Zero); - ChangeDisplaySettingsEx(null, IntPtr.Zero, (IntPtr)null, ChangeDisplaySettingsFlags.CdsNone, (IntPtr)null); + if (_loggedFailures.Add(DeviceKey(deviceName, logDedupSuffix))) + { + LoggedLineCountForTests++; + Program.LogSafely(logMessage); + } - if (DispChange.DispChangeSuccessful == returnValue) + bool isGivingUp = count >= bound; + if (isGivingUp && _notifiedFailures.Add(DeviceKey(deviceName, DescribeTarget(target)))) + { + EventHandler handler = ResolutionChangeFailed; + if (handler != null) { - return true; + handler(null, new ResolutionFailureEventArgs(deviceName, target, eventFailureCode, isRevert)); } - else + } + } + + // A genuine driver rejection at step 4 (CDS_TEST) or step 5 (CDS_UPDATEREGISTRY) - code is + // the real DispChange the driver returned, used both as the log's dedup key and in the + // message, so a device that starts failing with a DIFFERENT code still gets its own line. + private static ResolutionChangeResult RecordFailure(string deviceName, ResolutionModeWrapper target, bool isRevert, int bound, DispChange code) + { + RecordFailureAccounting(deviceName, target, isRevert, bound, + code.ToString(), + string.Format("{0} the resolution for screen {1} failed: {2}", isRevert ? "Restoring" : "Changing", deviceName, code), + code); + return ResolutionChangeResult.Failed; + } + + // Step 6's readback did not confirm a CDS_UPDATEREGISTRY call that itself reported success + // (or Notupdated) - shares RecordFailure's counting/notification machinery (a driver that + // never confirms is still worth eventually giving up on), but under its own dedup key, so + // it never collides with a genuine DispChangeFailed logged elsewhere for the same device, + // and with a log line that reports the achieved-vs-target values instead of a synthetic + // DispChange code that would misrepresent this as a driver rejection. readAchievedMode + // distinguishes "read the wrong mode back" from "couldn't even read it this time" in that + // message; achievedMode is only meaningful when readAchievedMode is true. + private static ResolutionChangeResult RecordUnverifiedApply(string deviceName, ResolutionModeWrapper target, bool isRevert, int bound, + bool readAchievedMode, Devmode achievedMode) + { + string message = readAchievedMode + ? string.Format( + "{0} the resolution for screen {1} reported success but a readback did not confirm it: achieved {2}x{3}x{4}bpp@{5}Hz, target {6}x{7}x{8}bpp@{9}Hz", + isRevert ? "Restoring" : "Changing", deviceName, + achievedMode.dmPelsWidth, achievedMode.dmPelsHeight, achievedMode.dmBitsPerPel, achievedMode.dmDisplayFrequency, + target.DmPelsWidth, target.DmPelsHeight, target.DmBitsPerPel, target.DmDisplayFrequency) + : string.Format( + "{0} the resolution for screen {1} reported success but the mode could not be read back afterward to confirm it", + isRevert ? "Restoring" : "Changing", deviceName); + + RecordFailureAccounting(deviceName, target, isRevert, bound, "readback-mismatch", message, DispChange.DispChangeFailed); + return ResolutionChangeResult.AppliedUnverified; + } + + // Drops every recorded failure/log/notification entry for deviceName - across every + // target and direction, not just the one that just succeeded. Once a device is confirmed + // working, its entire prior failure history stops being useful information; keeping it + // around would only make a later, unrelated failure on the same device look like a + // continuation of an old streak it has nothing to do with. + private static void ClearFailureState(string deviceName) + { + string prefix = deviceName + KeySeparator; + + List staleFailureKeys = new List(); + foreach (string key in _consecutiveFailures.Keys) + { + if (key.StartsWith(prefix, StringComparison.Ordinal)) { - MessageBox.Show("Changing the resolution failed: " + Enum.GetName(typeof(DispChange), returnValue)); + staleFailureKeys.Add(key); } } - return false; + foreach (string key in staleFailureKeys) + { + _consecutiveFailures.Remove(key); + } + + _loggedFailures.RemoveWhere(delegate(string key) { return key.StartsWith(prefix, StringComparison.Ordinal); }); + _notifiedFailures.RemoveWhere(delegate(string key) { return key.StartsWith(prefix, StringComparison.Ordinal); }); + } + + // A control character (U+0001) - cannot appear in a device name or in the numeric/boolean + // descriptions built below, so prefix matching in ClearFailureState can never straddle two + // different device names (e.g. "DISPLAY1" is never a prefix-match for a key that actually + // belongs to "DISPLAY10"). + private const string KeySeparator = "\u0001"; + + private static string DeviceKey(string deviceName, string suffix) + { + return deviceName + KeySeparator + suffix; + } + + private static string BuildFailureKey(string deviceName, ResolutionModeWrapper target, bool isRevert) + { + return DeviceKey(deviceName, DescribeTarget(target) + KeySeparator + (isRevert ? "revert" : "apply")); + } + + private static string DescribeTarget(ResolutionModeWrapper target) + { + return string.Format("{0}x{1}x{2}@{3}#{4}", + target.DmPelsWidth, target.DmPelsHeight, target.DmBitsPerPel, + target.DmDisplayFrequency, target.DmDisplayFixedOutput); + } + + // The production IDisplayModeDevice: EnumDisplaySettings/ChangeDisplaySettingsEx against a + // real display, exactly as the old ChangeResolutionEx called them directly before this + // seam existed. + private class RealDisplayModeDevice : IDisplayModeDevice + { + public bool TryGetCurrentMode(string deviceName, out Devmode mode) + { + return TryEnumerateMode(deviceName, EnumCurrentSettings, out mode); + } + + public bool TryEnumerateMode(string deviceName, int modeNum, out Devmode mode) + { + mode = new Devmode(); + mode.dmSize = (ushort)Marshal.SizeOf(mode); + return EnumDisplaySettings(deviceName, modeNum, ref mode); + } + + public DispChange ChangeMode(string deviceName, Devmode mode, ChangeDisplaySettingsFlags flags) + { + // The interface takes Devmode by value (see IDisplayModeDevice) - copied to a local + // here purely because the P/Invoke itself needs "ref". + Devmode localMode = mode; + return ChangeDisplaySettingsEx(deviceName, ref localMode, IntPtr.Zero, flags, IntPtr.Zero); + } } } @@ -275,4 +667,20 @@ public enum ChangeDisplaySettingsFlags : uint CdsNoreset = 0x10000000 } + // The DEVMODE dmFields bits ChangeResolutionEx cares about - a small, named subset of the + // full DM_* constant set (winuser.h), added alongside ChangeDisplaySettingsFlags rather than + // reusing raw hex so ApplyTargetFields' mask reads as what it is. Values are the real Win32 + // DM_* constants, unchanged. + [Flags] + public enum DevmodeFields : uint + { + DmPosition = 0x20, + DmDisplayOrientation = 0x80, + DmBitsPerPel = 0x40000, + DmPelsWidth = 0x80000, + DmPelsHeight = 0x100000, + DmDisplayFlags = 0x200000, + DmDisplayFrequency = 0x400000, + DmDisplayFixedOutput = 0x20000000 + } } diff --git a/vibrance.GUI/common/ResolutionModeWrapper.cs b/vibrance.GUI/common/ResolutionModeWrapper.cs index b9fa2e1..7004a97 100644 --- a/vibrance.GUI/common/ResolutionModeWrapper.cs +++ b/vibrance.GUI/common/ResolutionModeWrapper.cs @@ -54,5 +54,28 @@ public override bool Equals(object obj) } return false; } + + // Used by ResolutionHelper.ChangeResolutionEx/IsResolutionChangeNeeded to decide whether a + // mode change is still needed and whether one that was just attempted actually landed - + // deliberately comparing only the four fields a change actually declares and can verify + // (DmPelsWidth, DmPelsHeight, DmBitsPerPel, DmDisplayFrequency), NOT DmDisplayFixedOutput, + // unlike Equals above. DmDisplayFixedOutput (the "(Center)"/"(Stretch)" scaling choice) is + // only honoured by ChangeDisplaySettingsEx when DM_DISPLAYFIXEDOUTPUT survives into the + // achieved mode's own dmFields, which is driver-dependent - some drivers apply the four + // real fields correctly but silently pin this one to their own default regardless of what + // was requested. Basing the "does this still need changing?" guard on a field a driver is + // free to never honour is what let a user's "(Center)" mode selection re-fire a real mode + // set and registry write on every single foreground event, forever, even though the mode + // had genuinely already been achieved on every field the driver actually supports. + // Equals/ToString above are intentionally untouched by this - the combo box in + // VibranceSettings and the applicationData.xml round trip both depend on all five fields + // matching exactly. + public bool MatchesAchievedMode(Devmode mode) + { + return this.DmPelsWidth == mode.dmPelsWidth && + this.DmPelsHeight == mode.dmPelsHeight && + this.DmBitsPerPel == mode.dmBitsPerPel && + this.DmDisplayFrequency == mode.dmDisplayFrequency; + } } } diff --git a/vibrance.GUI/common/VibranceGUI.cs b/vibrance.GUI/common/VibranceGUI.cs index fe5804c..3ef8b75 100644 --- a/vibrance.GUI/common/VibranceGUI.cs +++ b/vibrance.GUI/common/VibranceGUI.cs @@ -8,6 +8,7 @@ using System.Runtime.InteropServices; using System.Threading; using System.Windows.Forms; +using Microsoft.Win32; using Application = System.Windows.Forms.Application; using MessageBox = System.Windows.Forms.MessageBox; @@ -52,24 +53,34 @@ public VibranceGUI( trackBarWindowsLevel.Maximum = maxTrackBarValue; _windowsResolutionSettings = new Dictionary>>(); - foreach(Screen screen in Screen.AllScreens) - { - Devmode currentResolutionMode; - if (ResolutionHelper.GetCurrentResolutionSettings(out currentResolutionMode, screen.DeviceName)) - { - List availableResolutions = ResolutionHelper.EnumerateSupportedResolutionModes(screen.DeviceName); - if(screen.Primary) - { - _supportedResolutionList = availableResolutions; - } - var tuple = new Tuple>(new ResolutionModeWrapper(currentResolutionMode), availableResolutions); - _windowsResolutionSettings.Add(screen.DeviceName, tuple); - } - else - { - MessageBox.Show("Current resolution mode could not be determined. Switching back to your Windows resolution will not work."); - } + RebuildWindowsResolutionSettings(true); + + // _supportedResolutionList is readonly, so it can only be assigned from inside a + // constructor body - not from a method the constructor merely calls, even a private + // one - which is why this is pulled back out of RebuildWindowsResolutionSettings + // (shared with the refresh path below) instead of living inside it. Equivalent to the + // old "if (screen.Primary) { _supportedResolutionList = availableResolutions; }": if + // the primary screen's own read failed, it never made it into the dictionary either, + // and this is left null exactly as it was before. + Screen primaryScreen = Screen.PrimaryScreen; + Tuple> primaryEntry; + if (primaryScreen != null && _windowsResolutionSettings.TryGetValue(primaryScreen.DeviceName, out primaryEntry)) + { + _supportedResolutionList = primaryEntry.Item2; } + + // Subscribed here - after _windowsResolutionSettings exists, before getProxy hands it + // to the vendor proxy - and unsubscribed in CleanUp(). ResolutionChangeFailed lets the + // resolution-change fix (see ResolutionHelper.ChangeResolutionEx) report a give-up + // without ever showing a MessageBox from inside the WinEvent callback thread. + // SystemEvents.DisplaySettingsChanged keeps _windowsResolutionSettings from going stale + // when the user changes their desktop resolution directly in Windows - unsubscribing it + // is mandatory (not just good practice): SystemEvents holds a strong reference to this + // handler on its own dedicated thread, and leaving it subscribed leaks this form and can + // fault at shutdown. + ResolutionHelper.ResolutionChangeFailed += OnResolutionChangeFailed; + SystemEvents.DisplaySettingsChanged += OnDisplaySettingsChanged; + _applicationSettings = new List(); _v = getProxy(_applicationSettings, _windowsResolutionSettings); @@ -329,6 +340,159 @@ private void CleanUp() { Log(ex); } + finally + { + // In a finally, not just after the try: these must run even if the block above + // throws. SystemEvents.DisplaySettingsChanged above all - see the ctor's own + // comment for why leaving it subscribed leaks this form and can fault at shutdown. + SystemEvents.DisplaySettingsChanged -= OnDisplaySettingsChanged; + ResolutionHelper.ResolutionChangeFailed -= OnResolutionChangeFailed; + } + } + + /// + /// (Re)populates _windowsResolutionSettings from the currently attached screens, mutating + /// the existing Dictionary instance in place (Clear() then re-add) rather than replacing + /// it - both proxies hold a reference to this very instance (NVIDIA's is static), so only + /// in-place mutation is visible to them. Shared by the constructor (showFailureDialog: + /// true) and OnDisplaySettingsChanged below (showFailureDialog: false) - see that method + /// for why the dialog must never fire from the refresh path. + /// + private void RebuildWindowsResolutionSettings(bool showFailureDialog) + { + // This is the single most dangerous line in the resolution-change fix: if a refresh + // runs while a game's resolution change is currently applied, a live read of "the + // current mode" for the game's own screen returns the GAME's mode, not the desktop's. + // Overwriting the captured "Windows resolution" (Item1) with that would strand the + // desktop at the game's resolution forever - the revert path compares against Item1, so + // once it has silently become the game's own mode, "reverting" turns into a no-op that + // still reports success. A game going fullscreen is exactly the kind of change that + // fires DisplaySettingsChanged, so this is not a rare interleaving to guard against. + // + // While a resolution change is applied, every screen this dictionary already has an + // entry for keeps its previously captured Item1 untouched, and only Item2 (the + // device's supported-mode list, a property of the device rather than of whichever mode + // happens to be active right now) is refreshed. A screen with no previous entry still + // needs one captured fresh - it cannot be the screen the game is running on, since that + // one is already recorded. + bool preserveCapturedMode = _v != null && _v.GetVibranceInfo().isResolutionChangeApplied; + + Dictionary>> previous = + new Dictionary>>(_windowsResolutionSettings); + + _windowsResolutionSettings.Clear(); + foreach (Screen screen in Screen.AllScreens) + { + Tuple> existing; + bool hasExisting = previous.TryGetValue(screen.DeviceName, out existing); + + // Item2 is a property of the device's capability, not of whichever mode happens to + // be active right now (the comment above already relies on that to justify reusing + // it while a game's own change is applied) - so a device this dictionary already + // has an entry for reuses that SAME List instance rather than + // re-enumerating. Two reasons this matters beyond the obvious P/Invoke cost (up to + // several hundred EnumDisplaySettings calls per screen, on the UI thread): first, + // vibranceGUI's OWN resolution changes also fire DisplaySettingsChanged, so an + // unconditional re-enumerate here would run twice per alt-tab cycle; second, reusing + // the identical instance (not a fresh copy) is what keeps _supportedResolutionList - + // captured once, in the constructor, and readonly - from silently going stale after + // a refresh, since it then still points at the very list being kept up to date here. + List availableResolutions = hasExisting + ? existing.Item2 + : ResolutionHelper.EnumerateSupportedResolutionModes(screen.DeviceName); + + if (preserveCapturedMode && hasExisting) + { + _windowsResolutionSettings.Add(screen.DeviceName, + new Tuple>(existing.Item1, availableResolutions)); + continue; + } + + Devmode currentResolutionMode; + if (ResolutionHelper.GetCurrentResolutionSettings(out currentResolutionMode, screen.DeviceName)) + { + _windowsResolutionSettings.Add(screen.DeviceName, + new Tuple>(new ResolutionModeWrapper(currentResolutionMode), availableResolutions)); + } + else if (showFailureDialog) + { + MessageBox.Show("Current resolution mode could not be determined. Switching back to your Windows resolution will not work."); + } + } + } + + /// + /// Keeps _windowsResolutionSettings current when the user changes their desktop resolution + /// (or a monitor is hot-plugged) outside of vibranceGUI itself - without this, the revert + /// path drags the desktop back to whatever mode was active at startup, with every API call + /// still reporting success. + /// + private void OnDisplaySettingsChanged(object sender, EventArgs e) + { + // Raised on SystemEvents' own dedicated thread, not necessarily the UI thread that owns + // this form and that OnWinEventHook's callbacks arrive through - marshal onto it before + // touching _windowsResolutionSettings, which that hook handler reads with no locking of + // its own. InvokeRequired is NOT sufficient on its own: it returns false whenever the + // form has no window handle yet (Control.InvokeRequired falls through to + // FindMarshalingControl(), which returns false if !IsHandleCreated) - and the handle + // genuinely does not exist for the whole span of the constructor's NvAPI/ADL + // initialisation after this handler is subscribed (backgroundWorker_DoWork busy-waits + // on !IsHandleCreated), which is exactly when a SystemEvents notification is likely at + // autostart, as monitors settle. Without the explicit IsHandleCreated check this method + // would run its dictionary mutation directly on the SystemEvents thread in that window. + // IsDisposed also guards the symmetric case at shutdown: CleanUp()'s unsubscribe cannot + // cover a notification already in flight, which could otherwise find the handle + // destroyed (same wrong-thread mutation) or the form disposed (BeginInvoke throwing + // ObjectDisposedException on the SystemEvents thread, with nothing there to catch it). + if (this.IsDisposed || !this.IsHandleCreated) + { + return; + } + if (this.InvokeRequired) + { + this.BeginInvoke((MethodInvoker)delegate { OnDisplaySettingsChanged(sender, e); }); + return; + } + + // showFailureDialog: false - a MessageBox popping up on every hot-plug or resolution + // change, potentially over a fullscreen game, is exactly the modal-on-the-callback- + // thread mistake this whole fix removes. The constructor's own one-time build above + // still shows it once, at startup, where the user is looking at the window and can act + // on it immediately. + RebuildWindowsResolutionSettings(false); + } + + /// + /// Reports a resolution change ChangeResolutionEx has given up on, via a balloon tip instead + /// of the modal MessageBox the pre-fix code raised from inside the WinEvent callback thread + /// (see ResolutionHelper.cs). ResolutionHelper only ever raises this once it has given up - + /// never on a single transient failure - so there is no "still retrying" wording here; see + /// ResolutionHelper.RecordFailure. + /// + private void OnResolutionChangeFailed(object sender, ResolutionFailureEventArgs e) + { + // Same reasoning as OnDisplaySettingsChanged above - this is also raised from + // ResolutionHelper's own call stack, which for the WinEvent-driven cases below runs on + // the UI thread already, but ResolutionHelper offers no guarantee of that in general. + if (this.IsDisposed || !this.IsHandleCreated) + { + return; + } + if (this.InvokeRequired) + { + this.BeginInvoke((MethodInvoker)delegate { OnResolutionChangeFailed(sender, e); }); + return; + } + + this.notifyIcon.BalloonTipIcon = ToolTipIcon.Warning; + // The desktop is now stuck at the game's resolution (revert) or the game never got its + // requested resolution (apply) with nothing else in the program that will ever retry + // it, so this has to name the device and, for a revert, point the user at where they + // can fix it themselves. + this.notifyIcon.BalloonTipText = e.IsRevert + ? string.Format("vibranceGUI could not switch display {0} back to your Windows resolution and has stopped trying. Check Windows Display settings.", e.DeviceName) + : string.Format("vibranceGUI could not change display {0} to this game's resolution and has stopped trying.", e.DeviceName); + this.notifyIcon.ShowBalloonTip(250); } public static void Log(Exception ex) diff --git a/vibrance.GUI/vibrance.GUI.csproj b/vibrance.GUI/vibrance.GUI.csproj index 71e63a8..40033a7 100644 --- a/vibrance.GUI/vibrance.GUI.csproj +++ b/vibrance.GUI/vibrance.GUI.csproj @@ -121,6 +121,7 @@ ProcessExplorer.cs +