Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

Layer shell #49

Merged
merged 44 commits into from
Dec 25, 2020
Merged

Layer shell #49

merged 44 commits into from
Dec 25, 2020

Conversation

guidocella
Copy link
Contributor

@guidocella guidocella commented Aug 24, 2020

I am attempting to adapt the code from sway/desktop/layer_shell.c (no popups yet).
This doesn't work yet and it's based on a mediocre understanding of Wayland and C, so help is welcome!
Fixes #48, fixes #60.

rename Layer to LayerSurface; layer should refer to overlay, top, bottom
or background
LayerSurface variables are always called layersurface
wlr_layer_surface_v1 variables are always called wlr_layer_surface
@guidocella
Copy link
Contributor Author

guidocella commented Aug 26, 2020

I figured out how to render layer surfaces from wio's output.c and this now works. I tested swaybg, mako and waybar on a single monitor. waybar doesn't reserve an exclusive area though which requires further investigation.

Lets layers with an exclusive area shrink the usable area
wlr_output_layout_get_box internally calls
wlr_output_effective_resolution
@guidocella guidocella marked this pull request as ready for review August 27, 2020 04:13
@guidocella
Copy link
Contributor Author

Exclusive areas are fixed so I marked this a ready for review.

@guidocella guidocella closed this Aug 27, 2020
@guidocella guidocella reopened this Aug 27, 2020
@guidocella guidocella changed the title Layer shell initial attempt Layer shell Sep 1, 2020
@FoundOne
Copy link

FoundOne commented Sep 3, 2020

Great job! The only issue I have, so far, is that the selection on the upper layers doesn't work. That makes programs like slurp not working.

@Stivvo
Copy link
Contributor

Stivvo commented Sep 9, 2020

In fact, while merging layer-shell to my personal branch i noticed that floating windows that I had configured to spawn on the focused monitor (-1) always appeared on the left one. This fixed the issue:

diff --git a/dwl.c b/dwl.c
index 86b0977..df09cf9 100644
--- a/dwl.c
+++ b/dwl.c
@@ -452,9 +452,11 @@ applyrules(Client *c)
                        c->isfloating = r->isfloating;
                        newtags |= r->tags;
                        i = 0;
-                       wl_list_for_each(m, &mons, link)
+                       wl_list_for_each(m, &mons, link) {
+                               mon->m = *wlr_output_layout_get_box(output_layout, mon->wlr_output);
                                if (r->monitor == i++)
                                        mon = m;
+                       }
                }
        }
        setmon(c, mon, newtags);

At this point, setmon() is the only place where m->m is actually used.

I can confirm that wlr_output_layout_get_box() in createmon() returns the wrong results, and I still don't know why. However, it doesn't always have to be recalculated. This also works (even without applying the patch above):

iff --git a/dwl.c b/dwl.c
index 8ef598f..1e20e1c 100644
--- a/dwl.c
+++ b/dwl.c
@@ -552,7 +552,7 @@ arrangelayers(Monitor *m)
        FILE *debugmon = fopen("/tmp/dwl/debugmon", "a");
        FILE *debugUsable = fopen("/tmp/dwl/usable", "a");

