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

Update for wlroots 0.17 #626

Merged
merged 21 commits into from
Nov 27, 2023
Merged

Update for wlroots 0.17 #626

merged 21 commits into from
Nov 27, 2023

Conversation

jlindgren90
Copy link
Contributor

This is a set of changes required to build with wlroots master / 0.17.0-dev (labwc master currently uses wlroots 0.16.x).

Perhaps it will be useful to other wanting to use wlroots-git. I plan to update/rebase it as further breaking changes are made on the wlroots side.

@johanmalm
Copy link
Collaborator

Great. Thank you.

@Consolatis
Copy link
Member

Consolatis commented Nov 17, 2022

I think the current plan is to keep developing labwc based on wlroots 0.16.x for a while but there were thoughts about having an additional branch which contains all the labwc changes but instead of 0.16.x follows wlroots master.

If you intend to keep doing this it would be great if you could use single commits to track upstream refactorings / API changes (including the wlroots commit hash that requires changes in the labwc commit message).
That way its easier to merge / rebase current work in labwc into that 2nd branch or the other way round.

I am not sure what the best way would be, if it would be a personal branch or PR I would just keep rebasing the wlroots 0.17.x "chase" commits on top of labwc master but changing history for a public branch will cause some annoyances for users. Merging on the other side kind of pollutes the history. Opinions?

Edit:
The PR misses changing subprojects/wlroots.wrap to track master / a specific commit instead of 0.16.0, then the CI should be happy again.

@droc12345
Copy link
Contributor

This is of interest, to me. I do like to follow the latest from wlroots git.
I did fall back to 0.6.0 when it labwc git didn't compile against latest wlroots, but I didn't chase it down.

@Consolatis

This comment was marked as outdated.

src/output.c Outdated Show resolved Hide resolved
@Consolatis
Copy link
Member

Consolatis commented Jan 24, 2023

I've split this PR into multiple single commits (and addressed the output_layout feedback above).
I could either push them to a shared labwc/wlroots_0.17 (wlroots_master ?) branch so multiple people can do the rebasing or I could force-push into this PR. Thoughts @johanmalm @jlindgren90 ?

Edit:
The Debian Testing CI should be back working in around 5 days once the latest xwayland release trickles through. They will now also include xwayland.pc in the package which is required by wlroots >= 0.17.

@johanmalm
Copy link
Collaborator

johanmalm commented Jan 24, 2023

Separate commits is better in my view.

I vote for force-pushing on John’s branch so that we’ve got it all in one place, but let’s check with John first before hacking on his work 😄

@jlindgren90
Copy link
Contributor Author

Force-pushing this branch is fine

@Consolatis Consolatis force-pushed the wlroots-0.17 branch 2 times, most recently from 523b2ee to 7b30106 Compare January 30, 2023 04:55
@Consolatis
Copy link
Member

Consolatis commented Jan 30, 2023

Rebased on latest master (e.g. 0.6.1) and restored resizing for nested instances.

The fix needs to be debugged / redefined:

  • using the wayland backend, windows within the nested instance now suddenly flicker while resizing the virtual output window
  • using the x11 backend (WLR_BACKENDS=x11), sometimes there is a feedback loop which causes endless resizing which can only be stopped by A-F4 to close the virtual output window

The X11 backend issue may point to an underlying issue within labwc itself, with the way we reconfigure xwayland windows.
Both issues also show up when using tinywl from the current wlroots (but not when compiling from the 0.16 branch), running nested on top of 0.6.1@0.16.x labwc.

@Consolatis
Copy link
Member

Consolatis commented Feb 15, 2023

Regarding the Debian CI build error, I assume libxcb-ewmh-dev is missing from the apt install command in .github/workflows/build.yml. Didn't verify.

@jlindgren90
Copy link
Contributor Author

Regarding the Debian CI build error, I assume libxcb-ewmh-dev is missing from the apt install command in .github/workflows/build.yml. Didn't verify.

Thanks for the pointer, I will try adding libxcb-ewmh-dev.

@jlindgren90
Copy link
Contributor Author

Looks like a (new?) dependency is missing on FreeBSD:

wlroots| Build-time dependency hwdata found: NO (tried pkgconfig and cmake)
wlroots| Message: Required for the DRM backend.

@Consolatis
Copy link
Member

Consolatis commented Feb 21, 2023

Strange. It is part of the CI build in master for Debian Testing (and Void apparently, judging from 52488e1) but not for any of the other systems. I'd say lets just add it to the CI for FreeBSD in master and fix it here via the next rebase.

Edit:
Seems its called hwdata for FreeBSD (found via repology).

Edit 2:

Chases: 711a1a3ed42150fdbc716e80719d482006075f69
xdg-shell: convert to try_from

Chases: f9bd416d4156942ce3951a6c5cf9f81a3cf4a3dd
layer-shell-v1: convert to try_from

Chases: fbf5982e3838ee28b5345e98832f6956c402b225
xwayland/xwm: introduce wlr_xwayland_surface_try_from_wlr_surface()
Chases: 7b32c25a4fbdcde4197a06c8e0ff638c54753bd7
wlr_scene: Rename wlr_scene_surface_from_buffer
Chases: 0bb574239d3b164596677bf4cec371ff0671dc4f
compositor: pass version in wlr_compositor_create
@Consolatis
Copy link
Member

