Almamu / linux-wallpaperengine

Wallpaper Engine backgrounds for Linux!
GNU General Public License v3.0
1.59k stars 61 forks source link

Wayland Driver #159

Closed vaxerski closed 1 year ago

vaxerski commented 1 year ago

This implements a wayland driver.

Issues that need to be addressed:

Subsequent MR plans:

Some of the issues are best directed at @Almamu as the lead dev of this:

Gustash commented 1 year ago

I'm not sure if the same happens to you, but on my system videos get rendered flipped on the Y axis.

I was able to fix this by making CWaylandOutput::renderVFlip () return true instead, but I'm not sure why this is necessary compared to CX11Output.

For more context, I am using Sway 1.8.1-1 on Arch.

EDIT:

It seems like flipping vertically is also required for CGLFWWindowOutput, as that is returning true. So it seems like CX11Output is the outlier afterall.

Almamu commented 1 year ago

It seems like flipping vertically is also required for CGLFWWindowOutput, as that is returning true. So it seems like CX11Output is the outlier afterall.

My mind is a bit hazy on why exactly it was required, but it's this way due to how the images are drawn on screen. CX11Output is using a image buffer that is filled with glReadPixels, so that already flips it upside down, while the rendering onto the screen requires you to do that manually. This is mainly because OpenGL works with a right-hand coordinates system: https://www.google.com/search?client=firefox-b-d&q=opengl+coordinate+system

Gustash commented 1 year ago

It seems like flipping vertically is also required for CGLFWWindowOutput, as that is returning true. So it seems like CX11Output is the outlier afterall.

My mind is a bit hazy on why exactly it was required, but it's this way due to how the images are drawn on screen. CX11Output is using a image buffer that is filled with glReadPixels, so that already flips it upside down, while the rendering onto the screen requires you to do that manually. This is mainly because OpenGL works with a right-hand coordinates system: https://www.google.com/search?client=firefox-b-d&q=opengl+coordinate+system

That makes sense. Since the wayland driver is just a client that renders the buffer in the shell layer, it probably abides by the same rules as a normal window, hence the need to flip manually

vaxerski commented 1 year ago

lmao, haven't noticed it with my wallpaper. Yeah. Fixed that flip

vaxerski commented 1 year ago

wayland impl now optional with -DENABLE_WAYLAND=True/False off by default protocols now built with protocols.sh ran automatically at configure if wayland

Almamu commented 1 year ago

To search for EGL in CMake, wouldn't FindOpenGL module be a better fit than searching for the libraries manually? https://cmake.org/cmake/help/latest/module/FindOpenGL.html

Regarding wayland protocols, I'm looking at how other apps do it, wlc seems to use a custom CMake module, but I guess it will only be useful for xdg-protocol and not for "wlr-layer-shell-unstable-v1", right?: https://github.com/Cloudef/wlc/blob/master/CMake/FindWaylandProtocols.cmake

An alternative would be to use something like this to search for the wayland-scanner binary and the protocols: https://github.com/KDE/extra-cmake-modules/blob/master/find-modules/FindWaylandScanner.cmake https://github.com/KDE/extra-cmake-modules/blob/master/find-modules/FindWaylandProtocols.cmake

I'd prefer if all the build steps are contained within the CMake files and do not depend on external shell scripts whenever possible.

vaxerski commented 1 year ago

fair, I'll try to see what I can do in cmake.

vaxerski commented 1 year ago

protocols now build within cmake

vaxerski commented 1 year ago

Also worth noting the wayland impl has 0 detection for fullscreen. We could use a wlr protocol but it's not supported outside wlroots. Since it's not really a critical thing, we can either impl it in a followup mr some time later or wait for the ext- mr

vaxerski commented 1 year ago

also, @Almamu how can I test mouse input? I launched some wallpapers that show stuff on mouse hover but no mouse input seems to be registered. printing values they do seem to be registered, but it's kinda "just the values" method and I can't see the effect. Or is that still a todo and not impl'd yet?

Almamu commented 1 year ago

Also worth noting the wayland impl has 0 detection for fullscreen. We could use a wlr protocol but it's not supported outside wlroots. Since it's not really a critical thing, we can either impl it in a followup mr some time later or wait for the ext- mr

We can work on that after the main wayland implementation is working. Fullscreen detection is not really a big concern. For now writting a dummy implementation that just returns false on anythingFullscreen should be enough.

also, @Almamu how can I test mouse input? I launched some wallpapers that show stuff on mouse hover but no mouse input seems to be registered. printing values they do seem to be registered, but it's kinda "just the values" method and I can't see the effect. Or is that still a todo and not impl'd yet?

Depends on what the background is doing. The easiest to debug should be a XRay background, as It's easy to check if the effect is working or not, any that appear on the steam workshop search should be fine. They're mostly NSFW by the way

vaxerski commented 1 year ago

Depends on what the background is doing. The easiest to debug should be a XRay background, as It's easy to check if the effect is working or not, any that appear on the steam workshop search should be fine. They're mostly NSFW by the way

That was precisely what I was testing it on. No mouse seems to register, even in an gflw window, regardless of the fact that CMouseInput does set the coordinates.

Almamu commented 1 year ago

That was precisely what I was testing it on. No mouse seems to register, even in an gflw window, regardless of the fact that CMouseInput does set the coordinates.

That's weird :/ I'm testing with https://steamcommunity.com/sharedfiles/filedetails/?id=2858510065 and seems to work just fine on GLFW. What background are you using? Might be something specific to the one you picked.

vaxerski commented 1 year ago

oh, that one does indeed work. Odd. 1583182109 and 1512911743 did not.

vaxerski commented 1 year ago

mouse handling done, thanks for the fix too!

vaxerski commented 1 year ago

Am I testing multi-monitor right?

./linux-wallpaperengine --assets-dir ./assets -i DP-2 ./1512911743 -i DP-3 ./1618674660 This applies 1512911743 to both my screens :thinking:

Edit: yeah, --bg. Now I get asset exceptions tho. Odd.

Almamu commented 1 year ago

Am I testing multi-monitor right?

./linux-wallpaperengine --assets-dir ./assets -i DP-2 ./1512911743 -i DP-3 ./1618674660 This applies 1512911743 to both my screens thinking

Based on how the code is designed, that looks right to me. Most likely the issue is with the rendering. If you take a look with RenderDoc you can see exactly what's going on. If I had to guess, something between the viewports for the render context is not right and is rewriting the contents.

EDIT: Oh yeah, --bg is needed

vaxerski commented 1 year ago

yeah, now I get this: image

image

if I load more than one scene at once.

Doesn't seem like a Driver could cause that tho?

Tried with multiple scene combos

Loading the same wp for both is fine

Almamu commented 1 year ago

That one is weird... Let me take a look, might be something I missed while testing... Mixing video and scene backgrounds seem to work just fine

vaxerski commented 1 year ago

I can't test video cuz videos atm literally crash on the wl driver for some reason, mpv insists there is no gl context

Almamu commented 1 year ago

The exception should be fixed now, there was some exceptions that should have been caught but were not...

vaxerski commented 1 year ago

brilliant, works! and so does multimon on wayland :D Thanks as usual

image

vaxerski commented 1 year ago

@Almamu I fixed videos, but audio is completely broken on pipewire.

image

CPulseAudioPlayingDetector::update () @ pa_mainloop_iterate (this->m_mainloop, 1, nullptr); (L82 CPulseAudioPlayingDetector.cpp) void CAudioDriver::update () @ this->m_detector.update (); (L15 CAudioDriver.cpp)

Commenting out audio updates fixes the crash, but obviously, no audio.

Adding a nullcheck fixes the crash: image but still doesn't seem to output audio.

That's with video playing.

vaxerski commented 1 year ago

All issues off the list have been done, now it's just a matter of code tidiness and eventual comments from Almamu.

Seems to work well on my end. (Except for the video audio crash lole)

Gustash commented 1 year ago

@vaxerski can you check if the screenshot flag is working on your end?

When I last tried it, it gave me a 3840x2160 image with the background in the top-left quadrant.

I have a dual 1080p monitor setup, and from skimming your code it seems like the function for setting up the full height and width is just summing all the widths and heights of the monitors indiscriminately.

This might have been fixed with your latest changes, haven't checked yet

