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

[WIP] libobs-opengl: Support Wayland-EGL #2097

Closed
wants to merge 2 commits into from
Closed

[WIP] libobs-opengl: Support Wayland-EGL #2097

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 6, 2019

Description

Enables support for EGL on wayland as a libobs-opengl backend. It supports swapping between the X11/Wayland backends on Unix-like platforms. This is for the rendering backend only, it doesn't cover the capture plugins.

Motivation and Context

Qt already supports an environment variable to change backends at program startup, by setting QT_QPA_PLATFORM=xcb or QT_QPA_PLATFORM=wayland-egl. Without this PR, OBS ignores this Qt setting and instead assumes that on Unix-like platforms, X11 is always used. This causes creation of the glx context to fail and the OpenGL preview to show up blank when the Qtwayland backend is used. The relevant bug is 0001518, which this PR aims to fix. This also clears up half of 0000609.

How Has This Been Tested?

I test on Ubuntu Disco with an Intel i915. The wayland compositors I've tested with are Weston, Sway, Mutter (GNOME), and KWin (KDE). It works in Weston and Sway, but there are issues in the others:

  • Mutter displays a blank preview window Testing with Mutter/gnome-shell 3.34, it seems to work for me now.
  • KWin causes OBS to deadlock when resizing the window

Note that currently this PR is not ready to merge because of those issues. It's not clear to me yet if those are bugs in OBS, in Qt, or in those compositors. Since it works fine in Weston, it leads me to believe that those are compositor issues. If you're using Sway or another wlroots compositor then you can use this PR as-is before it's merged.

I've also tested this on my machine X11 in GNOME, KDE, and with i3 to make sure nothing broke there. I have not tested on any BSDs. Some of the general backend code was touched, so this should also be tested on the non-Unix platforms to make sure nothing broke. I have not tested on Windows or MacOS.

Types of changes

New feature: Support wayland as a rendering backend.

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.

@kkartaltepe kkartaltepe added Bug Fix Non-breaking change which fixes an issue Enhancement Improvement to existing functionality Work In Progress labels Oct 6, 2019
@kkartaltepe
Copy link
Collaborator

kkartaltepe commented Oct 6, 2019

Personally, a different name should be chosen for libobs/obs-nix, and libobs/obs-x11 should be git cped to preserve history and so the diff is readable. It may be possible to start a new history for libobs/obs-nix if you git mv to obs-x11 and then create libobs/obs-nix which if it works would be good too.

--edit

Also for the sake of reviewers it may be best to split this into two commits. 1 libobs modification to create a runtime platform layer only supporting existing platforms. And the 2nd commit adding in the wayland platform layer.

@ghost
Copy link
Author

ghost commented Oct 11, 2019

@kkartaltepe: I think those are good suggestions. I will try to update this soon.

@grimmy
Copy link
Contributor

grimmy commented Oct 25, 2019

I pulled the branch, rebased upstream/master (needed for the caption fix) and seems to be working alright for me. Debian unstable.

That said, I'm still seeing quite a few of these in the console log.

[1025/184204.066813:ERROR:gl_surface_presentation_helper.cc(237)] GetVSyncParametersIfAvailable() failed!

If you need/want any help, I'd love to see this merged.

@grimmy
Copy link
Contributor

grimmy commented Oct 25, 2019

Hah, i Just noticed there's some placement issues though... Notice that the preview is overlapping the stats panel... Happens outside of studio mode too

image

@w23
Copy link

w23 commented Nov 6, 2019

There's also EGL context creation in this PR: #1758
Although it is targeting X11 mostly, it does accidentally work under wayland (although very likely still not really natively) and allows for screen grabbing using KMS.
We should probably coordinate our efforts on getting EGL into OBS.

@kkartaltepe
Copy link
Collaborator

I dont really see much of a reason to try and coordinate between the two PRs (this adds wayland windowing support, egl is just incidental to that cause, and such a context couldnt be shared with X11 anyway). What did you have in mind for coordinating?

@WizardCM WizardCM added the Linux Affects Linux label Nov 7, 2019
@aycyang
Copy link

aycyang commented Nov 16, 2019

I like that you can select GLX/EGL at run-time with this PR, rather than it being a compile-time option. I'm a little intimidated by the #ifdefs and if-statements, as they introduce a lot of new states to consider. I wonder if there is a way to compile the GLX and EGL backends to .so files, and then dynamically link the one you want at run-time.

