obs-studio icon indicating copy to clipboard operation
obs-studio copied to clipboard

libobs/utils: Fix window selection behavior for window capture

Open novumiter opened this issue 7 months ago • 2 comments

Description

When a process spawns multiple windows with the same title (but different hwnd obviously), the window selection logic (window_rating) function will not be aware of that, and will try to pick the one that is focused or was last focused, creating undesired behavior, for example duplicating the existing window that was last selected.

Basically this -> #12088

Motivation and Context

It solves the problem, when, for example, you use OBS-WEBSOCKETS to remotely create sources or manage them, when you know beforehand the process name, window title and class name, but there are multiple windows that match that criteria 1:1 only the one that is currently/last focused will be picked EVERY TIME - this fix will make sure that if there is already a source existing with that HWND, it will set it's rating to INT_MAX so it skips over to the next matching window.

How Has This Been Tested?

- 02:20:25.892: CPU Name: AMD Ryzen 7 5800X 8-Core Processor             
- 02:20:25.892: CPU Speed: 3800MHz
- 02:20:25.892: Physical Cores: 8, Logical Cores: 16
- 02:20:25.892: Physical Memory: 32692MB Total, 8954MB Free
- 02:20:25.893: Windows Version: 10.0 Build 26100 (release: 24H2; revision: 4061; 64-bit)
- 02:20:25.893: Running as administrator: false
- 02:20:25.893: Windows 10/11 Gaming Features:
- 02:20:25.893: 	Game Bar: Off
- 02:20:25.893: 	Game DVR: Off
- 02:20:25.893: 	Game DVR Background Recording: Off
- 02:20:25.893: 	Game Mode: Probably On (no reg key set)
- 02:20:25.895: Sec. Software Status:
- 02:20:25.896: 	Malwarebytes: disabled (AV)
- 02:20:25.896: 	Microsoft Defender Antivirus: enabled (AV)
- 02:20:25.896: 	Windows Firewall: enabled (FW)
- 02:20:25.898: Current Date/Time: 2025-05-17, 02:20:25
- 02:20:25.898: Browser Hardware Acceleration: true
- 02:20:25.898: Hide OBS windows from screen capture: false
- 02:20:25.898: Qt Version: 6.8.2 (runtime), 6.8.2 (compiled)
- 02:20:25.898: Portable mode: false
- 02:20:26.236: OBS 31.0.0-283-gcaa3d3eb5 (64-bit, windows)

I tried creating window capture for 2 different calculator.exe apps on WINDOWS 11 (perfect example), and it worked first try, here's a demonstration attached:

https://github.com/user-attachments/assets/bfd2a545-4128-44bd-ab8f-1f69518bce7d

for comparison, here is the original behaviour:

https://github.com/user-attachments/assets/690140b3-5564-47a5-8e35-c1e5a27d250e

I tried copy-pasting the sources, and they worked flawlessly in case duplicates are needed.

I tried it with more than 5 instances, and it worked perfectly fine. I haven't seen any breaking changes, but honestly speaking I'm not a hardcore user so maybe I'm not aware of what I could have broke in the process, so please help me out as I'm not extremely familiar with OBS codebase so please guide me to the proper fix if this is inappropriate and I'll try my best.

Types of changes

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

Checklist:

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

novumiter avatar May 17 '25 01:05 novumiter

@RytoEX sorry for tagging, is there anything I need to change for the PR, or is there something inherently wrong with the way I introduced the fix? I would love for someone to take a look/review the PR if possible.

novumiter avatar May 27 '25 07:05 novumiter

I'm not sure how I feel about this fix. I don't have adequate alternate ideas to propose but I'm not sure if this is the solution. Does anyone else have any ideas about how to deal with this situation? (Might be better to discuss proposed solutions in the issue too)

Lain-B avatar Jun 07 '25 23:06 Lain-B

The issue this is trying to fix is good but if Lain isn't happy with the approach here, we should discuss a better solution

Warchamp7 avatar Jun 16 '25 20:06 Warchamp7

Yes I agree, this is why I specifically asked if there's a better way to do this, as I also felt this is not the proper way to fix it - perhaps someone with better knowledge of the codebase could chime in.

novumiter avatar Jun 17 '25 15:06 novumiter