Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

win-capture: Improve fullscreen window detection #10880

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kir-Antipov
Copy link

Description

This small change allows windows that extend slightly beyond the defined viewport to be properly detected as fullscreen ones.

While, in general, one should not assume that bottom > top and right > left, win-capture is tailored to Windows, where this assumption always holds true. Therefore, simply swapping == with >= when comparing rect.right with mi.rcMonitor.right and rect.bottom with mi.rcMonitor.bottom should suffice.

Motivation and Context

While it may sound crazy, some apps on Windows use a dirty hack of extending their windows slightly beyond the monitor's declared resolution, usually by 1px, to preserve "true" windowed fullscreen and circumvent some issues related to the exclusive fullscreen mode presented by the graphics library being used, Windows itself, or both. As an example, see: glfw/glfw#527 (comment).

At the moment, these windows are not recognized by OBS Game Capture because they fail the strict comparison rules defined here:

if (rect.left == mi.rcMonitor.left &&
rect.right == mi.rcMonitor.right &&
rect.bottom == mi.rcMonitor.bottom &&
rect.top == mi.rcMonitor.top) {
setup_window(gc, window);
} else {
gc->wait_for_target_startup = true;
}

And this is quite annoying because, from the user's perspective, these windows are clearly in the fullscreen mode.

Additionally, Windows itself doesn't resort to such strict comparison rules, as evidenced by the fact that the taskbar disappears, as it always does for fullscreen windows, even when a window stretches slightly beyond the defined viewport. So, if anything, this should be considered a parity feature.

How Has This Been Tested?

There isn't much to test, so I just ran the freshly built OBS on Windows in a VM to check if it works. Thankfully, a two-symbol change didn't break the project :D

The change is very localized, shouldn't affect other areas of the code, and doesn't introduce any breaking changes. It simply fixes a minor issue/tweaks the pre-existing functionality to better match the, ahem, nuances of Windows development.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Tweak (non-breaking change to improve existing functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation. (I.e., none)

It's not that uncommon for apps on Windows to use a hacky method of
extending their windows ~1px past the supported monitor resolution
to preserve "true" windowed fullscreen and circumvent some exclusive
fullscreen-related issues presented by their graphics library,
Windows itself, or both.

Therefore, the fullscreen detection technique has been relaxed
to properly identify windows where `(x, y) == (viewportX, viewportY)`
and `(width, height) >= (viewportWidth, viewportHeight)` as fullscreen.

Windows also does something similar, evident by the fact that
the taskbar disappears, as it always does for fullscreen windows,
even when a window stretches slightly beyond the defined viewport.
@WizardCM WizardCM added Bug Fix Non-breaking change which fixes an issue Enhancement Improvement to existing functionality labels Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue Enhancement Improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants