Skip display mode setting if already set #1489
Conversation
|
I'm fine with the functionality change (even though it's a bit of a non-issue when using fshack), but this introduces a non-trivial amount of code-duplication. @Joshua-Ashton what exactly was the reason again for having all the DXGI monitor code duplicated in D3D9, save struct conversion? Also, there's an unrelated commit that's already being tracked in #1436 and code formatting doesn't quite match ours, but we can fix those issues before merging. |
| (currentMode.dmDisplayFlags == devMode.dmDisplayFlags || | ||
| !(devMode.dmFields & DM_DISPLAYFLAGS))) | ||
| { | ||
| Logger::info(str::format("DXGI: Skipping redundant mode setting call: ", |
There was a problem hiding this comment.
I highly doubt we need a log message to inform the user that nothing happened.
There was a problem hiding this comment.
Agree, but if this will have some problems, log message will help with debugging. If it's too much, I will remove
src/dxgi/dxgi_monitor.cpp
Outdated
| (currentMode.dmDisplayFrequency == devMode.dmDisplayFrequency || | ||
| !(devMode.dmFields & DM_DISPLAYFREQUENCY)) && | ||
| (currentMode.dmDisplayFlags == devMode.dmDisplayFlags || | ||
| !(devMode.dmFields & DM_DISPLAYFLAGS))) |
There was a problem hiding this comment.
devMode.dmFields & DM_DISPLAYFLAGS is always false, so the entire flags check should probably be removed.
|
The code duplication really only exists because I wasn't exactly sure what I needed when I implemented, and at that time the D3D9 frontend was still a fork so I couldn't just move stuff around willy nilly there. 😛 Ideally this is the sort of thing that the wsi abstraction in the native port solves... Might be worth re-looking into rebasing that and stuff before merging this? |
|
Well I guess factoring out the monitor stuff into common code is an option. I kind of don't want to deal with the native stuff right now. |
|
How about merge this, and then thinking about code unify? :) Code duplication is already present and will not get worse after merge |
We all know that that's not going to happen and instead we'll end up with bit rot. I've been busy working on vkd3d lately so not much time to work on this. |
See #1489. Co-authored-by: zvova7890 <zvova7890@gmail.com>
See #1489. Co-authored-by: zvova7890 <zvova7890@gmail.com>
|
Superseded by #1494. This reworks the monitor stuff to remove code duplication, and includes the feature discussed in this PR based on your work (I added you as co-author to give credit, not sure if there's any better way to do this). |
|
Thank you, i had test you branch, looks fine |
See #1489. Co-authored-by: zvova7890 <zvova7890@gmail.com>
That mostly taken from wine's code. Before setting new display mode, we are checking if our current display mode isn't equal new required mode, if new mode is same as current, do not call ChangeDisplaySettingsExW. Excludes display flickering if the application several times setup same display mode