linux-capture: Prevent X11 display/window captures from capturing implicitly #11232
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Half a second was a bit too often and was spamming the debug logging when a window wasn't found.
Lain-B
force-pushed
the
seal-an-xcomp-curse
branch
from
202915d to
87c44a5
Compare
kkartaltepe
reviewed
Sep 2, 2024
Moves the window handle/name/class decoding code out of the xcb_find_window() function and into its own dedicated function so it can be used elsewhere. This s*** is cursed.
Lain-B
force-pushed
the
seal-an-xcomp-curse
branch
from
87c44a5 to
6f600ca
Compare
Lain-B
changed the title
|
I also added a fix for xshm Display Capture as well. |
WizardCM
added
Enhancement
I'm not happy.
This is cursed. Window ID storage for xcomposite capture is absolutely cursed. It should not be storing the window handle with this. I'm pretty sure that whoever wrote it at the time decided to store the god-forsaken window handle (which does not persist after the window closes) as part of the ID because they were afraid it might capture the wrong window if they close OBS and open it up again while the window still exists. Again, xcomposite capture is absolutely cursed.
There are two situations where xcomposite window capture will capture random windows: on first creation, and when going to the properties when the current window is invalid. The first happens because for whatever reason someone decided to just make it capture the first top-level window if there is no set value. The second happens because the properties widget cannot find the value it's looking for and defaults to the first one when the properties are opened, thus selecting and capturing the first window in the list (which is probably something we should fix in the properties view at some point but I don't want to dive into code that's even *more* cursed than xcomposite code right now) I think that this was a major oversight and that whoever wrote it however many countless years ago did not realize that this is something that maybe users don't want to have happen. So instead, this diff makes it so that on first creation, it creates a value that says "[Select a window to capture]" that keeps the capture inactive until a user actually chooses a window rather than the top-level window. It also makes it so that if the user has a window that is no longer valid, it will keep that window in the list and as the currently selected value, which prevents it from automatically selecting the first window in the list when properties are opened. (Have I mentioned xcomposite is cursed? Trying to debug xcomposite code in a debugger freezes my window compositor and forces me to do a hard restart of my entire computer, so I was forced to use printf debugging. Absolute nightmare-inducing code in here.)
Like xcomposite, this was programmed to select the first display by default. Change it to not capture any display unless explicitly selected by the user.
Lain-B
force-pushed
the
seal-an-xcomp-curse
branch
from
6f600ca to
d242a41
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
There are two situations where xcomposite window capture will capture random windows: on first creation, and when going to the properties when the current window is invalid. The first happens because it apparently captures the first top-level window if there is no set value. The second happens because when opening the properties, the properties widget cannot find the value it's looking for and defaults to selecting the first one when the properties are opened, thus selecting and capturing the first window in the list (which is probably something we should fix in the properties view at some point but I don't want to dive into code that's even more cursed than xcomposite code right now).
So instead, this diff makes it so that on first creation, it creates a value that says "[Select a window to capture]" that keeps the capture inactive until a user actually chooses a window rather than the top-level window. It also makes it so that if the user has a window that is no longer valid, it will keep that window in the list and as the currently selected value, which prevents it from automatically selecting the first window in the list when properties are opened.
The xshm Display Capture has also been fixed similarly to not automatically capture a display on first creation.
Motivation and Context
A window/display capture should not capture something without the user explicitly choosing it.
How Has This Been Tested?
Tested it locally, might be good if others test it to ensure it works as intended and doesn't have any issues. 99.9% sure that it works fine though.
Types of changes
Checklist: