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

Use the wlroots scene-graph API #197

Merged
merged 17 commits into from
Dec 21, 2021
Merged

Conversation

emersion
Copy link
Contributor

@emersion emersion commented Aug 9, 2021

This is a work-in-progress PR to make use of the upcoming wlroots scene-graph API.

  • Fix multi-output coordinates
  • Proper allocation error handling
  • Consider adding a wlr_scene API to replace the various surface_at functions

References: swaywm/wlroots#1966

@Hjdskes
Copy link
Collaborator

Hjdskes commented Aug 9, 2021

This is very nice! Let me know if you need any help, definitely excited about having this in Cage :)

@emersion
Copy link
Contributor Author

This is now ready for review: all of the wlroots PRs have been merged!

@Hjdskes
Copy link
Collaborator

Hjdskes commented Sep 22, 2021

This is fantastic! I'll try to review this as soon as possible, but as my wedding is coming up I can't promise anything.

@emersion
Copy link
Contributor Author

my wedding is coming up

Wow, congrats! 💐

No worries at all, in any case I think you'll want to wait for a new wlroots release before merging this?

@emersion emersion marked this pull request as ready for review September 22, 2021 09:45
@jbeich
Copy link
Contributor

jbeich commented Sep 23, 2021

1301a43 breaks -Dxwayland=true build:

xwayland.c:111:2: warning: implicit declaration of function 'view_damage_part' is invalid in C99 [-Wimplicit-function-declaration]
        view_damage_part(view);
        ^
xwayland.c:120:2: warning: implicit declaration of function 'view_damage_whole' is invalid in C99 [-Wimplicit-function-declaration]
        view_damage_whole(view);
        ^
xwayland.c:143:2: warning: implicit declaration of function 'view_damage_whole' is invalid in C99 [-Wimplicit-function-declaration]
        view_damage_whole(view);
        ^
ld: error: undefined symbol: view_damage_whole
>>> referenced by xwayland.c:143 (xwayland.c:143)
>>>               cage.p/xwayland.c.o:(handle_xwayland_surface_map)
>>> referenced by xwayland.c:120 (xwayland.c:120)
>>>               cage.p/xwayland.c.o:(handle_xwayland_surface_unmap)

ld: error: undefined symbol: view_damage_part
>>> referenced by xwayland.c:111 (xwayland.c:111)
>>>               cage.p/xwayland.c.o:(handle_xwayland_surface_commit)
cc: error: linker command failed with exit code 1 (use -v to see invocation)

@emersion
Copy link
Contributor Author

Thanks, fixed!

@dimkr
Copy link
Contributor

dimkr commented Oct 18, 2021

@emersion I'm working on a Cage-based compositor, and the scene graph API does solve some issues with Cage - for example, visual artifacts when moving between GTK+ 3 menu items (probably a damage tracking issue of some sort).

However, it crashes (ignore view.c line numbers, it's slightly modified) with wlroots 1b65a80:

==9922== Invalid write of size 8
==9922==    at 0x4869F0B: wl_list_remove (in /usr/lib/x86_64-linux-gnu/libwayland-server.so.0.1.0)
==9922==    by 0x48DA2BE: destroy_xdg_surface (wlr_xdg_surface.c:471)
==9922==    by 0x48D9BF1: xdg_surface_handle_resource_destroy (wlr_xdg_surface.c:282)
==9922==    by 0x4863DBE: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-server.so.0.1.0)
==9922==    by 0x4869EB0: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-server.so.0.1.0)
==9922==    by 0x486A38F: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-server.so.0.1.0)
==9922==    by 0x4864848: wl_client_destroy (in /usr/lib/x86_64-linux-gnu/libwayland-server.so.0.1.0)
==9922==    by 0x48650B5: wl_display_flush_clients (in /usr/lib/x86_64-linux-gnu/libwayland-server.so.0.1.0)
==9922==    by 0x48650F7: wl_display_run (in /usr/lib/x86_64-linux-gnu/libwayland-server.so.0.1.0)
==9922==    by 0x10DDB0: main (cage.c:497)
==9922==  Address 0xd00d028 is 120 bytes inside a block of size 144 free'd
==9922==    at 0x48399AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==9922==    by 0x48CAEA6: wlr_scene_node_destroy (wlr_scene.c:105)
==9922==    by 0x48CAD3D: scene_node_finish (wlr_scene.c:71)
==9922==    by 0x48CADAC: wlr_scene_node_destroy (wlr_scene.c:85)
==9922==    by 0x1118FD: view_unmap (view.c:198)
==9922==    by 0x11249C: handle_xdg_shell_surface_unmap (xdg_shell.c:250)
==9922==    by 0x490CC00: wlr_signal_emit_safe (signal.c:29)
==9922==    by 0x48D9384: unmap_xdg_surface (wlr_xdg_surface.c:40)
==9922==    by 0x48DA114: reset_xdg_surface (wlr_xdg_surface.c:426)
==9922==    by 0x48DA262: destroy_xdg_surface (wlr_xdg_surface.c:464)
==9922==    by 0x48D9BF1: xdg_surface_handle_resource_destroy (wlr_xdg_surface.c:282)
==9922==    by 0x4863DBE: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-server.so.0.1.0)

I'm still investigating this, but maybe you'll see something I don't.

@emersion
Copy link
Contributor Author

Does this help? swaywm/wlroots#3268

@dimkr
Copy link
Contributor

dimkr commented Oct 18, 2021

Does this help? swaywm/wlroots#3268

So far - nope! Doesn't reproduce.

@emersion
Copy link
Contributor Author

Do you mean that this patch fixes the issue? Or do you mean that it doesn't?

@dimkr
Copy link
Contributor

dimkr commented Oct 18, 2021

So far I don't see this - tried -fsanitize=address too, and looks like this particular problem doesn't reproduce with the patch. However, I'm not 100% sure how I triggered this condition.

@emersion
Copy link
Contributor Author

Dropped a few more things in favor of scene-graph helpers: direct scan-out, per-output surface iterators, and xdg_popup handling.

@Hjdskes
Copy link
Collaborator

Hjdskes commented Oct 27, 2021

This is looking so good!

@dimkr
Copy link
Contributor

dimkr commented Nov 7, 2021

@Hjdskes Will this PR be part of the 0.15.0 release of Cage?

@Hjdskes
Copy link
Collaborator

Hjdskes commented Nov 7, 2021

Yes! It is my intention to include this in the next release of Cage that will track wlroots 0.15.0 :)

@jbeich
Copy link
Contributor

jbeich commented Nov 19, 2021

cb7d6a5 is no longer enough after wlroots@fdf3169:

cage.c:308:13: warning: implicit declaration of function 'wlr_backend_get_renderer' is invalid in C99 [-Wimplicit-function-declaration]
        renderer = wlr_backend_get_renderer(server.backend);
                   ^
cage.c:308:11: warning: incompatible integer to pointer conversion assigning to 'struct wlr_renderer *' from 'int' [-Wint-conversion]
        renderer = wlr_backend_get_renderer(server.backend);
                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
ld: error: undefined symbol: wlr_backend_get_renderer
>>> referenced by cage.c:308 (cage.c:308)
>>>               cage.p/cage.c.o:(main)
cc: error: linker command failed with exit code 1 (use -v to see invocation)

@emersion
Copy link
Contributor Author

@Hjdskes I cannot reproduce. Can you share full debug logs?

@iamdavidcz I've opened this wlroots MR to avoid the confusion: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/3407

- Update wlr_box includes to util/box.h: the wlroots header has
  been moved upstream.
- Subsurface fields have been moved
- Create renderer and allocator, stop using wlr_backend_get_renderer
- Initalize output rendering
@iamdavidcz
Copy link

@emersion that's gonna work. What was the rationale behind making libinput optional in wlroots 0.15.0?

@emersion
Copy link
Contributor Author

What was the rationale behind making libinput optional in wlroots 0.15.0?

Some compositors are not designed to be run with DRM+libinput, like virtual reality compositors or nested compositors.

seat.c Show resolved Hide resolved
view.c Show resolved Hide resolved
@Hjdskes
Copy link
Collaborator

Hjdskes commented Dec 19, 2021

Indeed forcing the libinput backend makes the cursor work & input focus work :) Here's the debug log you requested @emersion: https://gist.github.com/Hjdskes/a1ca59f3637af865dc04e9b091d693f7. This is just a quick Cage session from TTY (./build/cage epiphany).

@iamdavidcz
Copy link

@Hjdskes any chance that this will be merged before Christmas? Can I help with testing somehow?

Copy link
Collaborator

@Hjdskes Hjdskes left a comment

Choose a reason for hiding this comment

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

Thanks for this @emersion!

@Hjdskes Hjdskes merged commit dfba2d8 into cage-kiosk:master Dec 21, 2021
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.

None yet

5 participants