-       struct wlr_box usable_area = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+       struct wlr_box usable_area;
        uint32_t layers_above_shell[] = {
                ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY,
                ZWLR_LAYER_SHELL_V1_LAYER_TOP,
@@ -561,6 +561,8 @@ arrangelayers(Monitor *m)
        LayerSurface *layersurface;
        struct wlr_keyboard *kb = wlr_seat_get_keyboard(seat);

+       m->m = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+       usable_area = m->m;

	// Arrange exclusive surfaces from top->bottom

So I think we should still keep m->m to save at least one recalculation.

As for the = operator, it can replace memcpy for this purpose.

In the end, I agree that we don't have to always follow sway.

@guidocella
Copy link
Contributor Author

guidocella commented Sep 10, 2020

In master m->m and m->w are calculated only in arrange(), but I had to move at least m->w because it overwrote usable_area, but I shouldn't have put them in createmon() since x and y are calculated wrong in there for some reason (wish I could have tested on multiple monitors).
You say the second patch makes the first redundant, but that probably relies on a layer surface such as swaybg being mapped before any client / arrangelayers() being called before applyrules(), in fact you may never create a layer surface.
I would like to find a sweet spot after createmon() where we can calculate the output geometry once. How about this?

diff --git a/dwl.c b/dwl.c
index 7bf8e5b..82e6f69 100644
--- a/dwl.c
+++ b/dwl.c
@@ -472,7 +472,7 @@ void
 arrangelayer(Monitor *m, struct wl_list *list, struct wlr_box *usable_area, bool exclusive)
 {
 	LayerSurface *layersurface;
-	struct wlr_box full_area = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+	struct wlr_box full_area = m->m;
 
 	wl_list_for_each(layersurface, list, link) {
 		struct wlr_layer_surface_v1 *wlr_layer_surface = layersurface->layer_surface;
@@ -547,7 +547,7 @@ arrangelayer(Monitor *m, struct wl_list *list, struct wlr_box *usable_area, bool
 void
 arrangelayers(Monitor *m)
 {
-	struct wlr_box usable_area = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+	struct wlr_box usable_area;
 	uint32_t layers_above_shell[] = {
 		ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY,
 		ZWLR_LAYER_SHELL_V1_LAYER_TOP,
@@ -556,6 +556,10 @@ arrangelayers(Monitor *m)
 	LayerSurface *layersurface;
 	struct wlr_keyboard *kb = wlr_seat_get_keyboard(seat);
 
+	if (!m->m.width)
+		m->m = m->w = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+	usable_area = m->m;
+
 	// Arrange exclusive surfaces from top->bottom
 	arrangelayer(m, &m->layers[ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY],
 			&usable_area, true);
@@ -813,10 +817,6 @@ createmon(struct wl_listener *listener, void *data)
 	wlr_output_layout_add_auto(output_layout, wlr_output);
 	sgeom = *wlr_output_layout_get_box(output_layout, NULL);
 
-	/* Get effective monitor geometry to use for window area */
-	m->m = *wlr_output_layout_get_box(output_layout, m->wlr_output);
-	m->w = m->m;
-
 	for (size_t i = 0; i < nlayers; ++i) {
 		wl_list_init(&m->layers[i]);
 	}
@@ -1794,6 +1794,11 @@ setmon(Client *c, Monitor *m, unsigned int newtags)
 		arrange(oldmon);
 	}
 	if (m) {
+		if (!m->m.width)
+			/* Calculating these in createmon() returns wrong results */
+			m->m = m->w = *wlr_output_layout_get_box(output_layout,
+					m->wlr_output);
+
 		/* Make sure window actually overlaps with the monitor */
 		applybounds(c, &m->m);
 		wlr_surface_send_enter(WLR_SURFACE(c), m->wlr_output);

@Stivvo
Copy link
Contributor

Stivvo commented Sep 10, 2020

Your last implementation should be the sweet spot. It is also more efficient than what the master branch does (always recalculates in arrange() which is called way more often) and works well.

Starting dwl in a wrapper script together with waybar, swaybg and other stuff made me forget about the possibility of not using any layer surface...

Then, I tried to plug another monitor (I don't need/have 3 monitors, used a TV just for this test) while dwl was already running and lots of "horrors" appeared. This is due to the if in arrangelayers(), which is nice for efficiency but prevents m->m from being recalculated once a new screen is added. This works if applied on top of your most recent patch:

diff --git a/dwl.c b/dwl.c
index 6a5fc54..2518084 100644
--- a/dwl.c
+++ b/dwl.c
@@ -556,8 +556,7 @@ arrangelayers(Monitor *m)
        LayerSurface *layersurface;
        struct wlr_keyboard *kb = wlr_seat_get_keyboard(seat);

-       if (!m->m.width)
-               m->m = m->w = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+       m->m = m->w = *wlr_output_layout_get_box(output_layout, m->wlr_output);
        usable_area = m->m;

        // Arrange exclusive surfaces from top->bottom

Instead, without the if in setmon() clients take over waybar for the first couple of seconds after they are spawned: the recalculation in arrangelayers() should always have the priority, since layers aren't considered elsewhere.

Ironically wlr_output_layout_get_box() works in createmon() if dwl is already running. However, we don't need it any more since it will cause the same kind of issue: it will display the "horror" for a fraction of a second after plugging the monitor while waiting a further recalculation byarrangelayers().

scrn_2020-09-10_12-19-05

On the picture, only the smaller monitor (the TV) is displaying correctly (It doesn't have waybar because I've explicitly configured it to not be displayed there).

@guidocella
Copy link
Contributor Author

Going from your experience, I believe that with just that patch applybounds(c, &m->m); in setmon() as well as m->w in tile() and monocle() will be wrong if you never create a layer surface and plug in a new monitor.

You say that wlr_output_layout_get_box in createmon() actually works but recalculation is necessary. Does that mean that it's just the geometry of the outputs that were present before that needs to be updated? If so how about calculating the geometries of every monitor in createmon()?

diff --git a/dwl.c b/dwl.c
index 7bf8e5b..b7560b9 100644
--- a/dwl.c
+++ b/dwl.c
@@ -472,7 +472,7 @@ void
 arrangelayer(Monitor *m, struct wl_list *list, struct wlr_box *usable_area, bool exclusive)
 {
 	LayerSurface *layersurface;
-	struct wlr_box full_area = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+	struct wlr_box full_area = m->m;
 
 	wl_list_for_each(layersurface, list, link) {
 		struct wlr_layer_surface_v1 *wlr_layer_surface = layersurface->layer_surface;
@@ -547,7 +547,7 @@ arrangelayer(Monitor *m, struct wl_list *list, struct wlr_box *usable_area, bool
 void
 arrangelayers(Monitor *m)
 {
-	struct wlr_box usable_area = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+	struct wlr_box usable_area = m->m;
 	uint32_t layers_above_shell[] = {
 		ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY,
 		ZWLR_LAYER_SHELL_V1_LAYER_TOP,
@@ -690,6 +690,11 @@ cleanupmon(struct wl_listener *listener, void *data)
 
 	wl_list_remove(&m->destroy.link);
 	free(m);
+
+	wl_list_for_each(m, &mons, link) {
+		m->m = m->w = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+		arrangelayers(m);
+	}
 }
 
 void
@@ -813,13 +818,15 @@ createmon(struct wl_listener *listener, void *data)
 	wlr_output_layout_add_auto(output_layout, wlr_output);
 	sgeom = *wlr_output_layout_get_box(output_layout, NULL);
 
-	/* Get effective monitor geometry to use for window area */
-	m->m = *wlr_output_layout_get_box(output_layout, m->wlr_output);
-	m->w = m->m;
-
 	for (size_t i = 0; i < nlayers; ++i) {
 		wl_list_init(&m->layers[i]);
 	}
+
+	/* Get effective monitor geometry to use for window area */
+	wl_list_for_each(m, &mons, link) {
+		m->m = m->w = *wlr_output_layout_get_box(output_layout, m->wlr_output);
+		arrangelayers(m);
+	}
 }
 
 void

If this doesn't work either, even if we just call wlr_output_layout_get_box over and over we're back to the problem of usable_area being overwritten.

(Also, I noticed I have yet to hande layersurface->output_destroy.notify)

@Stivvo
Copy link
Contributor

Stivvo commented Sep 11, 2020

Your last patch works perfectly. Actually, I just had noticed that wlr_output_layout_get_box() gives correct results in createmon() if the monitor is plugged while dwl is running but I hadn't thought to do what you did.

By the way, my last patch was meant to be applied on top of yours, although it is no longer necessary.

With this minor correction (on top of yours) clients don't move to the left monitor when plugging or unplugging monitors (clients keep the same coordinates but the field below them changes).

diff --git a/dwl.c b/dwl.c
index b7560b9..0d92d35 100644
--- a/dwl.c
+++ b/dwl.c
@@ -694,6 +694,7 @@ cleanupmon(struct wl_listener *listener, void *data)
        wl_list_for_each(m, &mons, link) {
                m->m = m->w = *wlr_output_layout_get_box(output_layout, m->wlr_output);
                arrangelayers(m);
+               arrange(m);
        }
 }

@@ -826,6 +827,7 @@ createmon(struct wl_listener *listener, void *data)
        wl_list_for_each(m, &mons, link) {
                m->m = m->w = *wlr_output_layout_get_box(output_layout, m->wlr_output);
                arrangelayers(m);
+               arrange(m);
        }
 }

Of course, a more complex fix is needed to fix the behavior of floating windows. I'm working on that.

I was going to talk about removing monitor later to avoid confusion. However, unplugging a monitor with a client or a surface on it while dlw is running causes a crash, and the user is back to the tty. I'm looking what can be done about this.

When a monitor is created or removed, the geometries of the old ones
must be updated. This is also more efficient than before since we
calculate the monitor geometries only when creating and destroying
monitors. arrangelayers() is needed to recalculate m->w. arrange() is so
clients don't move to the left monitor when plugging or unplugging
monitors (clients keep the same coordinates but the field below them
changes).
@guidocella
Copy link
Contributor Author

guidocella commented Sep 11, 2020

After taking a better look at sway's handle_output_destroy I think it shouldn't be necessary to copy it since destroylayersurfacenotify already unsets everything in layer surfaces, I think it's meant for when you disable a monitor to re-enable it later which we don't support yet.

@w1ll-i-code
Copy link

I have an issue where, when this patch is applied, video playback will randomly freeze the rendering on the compositor. It still plays back the sound and the keyboard and mouse inputs still work, just the rendering will freeze. Switching to another tty and back will resume the playback.

@guidocella
Copy link
Contributor Author

Is that with mpv? If so can you try updating it? I had this issue before even without layer shell but it got fixed on mpv's side, see #50

@w1ll-i-code
Copy link

Is that with mpv? If so can you try updating it? I had this issue before even without layer shell but it got fixed on mpv's side, see #50

It happens with video playback in firefox and chromium. Tried it out with both on youtube.

@guidocella
Copy link
Contributor Author

It happens with video playback in firefox and chromium. Tried it out with both on youtube.

Videos work for me in Firefox and Chromium and they should have nothing to do with layer shell so I'm not sure how to help you.

@w1ll-i-code
Copy link

Ok, sorry for the inconvenience

@guidocella
Copy link
Contributor Author

By the way, I started a fork since this repo seems abandoned. It comes with layer shell, fullscreen and more protocols (currently 130 commits ahead of here) and will be developed further. You can check it out at https://sr.ht/~guidocella/dwl/

@djpohly
Copy link
Owner

djpohly commented Dec 25, 2020

Haha, I'm here, just having the semester from "heck". Finally on break and can get to reviewing some of these more complex changes.

@djpohly
Copy link
Owner

djpohly commented Dec 25, 2020

Looks great, and thanks for all the work you've been doing in development and answering issues! @guidocella Let me know if you'd be interested in co-maintaining - usually preferable to a fork. 🙂

@djpohly djpohly merged commit 62529e2 into djpohly:master Dec 25, 2020
@guidocella
Copy link
Contributor Author

Looks great, and thanks for all the work you've been doing in development and answering issues! @guidocella Let me know if you'd be interested in co-maintaining - usually preferable to a fork. 🙂

Co-maintaining is good, but does this mean you will disappear again after every holiday, and I won't know whether I can push on with changes for months?

And while you applied many of my changes, I think that losing the git history is not nice since I had motivated every change in the commit messages, especially tricky stuff like focus and the change to output management which was only temporary...

And honestly if possible I would prefer to keep using Sourcehut as it's a free platform meant exactly for projects like this, and closer to how dwm is developed. It lets you do everything without opening the website in a bloated browser or dealing with branches (I went from 21 branches to 2!).

@djpohly
Copy link
Owner

djpohly commented Dec 25, 2020

I am sorry you feel so frustrated, friend. It has been a tremendously stressful year for all of us. The reason I am looking for co-maintainers with commit/push access would be precisely so that development can continue to happen when some of us are busy.

Sourcehut looks cool but plans to discontinue free hosting once the alpha is over. GitHub has a couple of nice suites of command-line tools (the older hub and the newer GitHub CLI), the option to use the browser interface makes the repository accessible to a broader range of contributors, and Git branches are cheap. Additionally, the fellow who runs Sourcehut has shown himself in issue/PR comments to be a bit toxic toward potential contributors, which is something that really saddens me. You always, always speak kindly to contributors, especially volunteers, as I hope I do here. (Yes, I know he is also the person in charge of sway/wlroots. 🙁)

It is super important to me that everyone is credited for their work, and it does hurt to have someone suggest I would ever do otherwise. I never intentionally squash merge unless requested for this very reason, and in this case I pulled directly from the Sourcehut repository to make sure everything was here. I even double-checked git shortlog -s to be absolutely sure! I will be happy to graft in anything which slipped through the cracks; please provide a commit hash of the work that does not appear in the history here and a repository where I can find it.

(Also... peace and joy to you this Christmas/holiday season! 🎄)

@guidocella
Copy link
Contributor Author

guidocella commented Dec 25, 2020

Ok, this will be long and off-topic...
Sourcehut requiring payments is a good thing, because it means you're paying with money instead of your freedom. A paid account is only 20$ at year (I already got one to contribute), and you don't need an account to contribute to projects, but only to push to personal repositories, and they will probably even give you push rights for free if you have issues with paying and contact them. You naturally get a local copy of every discussion via email to keep and browse offline, or can easily download it all from the website, and can transfer it to a self-hosted instance and pay nothing, unlike say, what happened to youtube-dl which had thousands of pull requests and issues deleted. And you can also contribute to Sourcehut itself, unlike GitHub which is a proprietary platform you have no control over.
While GitHub CLI exists, it's a client to a proprietary API that you can't reuse anywhere else and thus learning it locks you in further to GitHub, also it currently can't even show comments to pull requests and issues so it can't replace the website at all.
Sourcehut also has a web interface for normies including the ability to send patches, while still letting you enjoy the 100% open source, customizable and decentralized email workflow. And sending a patch via email is really simpler than the fork and branch mess you have to do on GitHub, it may seem more accessible only if you're used it; I definitely had trouble learning it in the beginning. I also question sacrificing power to make more accessible to contributors the port of a program for expert users which literally has
"Because dwm is customized through editing its source code, it's pointless to make binary packages of it. This keeps its userbase small and elitist. No novices asking stupid questions."
in its web page. :)
I don't see how Drew Devault being a bit toxic (which I don't see anyway) is an issue, but all the pure evil shit Microsoft has done throughout the years and still does (e.g. https://sneak.berlin/20200307/the-case-against-microsoft-and-github/ https://stallman.org/microsoft.html) is not? And if that's an issue we shouldn't take code from a group infamous for https://chaos.social/@raichoo/101880564196043164
Anyway, it feels weird to me to have the port a suckless program developed on a proprietary platform diametrically opposite to its principles of putting users in control and avoiding bloated tools (though probably even Sourcehut is bloated for them :)).

I do not care about credits, I was talking about letting people understand the purpose behind lines of code from git blames and logs without having to ask me. But whatever.

Anyway, my point is, if we're going to actively collaborate I will take one for the team, but if you're only going to show up during holidays, while I'm grateful to you for creaing the project and you surely more know about C than me, I would rather continue working on Sourcehut without worrying about what I can change and with my style changes (I hate that idiotic error=declaration-after-statement). This worries me especially since it seems that you didn't even properly look at the code you applied, like: commenting on the adaptive sync issue but leaving it opened after applying the line that enables it, applying the toggle layout immediately line but leaving the pull request open, deleting the .github directory, mentioning damage tracking as a goal twice, mentioning a 2000 SLOC limit which is clearly impossibile with the feature goals listed right below.

I do not feel frustrated, but more confused about what to do, especially with you coming back right when I'm a bit burnt out from working on this for days (I had even tediously rebased every patch, which I couldn't even do here since they're outside of the project's source control), and when I'm having issues with the USB stick with my OS on it (you'd have to ask my boss about that) deciding to become read-only yesterday, the day stores to buy a replacement SSD closed (side note: it's surprising how much you can get done with your OS on a read-only filesystem).

Merry Christmas to all :)

@djpohly
Copy link
Owner

djpohly commented Dec 27, 2020

My hope for dwl is to create something that folks who appreciate the suckless approach to software would be willing to try - hence adhering to dwm style guidelines, aiming not to exceed dwm significantly in size, and being deliberate about officially adopting features which make the code significantly longer or more complex. This is based on threads I had read on [dev] in which people discussed what had kept them from trying out Wayland.

However, I think you found a key difference with the website quote: the one aspect of some parts of the suckless community (not all!) that I don't want to emulate is elitism. Suckless-style projects have so much to offer newcomers and learners. Showing disdain for "normies" and "novices" and using labels like "stupid" and "idiotic" may be condoned on the suckless MLs, but I'll fight to keep attitudes like that from taking root here.

Since it was mentioned again, let me reiterate that if there is any commit that was somehow merged here without its author, commit message, or other metadata, please provide

  1. the commit hash, and
  2. a repository where I can find the correct information

so that I can make it right. I'll leave some time for that before closing this now-off-topic thread.

@guidocella
Copy link
Contributor Author

I did not mean to convey disdain by saying "normies", that must have come from me using English incorrectly; I was just trying to say that Sourcehut also lets you work from the web if you prefer.

I was referring to all the commits you squashed into 2064275, but again I don't personally care, I just thought that some commit messages could have been useful to understand some changes; maybe no one would bother to read them anyway.

Anyway, if you're taking over maintenance again I will gladly step down. Good luck with maintaining this, and you can go ahead and close this. :)

@djpohly
Copy link
Owner

djpohly commented Dec 27, 2020

Ah, thanks for clarifying. 2064275 is not a squash but a merge commit, which in Git preserves history. One of its parents is 3695ac6, and a git log on either 2064275 or 3695ac6 includes the entire history of the merged branch.

Repository owner locked as off-topic and limited conversation to collaborators Dec 27, 2020
@guidocella guidocella deleted the layer-shell branch December 31, 2020 16:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is anyone working on layer shell? [question] Wallpaper support.
5 participants