vaxerski commented 1 year ago

oh right you can take screenshots with this thing. Right.

Does this take a screenshot of like all monitors on X? if so, this might be a bit tricky.

Gustash commented 1 year ago

oh right you can take screenshots with this thing. Right.

Does this take a screenshot of like all monitors on X? if so, this might be a bit tricky.

I'm not sure because I only have a use for it now (using the first frame of each wallpaper in swaylock).

But from the code it seems to fill out a bitmap with the OpenGL render, so it's not an actual monitor screenshot. Therefore, Wayland limitations shouldn't apply in this case

vaxerski commented 1 year ago

But from the code it seems to fill out a bitmap with the OpenGL render, so it's not an actual monitor screenshot. Therefore, Wayland limitations shouldn't apply in this case

That's precisely why they do apply.

Gustash commented 1 year ago

But from the code it seems to fill out a bitmap with the OpenGL render, so it's not an actual monitor screenshot. Therefore, Wayland limitations shouldn't apply in this case

That's precisely why they do apply.

Ah, sorry, I only have limited experience with wayland clients so I must have misunderstood something. I guess we should wait for someone who can tell us what the behavior is on X11 first

Almamu commented 1 year ago

oh right you can take screenshots with this thing. Right.

Does this take a screenshot of like all monitors on X? if so, this might be a bit tricky.

It takes a screenshot of the framebuffer to which the app renders to. In X11 mode that's a big ass texture that spans the whole screen configuration. For example, this is mine with 4 desktop backgrounds:

https://imgur.com/a/I6Pi6Ad

It's up to you how to implement it really. Ideally the wayland output would use that same framebuffer to output to the screens, otherwise you'll need to provide custom implementations of the screenshot taking code.

vaxerski commented 1 year ago

Screenshots in one wallpaper mode are fixed. Multi-wallpaper mode is broken. The first image will render properly, the next is random noise, which is probably bad reads.

It takes a screenshot of the framebuffer to which the app renders to. In X11 mode that's a big ass texture that spans the whole screen configuration. For example, this is mine with 4 desktop backgrounds:

Yeah that's the issue, in Wayland every screen is its own EGL surface.

I will fix this in a sec, tried one method and managed to crash mesa 10 times :P

vaxerski commented 1 year ago

fixado. Also made them properly flipped, as previously they werent and would be upside down

only caveat is that it will not take into account monitor positions. It just crudely goes LtR on monitors in the order they are reported.

I'd rather not bind into zxdg_output_management for that (and besides, if it's for pywal and stuff, who cares, really)

Almamu commented 1 year ago

I've pushed a commit with small fixes and improvements on how everything is organized. Everything seems to work perfectly on KDE with Wayland under Arch, but It'd be good if you can test it on wlroots too just to make sure I didn't break anything.

Regarding the screenshot, yeah, the position doesn't really matter much. The main idea was pywal, and I also use it from time to time to compare background rendering between commits in case something too big appears.

This is the commit message, which includes everything I did:

Wayland implementation cleanup

Removed wayland-display in favour of screen-root, it should automatically switch between X11 and Wayland implementations
Removed all the window-server-specific code from CWallpaperApplication
Moved COutput from CWallpaperApplication to each CVideoDriver as the output is tied to the driver
Moved CFullScreenDetector from CWallpaperApplication to each CVideoDriver as the detection is tied to the video driver
Applied the Driver treatment to the Input, this way CInputContext doesn't depend on the driver used
Updated CRenderContext to be aware of viewport-specific context updates
Viewport information is now held inside COutputViewport instead of being a simple map
Merged CLayerSurface and SWaylandOutput, inheriting from the new COutputViewport to standarize the minimum requirements of a viewport
  (makeCurrent and swapOutput are common requirements, X11 was the outlier not needing these per-viewport)
Moved all the viewport-specific code of Wayland from CWaylandOpenGLDriver to It's own class CWaylandOutputViewport
Fixed an issue under Wayland where sometimes the background surfaces wouldn't get anything drawn because the CWaylandOutput wasn't reset
Updated screenshot-taking code to be standard for all drivers again

Signed-off-by: Alexis Maiquez <almamu@almamu.com>