@WizardCM WizardCM added the Seeking Testers Build artifacts on CI label Nov 26, 2019
@NilsIrl
Copy link

NilsIrl commented Dec 30, 2019

I don't know if I'm doing something wrong but for me nothing changes. I'm on NixOS which might be the reason it's not working Somehow works now. I'm gonna test it on stream. This is great. OBS was the last thing that required xwayland.

Also BTW, for those who don't know, there is wlrobs that is a plugin/extension that allows to record your screen with OBS on wlroots based compositors

@NilsIrl
Copy link

NilsIrl commented Dec 30, 2019

Feedback:

  • OBS and sway crashed when I pressed the tray icon.
  • the right side of OBS overflows at the right of the screen when the OBS window is too small (this was already the case with xwayland, and might be intended)
  • The context menu can appear outside of obs in which case you can't click on it. It can sometimes happen, where it's on another window or goes out of the screen. Firefox also has this issue but only on another window. The Firefox context menu never goes out of the screen (IIRC). I'll try to send the Firefox issue as well as it might explain the solution.

@NilsIrl
Copy link

NilsIrl commented Dec 30, 2019

The OBS window, also doesn't have an app_id. This is quite annoying, because it there is no way for me to inhibit my idle manager (screensaver), based on it. The app_id shouldn't change and should always be the same.

@NilsIrl
Copy link

NilsIrl commented Dec 30, 2019

Please keep in mind that some of these problems might have to be implemented by QT. And something that should be implemented by QT should definitively not be done by OBS, otherwise we have 2 layers (qt and obs) doing the same job.

@kkartaltepe
Copy link
Collaborator

kkartaltepe commented Dec 30, 2019

Feedback:

None of these sound related to this PR. Please file a bug or create a github issue for general issues you have with OBS. Bringing unrelated issues into this PR make it harder for people to review and make a proper review take even longer. (things like "Firefox also has this issue" or "this was already the case with xwayland")

@NilsIrl
Copy link

NilsIrl commented Dec 30, 2019

@kkartaltepe I'm sorry but all the bugs I said except this

the right side of OBS overflows at the right of the screen when the OBS window is too small (this was already the case with xwayland, and might be intended)

are bugs that are related to this branch. And what I mean by "related" is that they only appear on this branch

And the reason I brought it up is that it might be related to the Wayland environment.

@kkartaltepe
Copy link
Collaborator

My personal opinons,

OBS crashed when I pressed the tray icon

Provide a backtrace from GDB.

The context menu can appear outside of obs in which case you can't click on it. It can sometimes happen, where it's on another window or goes out of the screen. Firefox also has this issue but only on another window. The Firefox context menu never goes out of the screen (IIRC). I'll try to send the Firefox issue as well as it might explain the solution.

This PR does not change context menu handling, so this sounds unrelated to this PR. Have you tested the behavior under X11 and is it really unique to wayland? Perhaps you mean there is an existing issue in the handling of context menus?

The OBS window, also doesn't have an app_id.

This also sounds like an unrelated enhancement once wayland is supported. Notably this functionality is not at all required to begin supporting wayland so it makes this PR smaller and easier to review by leaving it out. It should be easy to submit a PR even in parallel for this functionality as it lkely can be done without any wayland support in OBS due to how Qt handles it.

@NilsIrl
Copy link

NilsIrl commented Jan 9, 2020

Provide a backtrace from GDB.

#0  0x00007ffff4e9cbe0 in raise () from /nix/store/xhpwab5kavygbr1fswawmdyqvmn3wa4i-glibc-2.27/lib/libc.so.6
#1  0x00007ffff4e9ddc1 in abort () from /nix/store/xhpwab5kavygbr1fswawmdyqvmn3wa4i-glibc-2.27/lib/libc.so.6
#2  0x00007ffff56081cb in QMessageLogger::fatal(char const*, ...) const () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#3  0x00007fffe8a486d5 in QtWaylandClient::QWaylandDisplay::checkError() const () from /nix/store/rb943zallv57ngylmgmkz31rp9fjqfxv-qtwayland-5.12.6/lib/libQt5WaylandClient.so.5
#4  0x00007fffe8a4872e in QtWaylandClient::QWaylandDisplay::flushRequests() () from /nix/store/rb943zallv57ngylmgmkz31rp9fjqfxv-qtwayland-5.12.6/lib/libQt5WaylandClient.so.5
#5  0x00007ffff582ed0e in QMetaObject::activate(QObject*, int, int, void**) () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#6  0x00007ffff583a559 in QSocketNotifier::activated(int, QSocketNotifier::QPrivateSignal) () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#7  0x00007ffff583a8a1 in QSocketNotifier::event(QEvent*) () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#8  0x00007ffff62c34d1 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Widgets.so.5
#9  0x00007ffff62ca970 in QApplication::notify(QObject*, QEvent*) () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Widgets.so.5
#10 0x00007ffff5805159 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#11 0x00007ffff5859617 in ?? () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#12 0x00007ffff116cd1d in g_main_context_dispatch () from /nix/store/p89djj12rg449dbmkfbcby5wilb9iyfy-glib-2.62.3/lib/libglib-2.0.so.0
#13 0x00007ffff116cfb8 in g_main_context_iterate.isra () from /nix/store/p89djj12rg449dbmkfbcby5wilb9iyfy-glib-2.62.3/lib/libglib-2.0.so.0
#14 0x00007ffff116d04c in g_main_context_iteration () from /nix/store/p89djj12rg449dbmkfbcby5wilb9iyfy-glib-2.62.3/lib/libglib-2.0.so.0
#15 0x00007ffff58589d3 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#16 0x00007ffff5803d7b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#17 0x00007ffff580c002 in QCoreApplication::exec() () from /nix/store/yihcrvx841yz6yp5zpmrq5ij4hqhp1bx-qtbase-5.12.6/lib/libQt5Core.so.5
#18 0x00000000004953bd in main ()

@hedgepigdaniel
Copy link

Nice work!

I've rebased this here: https://github.com/obsproject/obs-studio/compare/master...hedgepigdaniel:wayland?expand=1

The screencopy backend works well, but the dmabuf backend only shows a black screen. I see the following in the log:

info: User added source 'Wayland output(dmabuf)' (wlrobs-dmabuf) to scene 'Scene'
error: glViewport failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: device_set_viewport (GL) failed
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
error: glEnable failed, glGetError returned GL_INVALID_OPERATION(0x502)
info: User Removed source 'Wayland output(dmabuf)' (wlrobs-dmabuf) from scene 'Scene'

@NilsIrl
Copy link

NilsIrl commented Jan 12, 2020

The screencopy backend works well, but the dmabuf backend only shows a black screen. I see the following in the log:

I just tested the dmabuf backend and it works for me on the latest commit.

@NilsIrl
Copy link

NilsIrl commented Jan 27, 2020

obs tries to call xset leading the following logs:

/nix/store/bjj053z5pcfg3kig4z9h5f41qza6q9iq-xset-1.2.4/bin/xset:  unable to open display ""
/nix/store/bjj053z5pcfg3kig4z9h5f41qza6q9iq-xset-1.2.4/bin/xset:  unable to open display ""
/nix/store/bjj053z5pcfg3kig4z9h5f41qza6q9iq-xset-1.2.4/bin/xset:  unable to open display ""
/nix/store/bjj053z5pcfg3kig4z9h5f41qza6q9iq-xset-1.2.4/bin/xset:  unable to open display ""

Jason Francis added 2 commits February 6, 2020 15:23
@heyakyra
Copy link

This would solve #2471 which didn't need to be closed prematurely, right?

@NilsIrl
Copy link

NilsIrl commented Jul 20, 2020

This would solve #2471 which didn't need to be closed prematurely, right?

This wouldn't solve #2471. This PR just makes OBS work under wayland but doesn't provide any feature to capture the screen. Additionally work has mostly moved to #2484.

@Fenrirthviti
Copy link
Member

Should this PR be closed in favor of #2484? Or are there parts of this that should remain open?

@NilsIrl
Copy link

NilsIrl commented Jul 20, 2020

Should this PR be closed in favor of #2484? Or are there parts of this that should remain open?

It should probably be closed. Especially considering the author of this PR has acknowledged, the existence of the new PR.

@Fenrirthviti
Copy link
Member

Works for me, I'll go ahead and close this in favor. If this needs to be reopened, feel free to comment again and we can review!

@@ -0,0 +1,66 @@
# Try to find Wayland on a Unix system
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why you copied over this version of the FindWayland module. If you use the last version you'll get to use Wayland::Egl as a target removing quite some boilerplate code in the actual use of the module.

https://invent.kde.org/frameworks/extra-cmake-modules/-/blob/master/find-modules/FindWayland.cmake

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 Has Conflicts Linux Affects Linux Seeking Testers Build artifacts on CI Work In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants