maximbaz / wluma

Automatic brightness adjustment based on screen contents and ALS
ISC License
631 stars 27 forks source link

[Feature request]: Support Hyprland screen capturing (after its transition away from wlroots) #111

Open Rishik-Y opened 3 months ago

Rishik-Y commented 3 months ago

Steps for reproducing the issue

Running Wluma on Hyprland

There is no issue, and the wluma works perfectly fine when keeping capture = "none"

but doesn't really work when keeping capture = "wlroots"

I was till now stuck in finding out how to set it up exactly properly

but thanks to https://github.com/maximbaz/wluma/issues/40

It was way easier, but one thing I am stuck on is that https://github.com/vega-d had gnome installed which doesn't have wlroots

While I have hyprland installed which I am pretty sure is based on wlroots Compositor

I feel like I am missing something since everything works just right when keeping capture = "none" and using my webcam as sensor.

Can anybody tell me what exactly is the issue here.

What is the buggy behavior?

❯ wluma
[2024-06-13T13:54:27Z INFO  wluma] Continue adjusting brightness and wluma will learn your preference over time.
thread 'predictor-eDP-1' panicked at src/frame/capturer/wlroots.rs:80:14:
Unable to init export_dmabuf_manager: Missing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

What is the expected behavior?

❯ wluma
[2024-06-13T13:51:29Z INFO  wluma] Continue adjusting brightness and wluma will learn your preference over time.

Logs

░▒▓   ~  
❯ RUST_LOG=debug wluma
[2024-06-13T13:58:18Z DEBUG wluma] Using Config {
        als: Webcam {
            video: 0,
            thresholds: {
                30: "dim",
                60: "bright",
                75: "outdoors",
                15: "dark",
                0: "night",
                45: "normal",
            },
        },
        output: [
            Backlight(
                BacklightOutput {
                    name: "eDP-1",
                    path: "/sys/devices/pci0000:00/0000:00:08.1/0000:07:00.0/drm/card2/card2-eDP-1/amdgpu_bl2",
                    capturer: Wlroots,
                    min_brightness: 1,
                },
            ),
            Backlight(
                BacklightOutput {
                    name: "keyboard-dell",
                    path: "/sys/class/net/enp4s0/enp4s0-2::lan",
                    capturer: None,
                    min_brightness: 0,
                },
            ),
        ],
    }
[2024-06-13T13:58:18Z INFO  wluma] Continue adjusting brightness and wluma will learn your preference over time.
thread 'predictor-eDP-1' panicked at src/frame/capturer/wlroots.rs:80:14:
Unable to init export_dmabuf_manager: Missing
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Version

4.4.0-1

Environment

░▒▓   ~  
❯ lsb_release -a; uname -a; pgrep -l sway; pacman -Q | egrep "(wlroots|vulkan|sway|clang|rust)"; dpkg -l | egrep "(wlroots|vulkan|sway|clang|rust)"
bash: lsb_release: command not found
Linux archlinux 6.9.3-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 31 May 2024 15:14:45 +0000 x86_64 GNU/Linux
egrep: warning: egrep is obsolescent; using grep -E
clang 17.0.6-2
rust 1:1.78.0-1
swaylock-effects 1.7.0.0-4
swaylock-effects-debug 1.7.0.0-4
vulkan-headers 1:1.3.285-1
vulkan-icd-loader 1.3.285-1
vulkan-intel 1:24.1.1-1
vulkan-radeon 1:24.1.1-1
vulkan-validation-layers 1.3.275-1
wlroots 0.17.3-1
bash: dpkg: command not found
egrep: warning: egrep is obsolescent; using grep -E
maximbaz commented 3 months ago

Hello! It doesn't sound like you are missing a dependency or something like that, and there have been people writing in this project who were using wluma on hyprland at the time, so it definitely used to work. Hyprland actually runs on a fork of wlroots, so it's possible that it differs too much from vanilla wlroots today, or that there's a bug somewhere (also possibly in wluma code!).

https://github.com/maximbaz/wluma/issues/88#issuecomment-1711797135

I'll try my luck: @riley-martin do you still use wluma and hyprland?

Rishik-Y commented 3 months ago

Thanks very much for looking into it. I like using Wluma, but I really don't know where the hiccup I am getting stuck is. I will post if I do find anything regarding this.

(Again, I am really sorry I clicked closed by mistake and so I opened it again)

riley-martin commented 3 months ago

I'll try my luck: @riley-martin do you still use wluma and hyprland?

I don't use hyprland much anymore. I'd be glad to test it when I get a chance but it might not happen right away.

riley-martin commented 3 months ago

I did some testing tonight and it didn't work for me either. Looking at some recent release notes (0.40.0 and 0.41.0) it appears that hyprland is switching to their own wayland implementation instead of using wlroots, so I guess that would be the problem.

maximbaz commented 3 months ago

Oh great catch, thanks for pointing it out!

We can conclude that wluma's wlroots capturer is not expected to work on Hyprland anymore.

We can leave this issue open as a feature request, in case someone from community would like to implement a new capturer for Hyprland (perhaps wait until Hyprland completes their transition away from wlroots first).

Thanks @Rishik-Y for spotting the issue and @riley-martin for tracking down the root cause!

Rishik-Y commented 3 months ago

I did some testing tonight and it didn't work for me either. Looking at some recent release notes (0.40.0 and 0.41.0) it appears that hyprland is switching to their own wayland implementation instead of using wlroots, so I guess that would be the problem.

Oh, Right, I totally didn't see that, All this while, I have been breaking myself, thinking I may be bad at understanding or I simply broke stuff somewhere. It seems the issue was simply something that had been in my blind spot.

Thank you, @riley-martin, for checking up on this issue and @maximbaz, for quickly responding to my issue. Finally, I can rest at ease now.

Rishik-Y commented 2 months ago

Hey @maximbaz, I have been trying to understand the code and how it works for the past few weeks and have kind of found the exact issue a while back.

To better give the info from What I understood.

The issue shown on my screen is: Unable to init export_dmabuf_manager: Missing at line 80 Which is in wluma/src/frame/capturer/wlroots.rs.

Which is the error when it executes

        let dmabuf_manager = globals
            .instantiate_exact::<ZwlrExportDmabufManagerV1>(1) 

Which is at line 78.

I have tried to understand where does the ZwlrExportDmabufManagerV1 initiates from, but it seems that main.rs doesn't have much relation to wlroots, so the first line starts from

use wayland_protocols::wlr::unstable::export_dmabuf::v1::client::{
    zwlr_export_dmabuf_frame_v1::{CancelReason, Event},
    zwlr_export_dmabuf_manager_v1::ZwlrExportDmabufManagerV1,
};

Which is on line 8 of wlroots.rs.

As @riley-martin stated, Hyprland Had changed the compositor from original wlroots to modified wlroots.

I tried to investigate what exactly was modified in the directories and found out that the directories were changed /usr/include/wlr/types/wlr_export_dmabuf_v1.h instead of wlroots folder (Which is not that much of an issue... yet) but wlr_export_dmabuf_v1.h contains the code very similar to the original wlr-export-dmabuf-unstable-v1.h (I think?) Where instead of zwlr_export_dmabuf_manager_v1, it contains wlr_export_dmabuf_manager_v1 everywhere, and some minor changes are made.

I have thought about changing the file itself, but I know it will instead break my system apart, so I tried finding ways to change the code for Hyprland Users, but the wluma program uses a crate called wlroots-Protocols which gives the program much of its wlroots functionality.

I believed that it shouldn't be much of a hassle to fix this, but it turns out there is no crate created for Hyprland Modified wlroots.

I tried finding out using other crates whether it would work or not by adding the wlroots-protocol-wlr crate into the Cargo.toml and modified a bit

like in line 8 I kept, use wayland_protocols_wlr::export_dmabuf::v1::client::{ (For trial and error)

And the program seems to detect it by showing the error in other code instead of that line where I need to modify to suit it with the use wayland-protocols-wlr, but even then (from what I think) it wont work because even in that the code is written as zwlr_export_dmabuf_manager_v1, and i really don't know now what to try next.

I tried asking Chatgpt, and it states that to make a fork of wayland-protocol-wlr crate and and modify there, and add that in cargo.toml, which seems again very complicated to me.

I Have become very obsessed with trying to make this program run on Hyprland. I even started learning rust in the past three days just so I could make it work.

I would really appreciate your thoughts on this, where I am assuming something is wrong, and what I should be doing next.

I can understand Rust somewhat since I already know a bit of C/C++, Python, Java and a bit of Haskell. But should I complete learning the basics of rust first? (it would probably take a few more days)

Note: I am not asking you to fix it; I am simply asking you to give me any tips on what I can do to contribute here and make wlroots work on my Hyprland. Your expertise in rust would be greatly appreciated.

PS: Please don't be too harsh with me; I am just a second-year College student.

maximbaz commented 2 months ago

Nice work and analysis, @Rishik-Y!

First of all, it looks like the dmabuf protocol has been marked stable, so in wlroots.rs we should remove the ::unstable parts (and probably cargo update the corresponding crate). The struct names still start with Zwlr, so perhaps you don't even need to change anything more than the import.

It might be that this change alone fixes wluma for Hyprland, if the only difference they did for now was to get rid of the now obsolete "unstable" version of the dmabuf protocol. Before we discuss more ideas, could you try this?

Rishik-Y commented 2 months ago

Thanks for the suggestion @maximbaz, I already tried removing the ::unstable tag (Through trial and error ☺️) So unfortunately, it doesn't work.

Also I am sorry for my wording if it was not explained by me properly, but what I meant was ZwlrExportDmabufManagerV1 is the same here with wlrExportDmabufManagerV1 instead.

Here is the attachment of the code for wlr_export_dmabuf_v1.h

/*
 * This an unstable interface of wlroots. No guarantees are made regarding the
 * future consistency of this API.
 */
#ifndef WLR_USE_UNSTABLE
#error "Add -DWLR_USE_UNSTABLE to enable unstable wlroots features"
#endif

#ifndef WLR_TYPES_WLR_EXPORT_DMABUF_V1_H
#define WLR_TYPES_WLR_EXPORT_DMABUF_V1_H

#include <stdbool.h>
#include <wayland-server-core.h>
#include <wlr/render/dmabuf.h>

struct wlr_export_dmabuf_manager_v1 {
    struct wl_global *global;
    struct wl_list frames; // wlr_export_dmabuf_frame_v1.link

    struct wl_listener display_destroy;

    struct {
        struct wl_signal destroy;
    } events;
};

struct wlr_export_dmabuf_frame_v1 {
    struct wl_resource *resource;
    struct wlr_export_dmabuf_manager_v1 *manager;
    struct wl_list link; // wlr_export_dmabuf_manager_v1.frames

    struct wlr_output *output;

    bool cursor_locked;

    struct wl_listener output_commit;
    struct wl_listener output_destroy;
};

struct wlr_export_dmabuf_manager_v1 *wlr_export_dmabuf_manager_v1_create(
    struct wl_display *display);

#endif

Here are the files which were supposed to work wluma However I believe is modified by hyprland, Hence not working anymore. image

I don't know whether it's just me making rookie mistakes, but from what I understood these are the modified wlroots used by Hyprland. Really appreciate any tip.

maximbaz commented 2 months ago

It looks like the file is actually identical to the wlroots sources :thinking:

https://gitlab.freedesktop.org/wlroots/wlroots/-/blob/master/include/wlr/types/wlr_export_dmabuf_v1.h

Don't worry this is definitely not rookie mistakes, from what we see so far it looks like the header files of the dmabuf did not change between wlroots and Hyprland (yet), so there must be something else that has changed or has broken :thinking: I think the Z in ZwlrExportDmabufManagerV1 means something else during the transition from C to Rust, as you can see Z is also absent in the C sources above from wlroots repo.

Rishik-Y commented 2 months ago

Oh, I totally didn't Know that. Thanks for Checking up @maximbaz I would have wasted a lot of time on this, if it weren't for you. I will continue to find the actual issue on where its causing.

Rishik-Y commented 2 months ago

Hello @maximbaz, So sorry that i keep disturbing you again and again. I just want to ask, Was there any specific reason why you decided to use wayland_protocolsV0.29.5 (Which includes wlr modules) or Will it be fine if I update the version of the wayland_protocols to V0.30.0 or higher (Which separated wlr modules to wayland_protocols_wlr extension, Assuming I am able to integrate it in the wlroots.rs file)

maximbaz commented 2 months ago

Don't worry, ask as much as you need 😁

The reason is that 0.29 was latest when I wrote that code initially, and migrating to 0.30 was a bit tricky so I never succeeded to make that work 😭

https://github.com/maximbaz/wluma/pull/86

maximbaz commented 2 months ago

That is all to say, if you want to give it a go, please do!

Rishik-Y commented 2 months ago

Don't worry, ask as much as you need 😁

Thank you very much. ☺️

The reason is that 0.29 was latest when I wrote that code initially, and migrating to 0.30 was a bit tricky so I never succeeded to make that work 😭

I had initially assumed that you probably chose that specific version probably due to wlr being incorporated in wayland-protocols or having incompatibility issues in further versions (at least, I hoped That was the case so that I would indirectly have a reason not to touch the complicated stuff) But it seems that's not the case. 🥲

That is all to say, if you want to give it a go, please do!

I know I will suffer a lot while figuring things around, However, I have become somewhat fixated on this project, so I will try migrating it to 0.30 or try other ways to see if it will be fixed for my Hyprland. 😁 Again thank you for your response.

daniel-sousa-me commented 2 months ago

I just found this project after weeks of searching for something like this. I got a new laptop with an oled screen. It is amazing not to have a light-bulb constantly pointed at my face, but now it is extra annoying when I open a bright webpage. The proper way to fix this would probably be on the firmware's brightness control, but it's closed source, so a hack like this is probably required :(

Unfortunately I'm running into this same problem.

@maximbaz Which wm do you use? I don't know rust, but I'd like to try to make this work.

From what I'm reading here, it seems like it would make sense to try to upgrade the wayland_protocols. Even if it doesn't solve our problem with hyprland, at least we'd have a contribution for this project :)

maximbaz commented 2 months ago

I'm using sway, and yeah I agree finishing #86 would be quite awesome, and help is certainly appreciated as I dont know when I can get to it... Splitting upgrade into wayland and not wayland to make it simpler could be a good way to start

daniel-sousa-me commented 2 months ago

I'm new to wayland. I thought hyprland and sway were different levels and that hyprland was using sway.

I've found this program is also grabbing frames with the same rust library, but using the new version: https://github.com/russelltg/wl-screenrec/blob/a3d0b5da69487441ce81ebfc3be8ba8d2255c714/src/main.rs

It works on hyprland. From what I understand, the things from the wlroots protocol are being integrated into the wayland protocol with the goal that all compositors use it. So hyprland most likely will try maintain compatibility.

I'm trying to use it as reference to update wluma's code, but it's going slowly because I have no idea what I'm doing

Rishik-Y commented 1 month ago

Hi @daniel-sousa-me, Nice to see you here as well trying to help fix the issue.

Also Hello @maximbaz It's been a few weeks and I have been learning rust and trying ways to fix the issue but turns out the Hyprland has completely migrated from wlroots-based wayland compositor written over their website But they do claim that the wlroots apps should continue to work.

In any case, The issue is still persistent, and I am still continuing to find a fix for my laptop.

I was just wondering whether updating the version of the toml files truly be a fix for it or should I go in some other route trying other ways instead?

wolf-yuan-6115 commented 1 month ago

But they do claim that the wlroots apps should continue to work.

I think it's not referring to everything depending on wlroots work, they are talking about "apps" here, it might only mention apps that use wlroots to display content. (Not sure, just my guess)

wluma doesn't display any GUI, it's capturing the display, so it's probably not in that scope.

maximbaz commented 1 month ago

I'm reasonably sure the primary reason for it not working on Hyprland is that the current code uses "unstable" dmabuf protocol (because it was unstable at the time of writing it), and Hyprland after the rewrite only announces the support for the stable one:

https://github.com/hyprwm/Hyprland/blob/1fa4b7d79baaad47fde8e72221cd77f569fbfe35/CMakeLists.txt#L307

We should really upgrade wayland_protocols in this repo, and then hopefully it will work, unless then we discover some other incompatibility issues between wlroots and hyprland :slightly_smiling_face:

Rishik-Y commented 6 days ago

Hey @maximbaz,

I was just looking through your code (trying to understand what each and every code is doing before again touching the wlroot.rs) And I didn't understand what exactly is this code doing (The bottom code is from wluma/src/brightness/backlight.rs (line 75)

        let brightness_hw_changed_path = Path::new(path).join("brightness_hw_changed");
        if Path::new(&brightness_hw_changed_path).exists() {
            inotify
                .watches()
                .add(&brightness_hw_changed_path, WatchMask::MODIFY)?;
        }

Is this the file which was created as a replacement for the original file if it doesn't have write permissions?

Even if it is, why and how did the name "brightness_hw_changed" even come? It could be from other files which I haven't read yet, but I can't seem to find what exactly it does except join brightness_hw_changed to the path, and check if it exists (Which I don't think would exist) and assuming that it does, it will simply monitor whether it's modified or not.

Please Correct me if I am wrong about it.

The main concern for me is when I usually remove some stuff from code, it breaks, and I would realise where exactly it is being used, but the thing here is, it works without any issue, with no error or warning.

maximbaz commented 6 days ago

This file brightness_hw_changed may or may not exist, when it does, it can indicate that brightness was changed by hardware rather than software. The file name is decided by the Linux kernel.

That code establishes inotify watches on several files, which allows us to run some code when any of those files get changed. Meaning that whenever brightness was changed (using software or hardware), we want to execute update function, which reads the current brightness value:

https://github.com/maximbaz/wluma/blob/08b48960cbb8325a8546d51aadc3ed5461747e74/src/brightness/backlight.rs#L103-L111

This is to avoid having to periodically check for the current brightness value, which is not very battery-efficient.

Hope that helps!

Rishik-Y commented 6 days ago

No, I didn't mean that Inotify is not needed by any means. I am sorry In case it was portrayed that way from my side. 😟

That code establishes inotify watches on several files, which allows us to run some code when any of those files get changed. Meaning that whenever brightness was changed (using software or hardware), we want to execute update function, which reads the current brightness value:

I totally understand the need for it.

What I don't get here is

This file brightness_hw_changed may or may not exist, when it does, it can indicate that brightness was changed by hardware rather than software. The file name is decided by the Linux kernel.

Can you clarify whether there may be some people who may get "brightness_hw_changed" and its just in my system that the file "brightness_hw_changed" doesn't appear? Or does it get generated somehow by the system?

Also if the File name is decided by the linux kernal, then why did we hardcode it there with brightness_hw_changed in the code?

    let brightness_hw_changed_path = Path::new(path).join("brightness_hw_changed");

Sorry for asking sudden questions; I just wanted to know more about how the program works. 😀

maximbaz commented 6 days ago

Can you clarify whether there may be some people who may get "brightness_hw_changed" and its just in my system that the file "brightness_hw_changed" doesn't appear?

Correct, some people may have this file, and others may not. My current system doesn't have that file either.

Also if the File name is decided by the linux kernal, then why did we hardcode it there with brightness_hw_changed in the code?

If it will be present, it will be called brightness_hw_changed, i.e. linux has decided to call that file brightness_hw_changed and nothing else. Similarly it decided to call the file that contains the brightness value brightness, and we also hardcode its name:

https://github.com/maximbaz/wluma/blob/08b48960cbb8325a8546d51aadc3ed5461747e74/src/brightness/backlight.rs#L28

And similarly it decided to name the file that contains maximum brightness value max_brightness, which is also hardcoded:

https://github.com/maximbaz/wluma/blob/08b48960cbb8325a8546d51aadc3ed5461747e74/src/brightness/backlight.rs#L68

I hope I understood the questions correctly?

Rishik-Y commented 6 days ago

Ohh! I see, It makes a lot of sense now. Sorry for taking your time, and thank you for your explanation.