If everything works fine I'll fix the build issues on ubuntu and merge the changes into main.

Almamu commented 1 year ago

Thanks for the work on the Wayland implementation @vaxerski!!

vaxerski commented 1 year ago

Mouse input got completely fucked, I'll see what happened. It blinks like crazy and is lagging behind severely

Also still keep in mind about my message with the video crash :P

vaxerski commented 1 year ago

Also, technically we shouldn't need a fullscreen detector, as the frameCallback should stop being fired once the surface is fully occluded in an ideal world, like it is with recent Hyprland commits.

Although idk how it is in kde/other compositors, they probably don't do this

Almamu commented 1 year ago

Mouse input got completely fucked, I'll see what happened. It blinks like crazy and is lagging behind severely

That's weird, I tried a couple of XRay backgrounds and seemed to work just fine on KDE...

Also still keep in mind about my message with the video crash :P

I've tried a couple of video backgrounds with sound and none seem to crash, which ones are you trying?

Also, technically we shouldn't need a fullscreen detector, as the frameCallback should stop being fired once the surface is fully occluded in an ideal world, like it is with recent Hyprland commits.

Although idk how it is in kde/other compositors, they probably don't do this

I'll try it under KDE and see what happens.

vaxerski commented 1 year ago

I've tried a couple of video backgrounds with sound and none seem to crash, which ones are you trying?

pipewire issue. Any video, because see https://github.com/Almamu/linux-wallpaperengine/pull/159#issuecomment-1517898688

vaxerski commented 1 year ago

That's weird, I tried a couple of XRay backgrounds and seemed to work just fine on KDE...

works ok with one wallpaper. With more, it flashes between the two rapidly and is delayed. Launch the wallpaper you linked me before on two screens.

vaxerski commented 1 year ago

Another regression: no longer able to CTRL+C in a terminal to exit linux-wallpaperengine. I have to hard kill the app

image

Almamu commented 1 year ago

I've tried a couple of video backgrounds with sound and none seem to crash, which ones are you trying?

pipewire issue. Any video, because see #159 (comment)

Oh yeah... Audio playback is handled by SDL (for audio files) and MPV for video. By default the volume is low, so maybe that's why you're not hearing it? Try --volume 100, it defaults to 15, which might be a bit low

vaxerski commented 1 year ago

it's not hearing, it segfaults on a null char*. I can live without audio, and now that I tested it, commenting out the detector and recorder fixes the crash while outputting audio

edit: yup that fix did it, and there is audio. Thanks :)

vaxerski commented 1 year ago

Nuuuu you can't do that :( Wayland cannot work like that. The surface frame event loop is mandatory

Almamu commented 1 year ago

Nuuuu you can't do that :( Wayland cannot work like that. The surface frame event loop is mandatory

I can revert the change and fix it differently, sure. Everywhere I look says that eglSwapBuffers should lock until the frame is presented, so in theory it should be similar to doing it manually with the surface frame event loop. (and seems to work locally).

vaxerski commented 1 year ago

But all other handlers are instantly completely unsafe and broken. Mouse input, monitor changes, etc. You need to use the wayland loop.

To check for exits, you should read the value of display dispatch and exit when it returns -1

vaxerski commented 1 year ago
void CWaylandOpenGLDriver::dispatchEventQueue() const
{
    wl_display_dispatch(m_waylandContext.display);
}

if wl_display_dispatch returns -1 it means you should exit

see https://gitlab.freedesktop.org/wlroots/wlroots/-/blob/master/examples/layer-shell.c#L662

vaxerski commented 1 year ago

generally in wayland you almost always want the event loop to be just

while (wl_display_dispatch(display) != -1) { }

(of course && keepRunning is fine too)

vaxerski commented 1 year ago

I could fix both of the issues (mouse & exit) but I don't wanna be annoying and commit when you are working on it at the same time :P

vaxerski commented 1 year ago

properly fixed exits

vaxerski commented 1 year ago

fixed mouse issues. @Almamu you forgot that in wayland, each output has its own callbacks. We should not render all when only one fires. We render only the one whose event fired.

With this all seems to be alright to me

Edit: actually I havent checked screenshots, I'll check them in 20 mins Screenshots work.