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

Port to wlroots 0.18.x #2452

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Port to wlroots 0.18.x #2452

wants to merge 22 commits into from

Conversation

soreau
Copy link
Member

@soreau soreau commented Aug 26, 2024

Gotchas:

wl_drm was dropped, so clients must update. A notable one is libva, which stopped working with wlroots 0.18 until upgrading libva.

wlr_input_inhibit was dropped and as of now, there is no replacement.

Possibilities:

linux-drm-syncobj-v1

@mark-herbert42
Copy link

Compiled 0.9 with this PR - works ok so far. Seems like the new wlroots is better in terms of battery life at least for firefox-youtube.

@soreau
Copy link
Member Author

soreau commented Aug 27, 2024

Compiled 0.9 with this PR - works ok so far. Seems like the new wlroots is better in terms of battery life at least for firefox-youtube.

Thanks for testing!

@soreau soreau force-pushed the track-wlroots branch 28 times, most recently from 0e5e05e to c9c1798 Compare September 1, 2024 10:35
@killown
Copy link
Contributor

killown commented Sep 3, 2024

@killown How do you know this crash is related to this branch? It would also be helpful if you can explain how to reproduce the crash without the fuzz test.

because i am using your branch in the pkgbuild... not reproducible, this is how me and ammen99 did in the past, he just had the crash reports with no clue, it's up to you to fix it or not, but ammen99 could consider this a really issue before merging.

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4345, I believe we need to change all places which use xdg_surface.destroy events to xdg_popup/xdg_toplevel.destroy.

Otherwise LGTM, thanks for the patience!

LOGD("Changed adaptive sync on output: ", handle->name, " to ", current_state.vrr);
} else
{
LOGE("Failed to change adaptive sync on output: ", handle->name);
wlr_output_rollback(handle);
// wlr_output_rollback(handle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should here revert &state to the previous vrr state I think, otherwise, &state gets permanently 'broken' afaict

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see this later commit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at this and what I think is best, is to call wlr_output_state_finish() and wlr_output_state_init() after each wlr_output_state_commit(), so I pushed a commit to do that. This way, we should never have to worry about rolling back states or potentially stale state data.

{
wlr_output_commit(handle);
wlr_output_commit_state(handle, &state);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, we ought to reset in the other case .. In all honestly previously we probably had to do a rollback as well, but we didn't (which might have been a compositor bug that nobody happened to complain about ...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted, will fix.

d["type"] = wlr_input_device_type_to_string(device->get_wlr_handle()->type);
d["id"] = (intptr_t)device->get_wlr_handle();
d["name"] = nonull(device->get_wlr_handle()->name);
d["type"] = wlr_input_device_type_to_string(device->get_wlr_handle()->type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no way to get vendor/product? Maybe by asking libinput directly for libinput devices, and just setting 'unknown' or 'nil' or whatever is appropriate for others. I would like to avoid any breaking changes to the IPC if possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the 0.18 release notes:

"..drop wlr_input_device.{vendor,product}, these aren't super useful without the bus type. Compositors can still obtain this information from the libinput device if they want to."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could use some help finding the call(s) to get the vendor/product id from libinput.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried libinput_device_get_id_{vendor|product}(wlr_libinput_get_device_handle(device->get_wlr_handle())); but it always crashes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devices are not necessarily libinput devices (especially in the wayland backend, you won't find a libinput device). So this needs to be in a check, and in the other cases we can set 'unknown' or -1 as a value.

src/view/xdg-shell.cpp Outdated Show resolved Hide resolved
src/view/xwayland.cpp Show resolved Hide resolved
@soreau
Copy link
Member Author

soreau commented Sep 28, 2024

Looking at https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4345, I believe we need to change all places which use xdg_surface.destroy events to xdg_popup/xdg_toplevel.destroy.

I thought I caught them all but apparently xdg-toplevel is still using destroy events from the base xdg_surface (see last link). xdg_popups from master vs. track-wlroots branch. Similarly for xdg_toplevel, master and track-wlroots

EDIT: Pushed a fix for this.

This leads to a crash on wlroots 0.18, as wlroots handles this already.
xdg wayland surfaces were not committing because compositors are expected
to send a configure when wlr_xdg_surface.initial_commit is set.
There is nothing needed to do here when using wlr_output_state.
Without edge, latest only has 1.22 until possibly Nov '24, when alpine image
3.21 is released.
If a subsurface is created and then becomes smaller without repositioning,
the old larger size was never damaged, leaving artifacts on the output.
This fixes the build.
bob-beck pushed a commit to openbsd/ports that referenced this pull request Nov 7, 2024
update to the latest git commit from the 'port to wlroots 0.18.x'
upstream branch, cf WayfireWM/wayfire#2452
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants