obsproject
linux-capture/xcomposite-input.c: Add a backup texture to store previous contents of a window
Description
- Add extra texture to store previous contents of X window, so that when it is unmapped by window manager, e.g. when
switching a workspace, preview does not got black, but shows some amount of preview. - Add more descriptive fail message for xcb_composite_name_window_pixmap when the window is unmapped.
Motivation and Context
When X window is unmapped xcb_composite_name_window_pixmap fails and does not retrieve the pixmap, which makes preview screen and recording screen go black, when for example switching workspace, which is at least aesthetically unpleasing. Also when the said function fails a rather cryptic message is printed that the function just failed does not note the fact that retrieval failed because the window is unmapped, which is kind of confusing.
This PR kind of fixes this issue: https://github.com/obsproject/obs-studio/issues/4160
How Has This Been Tested?
I opened up some application that had changing contents. The preview screen was updating as well. Then I moved the changing window to another workspace as expected the image froze, as the window is unmapped, but previous contents stayed. Then I moved the window back up and preview screen was updating with the window. Then I repeated the same procedure with recording, which yielded the same result. Also logs showed that xcb_composite_name_window_pixmap was failing due to the window being unmapped and stopped printing this error as soon as the window became viewable again.
testing environment: OS: ArchLinux Kernel: 6.12.20-lts WM: AwesomeWM GPU: Nvidia GeForce GTX 1050Ti
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.
As a note, this is undesirable in many situations as a "stuck frame" could be misinterpreted as capture still functioning, where a user doesn't realize it's no longer updating. At worst, if something is accidentally captured that wasn't intended, and then the window is closed, could lead to sensitive information being shown.
Those specific examples being non-exhaustive of the reasons you might not want this, it overall just feels like explicitly introducing a bug we've fixed in previous iterations of capture sources that hold the last frame, and every time it's happened it's been undesirable behavior as reported by our users.
I'll let others chime in, but I'm against this is a pattern we should allow in OBS.
My 2c as noted on Discord: I (and probably others) rely on the capture showing nothing when the window vanishes.
At least as I understand this PR, OBS has no information on whether a capture disappeared due to changing workspaces or if it was due to the window being closed. That in turn would mean that this change would also apply to windows being closed manually.
However, to make it easy to switch between games or applictions, users (like me) might have set up various window capture sources so that OBS can pick up the window as the game or application is started. This makes it easy to switch games without the need to adjust source settings or showing/hiding sources because the closed game would just disappear and the new one would just show up. With this change this wouldn't be possible anymore because the closed game's capture would be stuck on the last frame and therefore overlay the new game's capture, requiring to hide all capture sources except the new game's manually.
This, and also the fact that the current behaviour is consistent with other platforms, makes this change sub-optimal IMHO.
My 2c as noted on Discord: I (and probably others) rely on the capture showing nothing when the window vanishes.
At least as I understand this PR, OBS has no information on whether a capture disappeared due to changing workspaces or if it was due to the window being closed. That in turn would mean that this change would also apply to windows being closed manually.
However, to make it easy to switch between games or applictions, users (like me) might have set up various window capture sources so that OBS can pick up the window as the game or application is started. This makes it easy to switch games without the need to adjust source settings or showing/hiding sources because the closed game would just disappear and the new one would just show up. With this change this wouldn't be possible anymore because the closed game's capture would be stuck on the last frame and therefore overlay the new game's capture, requiring to hide all capture sources except the new game's manually.
This, and also the fact that the current behaviour is consistent with other platforms, makes this change sub-optimal IMHO.
This change is more for folks that use tiling wms that unmap windows when the window is minimized. When you switch workspace for a split second, a black screen (or whatever is underneath the capture source) is shown, that causes flickering and is kinda ugly (not to mention that I can't even preview what I am currently displaying if the window is in another workspace). For people that use macOS, Windows, or something like KDE this might not seem like much, because they usually don't rely heavily on workspaces, but with tiling window managers you must use them. The solution that was provided in the issue is more of a hack, rather then a fix (kinda like mine lol), and requires altering default behavior of window managers (and compositors with it). So personally I'd rather have a checkbox or something that allows me to turn on/off freezing, than hack window managers, compositors and everything with it. And personally having the window freeze makes much more sense than displaying nothing, because the window is still there, it has content, it's just now viewable. If I was recording the screen then it would make sense to display nothing, but I am recording a window, wherever it might be.
I understand now that people rely on this behavior and don't want to force my workflow onto others. You don't have to add this if this is bad. But I just want to note that current workflow is not ideal for the likes of me, and I personally don't want others' workflow be forced upon me. I think if this change was ever to be merged there should've been at least a way to opt out/in. I will add a bit more customizability when I have time.
UPD: I think this video will show what I mean by flickering. And this would happen a lot if i didn't have my patch.
https://github.com/user-attachments/assets/e56f9daf-7217-48e3-8a5b-524c501bdb2e