Consolatis commented Nov 27, 2023

  • updated 2b9b9bf wlroots.wrap hash by one commit because the previous one introduced a label can only be part of a statement error which was fixed one commit later
  • updated ccb9691 and removed ba00073
  • added 3b0a6be and 13f9ce9 as workaround to keep this branch compilable between wlroots introducing the bug and fixing it
  • combined b0e23e0 and f12a369 and moved it a bit further down as wlroots only got some helpers later in the dev cycle. This means that between the changes that broke nested resizing and it coming back there is no nested labwc resizing support. This could be changed by having a more basic implementation around first and then have a proper implementation later in the patchset. So basically what we had before. Up for debate if its required to have a working resize for nested labwc within the patchset or if its fine that the patchset temporarily breaks the nested resize.

I also ran everything through

git log 0.6.6.. | sed -n '/^commit / s/^commit // p' | tac | while read commit; do
	git checkout $commit && (cd subprojects/wlroots; git checkout $(sed -n '/^revision/ s/^.*= // p' < ../wlroots.wrap)) && nice meson compile -C build_017 || (echo "Failed: $commit" >> failed.log);
done;

and all the commits now compile fine. There must be some better way to testcompile every commit + changing the wlroots one but this was faster than looking up how to do it properly.

I also have a local branch with the state of this branch before my changes, just in case (note to future self: backup/626).

Consolatis and others added 12 commits November 27, 2023 03:11
This is a new dependency of wlroots.
Need to handle new unified mapping, where mapping is attached to the
wlr_surface objects instead of their parents. Also, most of them require
a new associate event for xsurface objects, their surface member will be
NULL before this event is received.

Refactored by jlindgren:
- add struct mappable
- unify map/unmap logic
Chases 70c1a5724814d2f786f7d3a0e55a05f11af14029
gamma-control-v1: introduce set_gamma event
Chases: 18bafbfc57039e16d1dabd78b882b3d6477f76b5
xcursor-manager: drop wlr_xcursor_manager_set_cursor_image()
Chases: 214df8eda07d18b032abfcf525c8344e077c0c7e
scene_output: optionally record and report timings
Chases: 657ca2205ff4d5f70cf294d9b5720acf2eaf76b4
wlr_gamma_control: add missing forward declarations
Chases: bdc34401ba8e4a59b3464c17fa5acf43ca417e57
xdg-decoration: store an xdg_toplevel instead of xdg_surface
Chases: 756ecf8ee9f1e75bc7b8297dc84f97c7d699174b
backend/wayland: use request_state when toplevel is resized

Chases: 3ef68a484243555b020200c6f95246d994932c3f
backend/x11: use request_state when window is resized

Ref: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/2693

We now delay requested resolution changes by the backend until
the next frame event which causes us to render the new content
on the already enlarged buffer. Before this change, an empty
(black) buffer would have been shown instead before the next
frame event caused a new render of the actual contents.

Keep commiting the new state and then scheduling a frame event
would not help as due to the commit call it would still show an
empty buffer in the meantime.

Just modifying wlr_output->pending wouldn't work either because
wlr_scene_output_commit() *completely* ignores it (and it will
be removed in future wlroots commits). For this reason we move
to wlr_scene_output_build_state() directly because it allows us
to supply the current wlr_output->pending state and thus apply
any resolution change in lockstep with new rendering. Result:
No more flickering in the wayland backend and resizing is again
smooth as butter.

This prevents constant flicker while resizing
when running nested via the wayland backend.

For the X11 backend (can be tested via `WLR_BACKENDS=x11 labwc`),
it is still rather janky but at least doesn't cause endless self-
resizing anymore.
Chases: f5917f0247600b65edec1735234d00de57d577a8
scene_output_layout: make output adding explicit
Chases: a289f812d62059d5aac92cbd374dcb7b03bb08a6
drop KDE idle protocol support
Chases: 5fb0007e0249388792f3772c30bfabf8d551dec0
output_event_commit: Remove committed and buffer
@jlindgren90 jlindgren90 changed the title wip: Update for wlroots 0.17.0-dev Update for wlroots 0.17 Nov 27, 2023
@jlindgren90 jlindgren90 marked this pull request as ready for review November 27, 2023 02:17
@jlindgren90
Copy link
Contributor Author

Thanks both. Everything looks good to me. Aside from the nested resize (which I haven't tested) I've been running with all these changes for a little while now and I'm pretty confident there aren't any serious regressions, so I vote to go ahead with the merge. 👍

@Consolatis
Copy link
Member

Consolatis commented Nov 27, 2023

Well, thanks to you as well, you were the one starting this whole PR in the first place :)

Although I haven't really tested anything other than short nested runs I am fine with merging this as well. I expect some stuff to break, for example I think there is an issue with gamma not working anymore due to a wlroots API change and there might be more like this. We might also want to look through the wlroots list with breaking changes for 0.17 to verify that we didn't miss anything hard to detect like changed event *data pointers or similar.

But as we are at the start of a dev cycle this seems pretty minor to me, we should wait a bit with a release though until those things are worked out. If we go for the merge we should likely also create a 0.6 branch for future backports / 0.6.x fixes.

@johanmalm johanmalm merged commit aab8061 into labwc:master Nov 27, 2023
5 checks passed
@johanmalm
Copy link
Collaborator

Thank you both 😄
Feel like a big step.
@Consolatis Thanks for persevering last night. Would have taken me ages.

@johanmalm
Copy link
Collaborator

@Narrat - note: now on wlroots 0.17

@jlindgren90 jlindgren90 deleted the wlroots-0.17 branch November 27, 2023 21:03
@Narrat
Copy link
Contributor

Narrat commented Nov 27, 2023

@Narrat - note: now on wlroots 0.17

Great! Adjusted the deps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants