Wayland Platform (Wayland, pt 2) #2482

GeorgesStavracas · 2020-03-10T19:19:45Z

(This depends on #2478 and thus includes its commits)

This pull requests builds on top of @w23 and @cyclopsian's work, and their code (or code heavily inspired by them) is appropriately credited.

Description

Introduce the concept of platforms to libobs, add a default platform (which is X11 on Unix, default Windows, and default Mac), and add a Wayland platform (only available on Unix).

Add a new ENABLE_WAYLAND build option to allow compiling OBS Studio with Wayland support. The Wayland platform (i.e the OBS_PLATFORM_WAYLAND enum value, and related code) is only available when building with -DENABLE_WAYLAND=true.

Motivation and Context

This is the 2nd step of RFC #14.

The goal of this pull request is introduce the concept of platforms to OBS Studio, and properly isolate X11-only code paths.

How Has This Been Tested?

Under a Wayland desktop environment, run:

$ QT_QPA_PLATFORM=wayland OBS_USE_EGL=1 obs

Running OBS Studio with those environment variable will crash. This is expected. The crash will be gone with EGL/Wayland winsys in libobs-opengl. Because running OBS Studio with QT_QPA_PLATFORM=wayland already results in segmentation faults, this shouldn't be considered a regression.

My particular setup is Linux + GNOME + Wayland.

Types of changes

New feature (non-breaking change which adds 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.

The code is generated by https://glad.dav1d.de/

This is in preparation for the future abstraction layer (gl-x11-*) and also to match the actual name of the windowing system. When running under X11, we can glue OpenGL through GLX or EGL, so the new file name matches that now.

Move the GLX-related code to gl-x11-glx, and introduce gl-nix as a winsys-agnostic abstraction layer. gl-nix serves as the runtime selector of which winsys going to be used. Only the X11/GLX winsys is available now, but later commits will introduce the X11/EGL winsys as well. The gl-nix code was originally written by Jason Francis <[email protected]>

Introduce the EGL/X11 winsys, and use it when the OBS_USE_EGL environment variable is defined. This variable is only temporary, for future commits will add a proper concept of platform. All the EGL/X11 code is authored by Ivan Avdeev <[email protected]>.

To keep consistency with the EGL line

List this dependency both under CI/install-dependencies-linux.sh, and .github/workflows/main.yml.

Currently, obs-nix.c is highly tied to the X11 display server. It includes X11 headers directly, and make use of X11 functions. Most of the code inside obs-nix.c that is X11-specific is related to hotkeys handling. Introduce a new vtable for hotkeys callbacks, that will used by X11 and Wayland to expose their specific routines. In this commit, only the X11 hotkeys vtable is implemented. Move all the X11-specific code to obs-nix-x11.c, and add a new function to retrieve the X11 hotkeys vtable.

This is a Unix-specific code. The only available platforms at this point are the X11/GLX and X11/EGL platforms. The concept of a platform display is also introduced. Again, the only display that is set right now is the X11 display.

Move the OBS_USE_EGL environment variable check to obs-app.cpp, and set the OBS platform to be either OBS_NIX_PLATFORM_X11_GLX or OBS_NIX_PLATFORM_X11_EGL.

Right now, linux-capture hard-depends on GLX. Disable it when running under EGL.

jp9000 · 2021-02-09T06:37:35Z

UI/obs-app.cpp

+#include <QX11Info>
+
+#ifdef ENABLE_WAYLAND
+#include <qpa/qplatformnativeinterface.h>


This concerns me. Is there no way to do what you want to do without requiring private Qt headers? I feel like I've asked about this before but it's been a long time.

Hm, I'm not aware of any replacement for that. This seems to be the documented way to retrieve the wl_surface from a window.

Yea that documentation seems to be from the wayland site, not the Qt site, which is fine I suppose, but generally it requires the qtbase5-private-dev on debian-based, and that might cause problems for other distributions. If you feel confident that other distros won't have problems, then that's fine I suppose. I don't really mind it, but usually using private headers for any dependency is somewhat taboo because of potential for trouble compiling at best, ABI breakage at worst if they change the virtual function table. But that particular header seems to be a known thing that people tend to use, so ABI breakage is probably unlikely, and Qt5 is pretty much on its last legs anyway so it probably doesn't matter.

Anyway, I hit this roadblock at first, and the cmake files weren't able to detect whether the required file/package was available or not. It just errored compiling instead. I think that ideally, we should get cmake to detect whether the required files or packages are installed to ensure others don't have the same issue.

And if the package/file isn't detected/available, it should probably still build, but throw a cmake warning and disable building of wayland stuff or something. I'm not sure.

Ideally, I just want to make the build process cleaner.

Ah, I see what you mean. I'll have a look at making CMake fail when that particular header is not there (it's the only header used from the private set, and I think it's safe to assume it'll be the only one too.)

Okay, I think this CMake change should work (haven't tested though). KDE folks pointed out this commit which does roughly the same, and I've replicated it here.

Looks good, detects it properly now. One question I might have is should we make it automatically compile without wayland support if the package isn't detected? Or should it be an explicit cmake option? I can't really decide. I think it's fine as is, but some people may be confused at first when they find that they can't compile OBS on Linux anymore. Might be good to have some sort of cmake warning at least telling them how to disable wayland support or something, or maybe just have it compile normally warning that wayland support was automatically disabled. I'm not sure.

Leaving it as a dependency unless they explicitly turn off support seems best to me. At least this way downstream maintainers know they need to take action (be it removing functionality or adding dependencies).

Introduce the OBS_NIX_PLATFORM_WAYLAND enum value, and try to detect it when OBS Studio runs by looking into the platform name.

We need to ensure we're running all X11 code on the same display.

jp9000 · 2021-02-10T04:00:22Z

Okay, the three pull requests look pretty good. Now the only problem is that they have merge conflicts between each of them (because I think they share commits). Should we create a single all-encompassing pull request for wayland support to solve this?

GeorgesStavracas · 2021-02-10T11:07:48Z

I think if you merge the 3rd PR, all the commits from the previous ones will be merged too and they'll be automatically closed by GitHub as merged too.

Gol-D-Ace · 2021-02-10T11:40:01Z

(This depends on #2478 and #2482 and thus includes its commits)
On the third PR

This was referenced Mar 10, 2020

[WIP] EGL/Wayland (Wayland, pt 3) #2483

Closed

Add RFC: Road to Native Wayland (Linux) obsproject/rfcs#14

Merged

EGL/Wayland (Wayland, pt 3) #2484

Merged

WizardCM added the Enhancement Improvement to existing functionality label Mar 11, 2020

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from 5707ede to a4c7f8e Compare March 13, 2020 01:06

ghost mentioned this pull request Mar 27, 2020

linux: add zero-copy screen capture using KMS and EGL #1758

Closed

10 tasks

GeorgesStavracas force-pushed the feaneron/wayland-platform branch 5 times, most recently from fc52b06 to 0cf9955 Compare April 12, 2020 02:50

kkartaltepe added Linux Affects Linux Seeking Testers Build artifacts on CI labels May 9, 2020

GeorgesStavracas force-pushed the feaneron/wayland-platform branch 2 times, most recently from c606fcd to 392e923 Compare August 10, 2020 13:47

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from 392e923 to a297698 Compare August 18, 2020 22:45

flexiondotorg mentioned this pull request Aug 22, 2020

Wayland support snapcrafters/obs-studio#50

Closed

GeorgesStavracas force-pushed the feaneron/wayland-platform branch 2 times, most recently from b7681d6 to df276d6 Compare August 27, 2020 17:22

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from df276d6 to 43d047e Compare September 20, 2020 15:26

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from 43d047e to 448e179 Compare October 3, 2020 17:12

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from 448e179 to d3dc76e Compare November 11, 2020 12:08

WizardCM mentioned this pull request Dec 28, 2020

Screen preview not working on wayland #3984

Closed

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from d3dc76e to abdb539 Compare December 28, 2020 15:29

kkartaltepe added this to the OBS Studio 27.0 milestone Jan 25, 2021

GeorgesStavracas added 5 commits February 1, 2021 19:03

deps-glad: Add EGL

7801b33

The code is generated by https://glad.dav1d.de/

libobs-opengl: Rename gl-x11.c to gl-x11-glx.c

83ae6f6

This is in preparation for the future abstraction layer (gl-x11-*) and also to match the actual name of the windowing system. When running under X11, we can glue OpenGL through GLX or EGL, so the new file name matches that now.

deps/glad: Make X11 required as well

59744e9

To keep consistency with the EGL line

GeorgesStavracas added 5 commits February 1, 2021 19:03

ci: Install qtbase5-private-dev on Linux

586767a

List this dependency both under CI/install-dependencies-linux.sh, and .github/workflows/main.yml.

libobs: Introduce the concept of a Unix platform

506b950

This is a Unix-specific code. The only available platforms at this point are the X11/GLX and X11/EGL platforms. The concept of a platform display is also introduced. Again, the only display that is set right now is the X11 display.

UI: Set the Unix platform on startup

27d0182

Move the OBS_USE_EGL environment variable check to obs-app.cpp, and set the OBS platform to be either OBS_NIX_PLATFORM_X11_GLX or OBS_NIX_PLATFORM_X11_EGL.

linux-capture: Fail to load when running on EGL

9810fe9

Right now, linux-capture hard-depends on GLX. Disable it when running under EGL.

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from abdb539 to 536aad3 Compare February 1, 2021 22:08

jp9000 reviewed Feb 9, 2021

View reviewed changes

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from 536aad3 to b502f2e Compare February 9, 2021 12:24

GeorgesStavracas added 2 commits February 9, 2021 09:39

libobs: Add a Wayland platform

2b3cb54

Introduce the OBS_NIX_PLATFORM_WAYLAND enum value, and try to detect it when OBS Studio runs by looking into the platform name.

libobs-opengl: Try to use the platform display if available

137966e

We need to ensure we're running all X11 code on the same display.

GeorgesStavracas force-pushed the feaneron/wayland-platform branch from b502f2e to 137966e Compare February 9, 2021 12:39

jp9000 merged commit 137966e into obsproject:master Feb 12, 2021

GeorgesStavracas deleted the feaneron/wayland-platform branch February 13, 2021 23:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wayland Platform (Wayland, pt 2) #2482

Wayland Platform (Wayland, pt 2) #2482

GeorgesStavracas commented Mar 10, 2020 •

edited

Loading

jp9000 Feb 9, 2021

GeorgesStavracas Feb 9, 2021

jp9000 Feb 9, 2021 •

edited

Loading

GeorgesStavracas Feb 9, 2021

GeorgesStavracas Feb 9, 2021

jp9000 Feb 10, 2021

kkartaltepe Feb 10, 2021 •

edited

Loading

jp9000 commented Feb 10, 2021

GeorgesStavracas commented Feb 10, 2021

Gol-D-Ace commented Feb 10, 2021 •

edited

Loading

Wayland Platform (Wayland, pt 2) #2482

Wayland Platform (Wayland, pt 2) #2482

Conversation

GeorgesStavracas commented Mar 10, 2020 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

jp9000 Feb 9, 2021

Choose a reason for hiding this comment

GeorgesStavracas Feb 9, 2021

Choose a reason for hiding this comment

jp9000 Feb 9, 2021 • edited Loading

Choose a reason for hiding this comment

GeorgesStavracas Feb 9, 2021

Choose a reason for hiding this comment

GeorgesStavracas Feb 9, 2021

Choose a reason for hiding this comment

jp9000 Feb 10, 2021

Choose a reason for hiding this comment

kkartaltepe Feb 10, 2021 • edited Loading

Choose a reason for hiding this comment

jp9000 commented Feb 10, 2021

GeorgesStavracas commented Feb 10, 2021

Gol-D-Ace commented Feb 10, 2021 • edited Loading

GeorgesStavracas commented Mar 10, 2020 •

edited

Loading

jp9000 Feb 9, 2021 •

edited

Loading

kkartaltepe Feb 10, 2021 •

edited

Loading

Gol-D-Ace commented Feb 10, 2021 •

edited

Loading