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

Wayland support #425

Draft
wants to merge 31 commits into
base: master
Choose a base branch
from
Draft

Wayland support #425

wants to merge 31 commits into from

Conversation

fleischie
Copy link

@fleischie fleischie commented Nov 12, 2020

Hello,

after some effort to understand the (basic) in-n-outs of how to setup a wayland-client I embarked on adding support for wayland for the sokol_app.h header lib. I just started implementing this, but for transparency reasons I opted to make this PR public as soon as possible (i.e. after the first commit).

Supersedes #173

✌️


Todo:

  • Setup barebone wayland-client
  • Hook up wayland input handling to sokol
    • Keyboard
    • Mouse
    • Touch
  • Add support for app events (e.g. SUSPENDED, RESUMED, etc.)
    • FOCUSED/UNFOCUSED - in accordance with gtk and qt this is done primarily via keyboard focus
  • Add support for high-dpi displays (as was already done in other PR)
  • Move sapp_frame() call from wayland's frame-done-cb unconditionally to main loop
  • Add support for toggling fullscreen (if possible with wayland)
  • Add option to disable the pointer rendering
  • Send UPDATE_CURSOR event (need to clarify intent of this flag) <- No priority for now.
  • Add option to lock mouse
  • Check if wayland natively supports setting the windowed-state <- Not as far as I can tell
  • Check if ICONIFIED/RESTORED can be supported <- Not as far as I can tell
  • Add support for clipboard
  • Add support for drag and drop
  • Add support for primary selection (as made popular by X11, via protocol extension)
  • Support rendering features (and research what they mean)
    • GLES2
    • GLES3
    • Swap Interval
    • MSAA
  • decide on used path (X11/WL) in runtime rather than compile time
  • Extend this list with sokol capabilities to port to wayland
  • Find solution for necessary protocol extensions:
    • XDG-Shell
    • Pointer Constraints
    • Relative Pointer
    • Primary Selection
  • Add server-side decorations.
  • Add support for setting an app icon (not possible as far as I can tell)
  • Create wayland demo for sokol-samples
  • Make style consistent with sokol styleguide
  • Add documentation for more obscure wayland-implementationdetails
  • Add wayland-specific github check to actually test this PR 😅

Note:
I will probably work this list from top to bottom (and thus re-arrange the list according to my personal priorities). Please let me know, if you have any comments/feedback/etc regarding this PR in general, the prioritized Todo-list, or regarding testing a specific feature. Especially as I am mainly using Sway, which being a tiling-manager may not be the most versatile environment to develop wayland-clients against (re: window decorations, auto-floating windows, etc.) But this can be arranged if a) desired, or b) necessary (after feedback).


Edits

Edit 2022-12-21:

  • I rebased the branch onto the latest master.

Edit 2022-07-18:

  • I removed the app_id setting again: It turns out it requires a more recent version of libwayland-dev than what the Ubuntu distro of the CI provides. (I am using Arch locally and it has v1.21, Ubuntu seems to have v1.18). As this feature was a relatively small patch, this shouldn't be too problematic currently.
  • Backported cursor-selection for wayland after it landed in the main branch.

Edit 2022-07-01:

  • Cleaned up some of the wayland-specific aux functions,
  • added linux-specific function to set the app_id, which is used by the compositor to figure out the relevant .desktop-file and group apps, etc.,
  • checked off the GH Actions/CI-item from the todo-list as they should be covered by the existing checks now.

Edit 2022-06-17:

  • After some helpful advice I am going to replace SOKOL_CALLOC/SOKOL_FREE with the updated _sapp_malloc/_sapp_free,
  • the wl_output protocol can be a number lower than is documented, because the event it says it uses, isn't actually used,
  • added two more action items (re: creating a sokol-sample file),
  • fix item for app icon setting, which is not possible on wayland (at least from what I can tell).

Edit 2021-09-23:

  • Add support for "Focused"/"Unfocused" events,
  • fix quit request behavior to allow users to cancel a requested quit, and
  • fix broken key event behavior to register alpha keys even when modifier keys are pressed, as well as emulate key repeats which is not natively done via the compositor.

Edit 2021-09-02:

  • Rebased current state onto main branch
  • added tasks regarding "Focused"/"Unfocused" events that are new (to me at least).

Edit 2021-03-07:

  • Fix compiler warnings,
  • mark SSD as "WONTFIX" for now in Todo-list.

Edit 2021-02-23:

  • Add option to decide at runtime what display protocol to use,
  • add "Serverside-Decorations" task to Todo-list.

Edit 2021-02-21:

  • Research open graphic setup options,
  • fix GLES setup (in respect to gl_force_gles2 request) as well as query egl context for ultimately used api version,
  • fix call to setup user-requested swap interval value,
  • appropriately update Todo-list.

Edit 2021-02-14:

  • Inline manually curated wayland protocol extensions, and
  • appropriately update Todo-list.

Edit 2021-02-12:

  • Drop primary selection from backlog.

Edit 2021-02-11:

  • Add support for drag and drop (finally)

Edit 2020-12-20:

  • Add basic clipboard support
  • cross off appropriate item from the TODO-list, and
  • add new item to TODO-list regarding primary-selection, which needs to be implemented via a protocol extension.

Edit 2020-12-03:

  • Investigate open points I had on the list:
    • There is no native way to "window", "iconify", or "restore" the app using wayland,
    • if there is, I don't have access for them using sway,
    • if there is, there will probably be a protocol extension from another major wayland compositor implementation (Qt/KDE/Gnome/wlroots/etc.),
    • all of these events are the responsibility of the compositor, and should be implemented by utilizing the surface-based frame-callback, so that the compositor can control when rendering is happening.

Edit 2020-12-02:

  • Implement pointer-lock,
  • cross off UPDATE_CURSOR todo. (I'll add a comment to the PR discussion about my thoughts.)

Edit 2020-12-01:

  • Implement cursor display toggle, as well as proper initial cursor display
  • Implement proper relative pointer motion support via the appropriate wayland protocol extension
  • Update feature matrix to reflect wayland platform
  • Extend TODO list in github PR description to mirror the current state of the feature/platform matrix

Edit 2020-11-24:

  • Add fullscreen toggle support
  • Move frame callback to main loop

Edit 2020-11-22:

  • Add high-dpi support as far as I could without having the proper hardware to test 🤷‍♂️
  • Update Todo-Items:
    • Add X11/Wayland differentiation reminder
    • Add reminder to fix frame-cb issue
    • Move protocol-extension task further down

Edit 2020-11-19:

  • Add pointer support
  • Add touch support
  • Add Todo-Items:
    • pointer-constraints protocol extension reference
    • drag-n-drop support
    • documentation reference

Edit 2020-11-13:

  • Fixed some miniscule issues with the bare-bone setup of wayland
    • fix naming of static functions,
    • remove unnecessary header
  • Add keyboard support
  • Add Todo-Items regarding app events, and github check
  • Refactor code slightly to send app events that can already be determined (e.g. suspended/resumed, resized, quit requested, etc.)

@floooh
Copy link
Owner

floooh commented Nov 13, 2020

Good stuff, let me know when you need some additional testing and code reviews :)

@fleischie fleischie force-pushed the feature/wayland branch 2 times, most recently from ba05b57 to b751909 Compare November 13, 2020 23:52
@fleischie
Copy link
Author

fleischie commented Nov 14, 2020

Hey @floooh

tl;dr: Operate on wayland input-events on a per-frame basis, or pipe them through sokol directly?

I am currently working on combining wayland's input events (pointer, and touch) into sokol, and I am contemplating how to best approach this. Wayland has a mechanism called "pointer-frames", where multiple input events are batched together. Think of it as the compositor essentially accumulating individual events (e.g. mouse-move, button release, scroll-wheel, etc.) before comitting them as packages to the client. More or less. Similar to the option of comitting rendered frames to the compositor unchecked, or alternatively using the frame-callback to only render and eglSwapBuffer on demand.

Theoretically I don't need to adhere to this pointer-frame mechanism, but it of course is the "wayland-way"™. I could also blindly pipe the events from wayland through sokol into the user event-cb.

Good introduction for wayland pointer frames:

My questions for how I should proceed (also other questions regarding input-handling):

  • What would be the preferred solution? (I personally would use the pointer-frame, and will first draft an implementation with it. We can still decide against it afterwards.)
  • This input-frame mechanism is also used for touch-events and thus a similar decision is to be made for these.
  • As far as I understand the events using a touchpad are handled as touch-events and not pointer-events, will this be a problem? (I don't know how other systems handle this)
  • Wayland additionally allows to handle touch-events: touch-orientation, and touch-shape to approximate oval touch areas. Is this something that should be supported by sokol. I tried to check how other branches handle this, and couldn't find a similar behavior. But I could implement it (given the compositor is capable of these events, which I cannot tell, but maybe on some wayland-driven handheld.)

I hope these questions aren't too complicatedly formulated. ✌️


Edit: Whoops, didn't notice I was writing a short novel. 😅

@floooh
Copy link
Owner

floooh commented Nov 16, 2020

Just some quick notes, I need to look in detail later into the linked documentation:

Currently all backends directly forward most events from the platform to the sokol event callback. This means that it's not guaranteed when the event callback will be called relative to the frame callback and how often. E.g. it's not guaranteed whether mouse events can arrive at a higher-than-frame frequency or not (PS: uh this sounds wrong, of course a sokol-app event callback can never "interrupt" the frame callback because it runs on the same thread, but theoretically at least, the event callback might be called multiple times between two frame-callback invocations with mouse-moved events).

Those "non-guarantees" also give more freedom for platforms that don't work that way. So for Wayland I'd say "whatever works best or is less code" :)

Regarding "pointer-events" vs mouse-events and touch-events: If Wayland has separate event types for mouse- and touch-input, then those should be used to feed the sokol mouse- and touch-events. For instance UWP has generic pointer-events (which essentially tries to emulate a mouse pointer with touch input), and that's a mess (for instance when handling multiple pressed mouse buttons). If a sokol-app application wants to provide unified mouse- and touch-input, it should do so explicitely.

Only the 4 "standard" low-level multi-touch event types should be implemented (if provided by Wayland): touches-began, touches-moved, touches-ended and touches-cancelled. This is basically the common set of touch events across platforms where more "sophisticated" cross-platform gesture recognizers can be implemented on top.

But IMHO for the initial implementation we can also ignore touch events.

@fleischie
Copy link
Author

After some more reading of the wayland references I have I found the way to add touch-support, but I was unable to test it due to lacking hardware. I did emulate the way wayland registers touch events using the pointer-callbacks and some dummy state machine, just to make sure my implementation doesn't misbehave for the most simplest cases (e.g. one touch-down, some movements, cancel/touch-up).

✌️

@fleischie
Copy link
Author

For transparency's sake: I spent the last couple of days getting to know the wayland way to implement pointer-locks, and I managed to implement a working solution in my wayland-only demo that I use for reference when adding wayland behavior to sokol.

It turned out to have a bit more to it, than I thought. Generally what we need to do is:

  • Add the unstable pointer-constraints wayland protocol-extension,
  • use it to lock the pointer in place, which also prevents any further default mouse events,
  • add the unstable relative-pointer wayland protocol-extension,
  • use it to create a relative pointer object that extends the default mouse with - as the name suggests - relative mouse movement events.

Now functionality-wise this would be enough, but what I neglected until now was the display of a cursor image. When a pointer enters a wayland surface the cursor surface itself is in an undefined state. So to ultimately hide the cursor as described in the sokol header documentation text, we additionally need to:

  • Add the wayland-cursor dependency to the project,
  • use it to generate a cursor buffer from a shared memory that is applied to the cursor-surface,
  • which is attached to the pointer as soon as it enters the main wayland surface, and
  • toggle the display when the pointer is locked.

Yeah, this is what I needed to find out to understand what I need to do to add this functionality to sokol.

✌️

@floooh
Copy link
Owner

floooh commented Nov 30, 2020

IMHO if some sort of mouse capture works (meaning mouse coordinates are reported while a mouse button is pressed also when the mouse is outside the window area), then mouse-lock can be ignored in the beginning. IFIR on X11 and macOS this was automatically the case, only Windows needed special handling.

Proper mouse lock (where the mouse pointer is hidden and the mouse doesn't bump against the screen borders) is mostly needed for camera-controls, while mouse capture is important for UI applications (e.g. grab a slider and drag it around with the mouse leaving the window area).

@fleischie fleischie force-pushed the feature/wayland branch 2 times, most recently from 3c9eaf8 to 741acf9 Compare December 1, 2020 18:26
@fleischie
Copy link
Author

fleischie commented Dec 2, 2020

@floooh Can you outline, what the UPDATE_CURSOR event is for, and how one would handle it?

Situation in the wayland branch:

  • The wayland-specific data contains a "cursor-surface" that is used to render the cursor into,
  • the buffer is filled via the wayland-pointer library using the default cursor-theme when handling the seat's capabilities, and
  • on wayland's pointer-enter event the buffer is initially assigned (depending on whether to actually show the pointer).

Now, theoretically I could emit the UPDATE_CURSOR event instead of manually loading the cursor-theme. How would this work on the user-facing implementation: Do I as a user need to know that I am using wayland? How do I pass the data back into sokol (i.e. assign some value to somewhere, call a public API function, etc.)? How does a user know what kind of data to provide? Am I as the user able to bypass the sokol mechanism to set the wayland cursor? etc.

Any insights?

I checked how sokol handles the Win and macOS capabilities, but neither documentation I found were rather helpful.

I'll leave it out for now, but if you find the time to elaborate, it would help.

Thanks in advance. 🙇‍♂️

✌️

I noticed I am probably using a bunch of wayland-specific lingo that might not be obvious, feel free to inquire what I mean. 😜

@floooh
Copy link
Owner

floooh commented Dec 2, 2020

UPDATE_CURSOR is a bit of a hack currently to allow application code to override the cursor image. Eventually this might be replaced with proper custom cursor image support similar to GLFW. You can ignore this in Wayland (currently UPDATE_CURSOR events are only generated in the Win32 and macOS backends).

@fleischie fleischie force-pushed the feature/wayland branch 2 times, most recently from 99e711a to b5ee8a6 Compare December 2, 2020 21:24
@fleischie
Copy link
Author

Two topics I contemplated today:

UPDATE_CURSOR

I played around with it for a few minutes, and theoretically it would be possible for the wayland implementation to expose this:

  • Send event in seat_handle_capabilities when pointer capability is enabled,
  • expose the required wayland cursor data to control the cursor rendering,
  • implement a function that explicitly sets the wayland cursor buffer/image/etc. that is to be drawn into the cursor_surface,
  • describe in the documentation that to handle the UPDATE_CURSOR event the user needs to call this flag with an appropriate input data, and how it will be fallen back if failed to do so.

In a general sense, I honestly didn't give it really much though. 😅

Wayland protocol extensions

I just wanted to note, that with the pointer lock, there are already three external wayland protocol extensions that would be required to add via definitions before importing sokol_app.h. This is not too pretty.
I will try to tinker a bit with manually copying the appropriate header content into sokol. Would probably increase the file length of sokol_app.h quite a bit, but also the DX. I'll keep you updated.

@floooh
Copy link
Owner

floooh commented Dec 3, 2020

I will try to tinker a bit with manually copying the appropriate header content into sokol.

Hmmm that doesn't sound too good, depends of course how much needs to be added. How commonly are those extensions supported? What are the downsides of not embedding the required definitions? Are those headers hard to come by?

@fleischie
Copy link
Author

How commonly are those extensions supported?

As far as I can tell most wayland installations bring them all.

What are the downsides of not embedding the required definitions?

We would need require the user to:

  1. Create the header definitions externally,
  2. define the header definitions as SOKOL_WAYLAND_..._HEADER_PATH variable, and
  3. pass the appropriate source files when compiling the sokol program on linux.

Are those headers hard to come by?

Yes, they aren't distributed as C files, but as XML files. The user usually has to create the source files via the wayland-scanner program.

I suspect we can get away, if I wayland-scan the protocol extensions once, and hand-pick the necessary definitions/implementations into the sokol header. This way we would have access to the necessary signatures, and would not require the user to do anything on top. Again, I'll tinker a bit with it, and let you know with a working solution. This way you don't need to stress about it for now. 😄

@floooh
Copy link
Owner

floooh commented Dec 3, 2020

Yes, they aren't distributed as C files, but as XML files.

I see, hmm, in that case I guess it's best to pick out the definitions we need and embed them in sokol_app.h, can't be worse than all the GL definitions in the Win32 GL loader ;)

@fleischie
Copy link
Author

@floooh Moin. I am currently working through the drag'n'drop API of wayland and had a question regarding Sokol client's intended behavior regarding dnd-sources: Is there an entrypoint, where these clients start the dnd-operations as sources?

All I found is how sokol clients are destinations for dnd. I will start implementing this particular part of the behavior. Let me know, if I am missing something relevant.

Happy holidays [(already)|(belatedly)] (depending on when you read this message, and whether we interact again before then).

✌️ 😎

@fleischie
Copy link
Author

Oh boi, I got caught up playing Bloodborne the last few weeks. 😅

I will try to focus now on this project a bit more. I rebased my branched on the latest master and will now work to get the drag and drop API working as intended. 👍

@fleischie
Copy link
Author

Finally managed to put the time and effort into adding support for drag and drop.
Next is:

  • primary selection (via protocol extension),
  • take care of inlining the external protocol extensions, and
  • make sure the backend is not compile-time specific.

After that I will see to checking whether the rendering features need extra setup for wayland. Or maybe create a Github check to automate code testing for the wayland branch.

Hey and after that I only need some code internal cleanup. So there is an end to the tunnel. 😅

✌️

@fleischie
Copy link
Author

After drafting a first implementation of the primary selection support to sokol, I think I will rather remove this feature from the backlog.

Couple of reasons:

  • Primary selection would require a completely distinct buffer from the currently available one,
  • this would also mean separate events, enable-flags, etc.,
  • I don't feel comfortable hard-coding the functionality to the middle mouse button by default,
  • new wayland protocol extension with unconventionally long names,
  • to have a separate clipboard under wayland I would have to move the clipboard struct typedef way up,
  • ...

It was not a lot of effort or even difficult to research and implement the functionality (you mainly have to add a primary selection device manager, get a device, register a callback, and read the offered selection). So if this will be requested in the future, let me know. ✌️🤓

@floooh
Copy link
Owner

floooh commented Feb 13, 2021

I didn't even know what "primary selection" is and had to look it up ;)

@fleischie
Copy link
Author

I didn't even know what "primary selection" is and had to look it up ;)

Oh right, I may have been too close to the substance. Primary Selection is a "feature" of the X display, that allows the user to highlight text to select in one application and pasting the implicitly copied text with the use of the middle mouse button. It is an alternative mechanism to the main selection buffer that requires the user to explicitly copy selected text.

See here for a critical - because wayland-centered - view on the topic: https://wiki.gnome.org/Initiatives/Wayland/PrimarySelection

This was yielding C++-compilation errors and was probably only working
implicitly.
…macro

The macro expanded to a `void *` assignment when accessing the `data`
field, which broke C++-compilation. This commit specifies the array-data
to cast to, so that the assignment is type-correct.
Inspect return values of various wayland-specific functions to handle
potential errors. These yielded compilation errors previously.
This commit adds cursor-selection to the wayland branch after it was
introduced to the main branch.

- Create wayland cursor objects and image buffer for each available
  cursor type,
- fill array with relevant variant at seat initialization,
- warn about missing cursor image,
- make cursor selection protocol-aware, and
- update feature matrix to reflect wayland support.
@frink
Copy link

frink commented May 11, 2023

Is this orphaned or are you still working on it?

@fleischie
Copy link
Author

Is this orphaned or are you still working on it?

Neither currently.

Your message makes me feel extremely unappreciated for my voluntary work that I do in my free time. This is in part due to its brevity, generality and lack of specific inquiry. I kindly ask you to be more aware of how you interact with open source developers in the future.

If you should have a specific question, please formulate it in a way I can give a meaningful answer. Otherwise you are wasting my time, not the other way around.

@frink
Copy link

frink commented May 22, 2023

Is this orphaned or are you still working on it?

Neither currently.

@fleischie @floooh What help do you need to get this buttoned up?

@floooh
Copy link
Owner

floooh commented Jun 5, 2023

Maybe some expectation management and explanations are in order:

The main reasons why Wayland support hasn't been merged doesn't have to do anything with this PR, but everything with Wayland not being a complete replacement for X11. Instead Wayland only seems to provide the compositor part and some random (and optional) bits and pieces of a complete windowing system, while leaving the rest to widget frameworks like Qt or GTK (even for "obvious" window system features like rendering the window chrome).

I'm aware that Wayland has all sorts of extensions for things like title bar rendering, but as long as those are optional they might as well not exist, and AFAIK the most popular Linux desktop environment (GNOME) still doesn't support title bar rendering (maybe I'm wrong there, I only did some casual googling) - but title bar rendering is far from the only problem with Wayland.

This is just one thing that "feels wrong" about Wayland from the PoV of an application developer, those design decisions only make sense when understanding that Wayland isn't supposed to be used directly by application developers, instead Linux UI applications are supposed to be written against a widget framework like GTK or Qt.

But libraries like GLFW, SDL or sokol_app.h fall into a crack that most likely wasn't deemed important when Wayland was designed. Such applications only need a standard window, but don't render any 'standard widgets'. Now this type of "non-UI applications" has a dilemma: either pick a 3rd-party UI framework like GTK or Qt, depend on an optional Wayland extension, or render their own window chrome which makes them stick out like a soar thumb.

And now suddenly XWayland doesn't look so bad anymore as a solution, because it provides a consistent window look and feel without having to link against a specific widget framework.

Long story short: don't expect 'official' Wayland support in sokol_app.h until Wayland and GNOME sort these problems out (which probably won't happen because that's how Wayland was intentionally designed).

Maintaining two backends (X11 and Wayland) doesn't make much sense either.

If XWayland stops working one day, sokol_app.h will most likely use GTK for the window system glue.

@frink
Copy link

frink commented Jun 8, 2023

So if I'm understanding correctly the problem is that Wayland doesn't have window chrome and Sokol does want to or can't create its own title bar and borders etc... So since Wayland doesn't take care of window chrome Sokol suggests using X11 with xWayland instead of this PR. Did I encapsulate everything correctly?

@fleischie
Copy link
Author

The main reasons why Wayland support hasn't been merged doesn't have to do anything with this PR, but everything with Wayland not being a complete replacement for X11. Instead Wayland only seems to provide the compositor part and some random (and optional) bits and pieces of a complete windowing system, while leaving the rest to widget frameworks like Qt or GTK (even for "obvious" window system features like rendering the window chrome).

I have two thoughts:

  • "complete replacement for X11": I may be a bit philosophical/ideological but I wonder why we let X11 not die but wayland not live, either. Wayland cannot fully replace X11 because of wayland's design decisions explicitly targeted towards resolving X11's almost 40 year old inception.
  • "compositor part and optional random bits and pieces": Especially for sokol this is in fact a problem. Unfortunately the client-/server-side decoration (CSD/SSD) issue has become a dogmatic/religious problem between Gnome and KDE. There are optional libraries that handle this, e.g. https://gitlab.freedesktop.org/libdecor/libdecor (which SDL uses if I remember correctly).

I'm aware that Wayland has all sorts of extensions for things like title bar rendering, but as long as those are optional they might as well not exist, and AFAIK the most popular Linux desktop environment (GNOME) still doesn't support title bar rendering (maybe I'm wrong there, I only did some casual googling) - but title bar rendering is far from the only problem with Wayland.

As reference only KDE supports SSD, Gnome doesn't. (Interestingly enough KDE was incepted because none of the apps at that time looked/felt the same.) A quick test yielded different CSD on Gnome for firefox and the libdecor demo application, the same apps on KDE Plasma was coherent.

Long story short: don't expect 'official' Wayland support in sokol_app.h until Wayland and GNOME sort these problems out (which probably won't happen because that's how Wayland was intentionally designed).

Realistically this means without much political pressure this PR is only a baseline for others to work off of. I would maintain it if I so desire, but it will always "just" be a fork of sokol. Right?

Maintaining two backends (X11 and Wayland) doesn't make much sense either.

I understand.

What would you suggest then? I mean honestly I am a bit disappointed to abandon the work I already poured in it, but I have learned enough to have made it worthwhile for myself. The only consequence would be no built-in wayland support for sokol users (only maybe via xwayland).

@frink
Copy link

frink commented Jun 9, 2023

I was hoping this would lead to eventual inclusion of direct kmsdrm rendering but I get the idea of not cluttering things. Sokol is so close to being the perfect app wrapper for everything. Just not embedded or new display server for now. Very sad...

@quadroli
Copy link

To chime in, I too I'm eager to see this come to fruition, I think Sokol_app is awesome and being able to have a single header drop in replacement to GLFW is awesome as well, not that GLFW isn't awesome, it is, but from a portability pov having your graphical app being fully self-contained is highly appealing. I am also highly pleased with the progress made as I'm seeing nearly all of the check boxes have been ticked 🥳 kudos!! 👏🏼

@digitalsignalperson
Copy link

digitalsignalperson commented May 2, 2024

I tried running this PR as-is with a sample program and also with it rebased to the latest master, but for both I get the same segfault at wl_proxy_set_queue(). I'm not sure if anyone has any ideas, but I'll share my notes in case anyone else starts poking at this. I have 0 knowledge of writing a wayland client so I'd have to do some studying to try to figure this out any further.

Some WIP on a rebase to latest master: master...digitalsignalperson:sokol-custom:wayland-pr-rebase
I squashed the commits from this PR, otherwise it was taking a long time to resolve conflicts in 31 commits. It may have mistakes. The logging parts are incomplete (added a temporary #define _sapp_fail(line) for quick and dirty get up and running).

If I test the original version, or the one rebased on master, I get a segfault during initialization.

My setup is arch linux, and tested kde plasma 6.0.4 and sway compositors.

Testing this from sokol-samples:

cc cube-sapp.c ../libs/sokol/sokol.c -o cube-sapp -DSOKOL_GLCORE33 -pthread -I../../../sokol-wayland \
  -I../libs -lGL -ldl -lm -lasound -lwayland-client -lwayland-cursor -lwayland-egl -DSOKOL_DISABLE_X11 \
  -lxkbcommon -lEGL -g

Backtrace of the segfault:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7d28b5f in wl_list_remove () from /usr/lib/libwayland-client.so.0
(gdb) bt
#0  0x00007ffff7d28b5f in wl_list_remove () from /usr/lib/libwayland-client.so.0
#1  0x00007ffff7d28b9c in wl_proxy_set_queue () from /usr/lib/libwayland-client.so.0
#2  0x0000555555567895 in _sapp_wl_setup (desc=0x5555555a65e0 <_sapp>) at ../../../sokol-wayland/sokol_app.h:13013
#3  0x000055555556860c in _sapp_linux_wl_run (desc=0x7fffffffe0b0) at ../../../sokol-wayland/sokol_app.h:13206
#4  0x00005555555688b6 in _sapp_linux_run (desc=0x7fffffffe0b0) at ../../../sokol-wayland/sokol_app.h:13300
#5  0x0000555555568921 in main (argc=1, argv=0x7fffffffe368) at ../../../sokol-wayland/sokol_app.h:13307

It fails when we call wl_proxy_set_queue. I confirmed the args are not NULL, but didn't find any hints for what else to try. In case it was a compositor issue I tried sway as an alternative to KDE, and tried a desktop and a laptop, all behaving the same.

Also FYI you can use native wayland windows using GLFW with sokol (without sokol_app.h). I added my notes to that in #425

@digitalsignalperson
Copy link

digitalsignalperson commented May 6, 2024

I got it working here master...digitalsignalperson:sokol-custom:wayland-pr-rebase after a couple small changes 22c243d

  • Using wl_proxy_create_wrapper (Create a proxy wrapper for making queue assignments thread-safe.) fixed the segfault in initialization.
  • Also needed to check for NULL when getting cursors (I had NULL for"right_ptr" which then segfaulted trying to get the cursor/buffer).

But so far I've only tested with the cube-sapp.c demo.

Also for reference, the big section of protocol code can be generated with

# xdg-shell-protocol:
wayland-scanner client-header \
  < /usr/share/wayland-protocols/stable/xdg-shell/xdg-shell.xml \
  > xdg-shell-protocol.h
wayland-scanner private-code \
  < /usr/share/wayland-protocols/stable/xdg-shell/xdg-shell.xml \
  > xdg-shell-protocol.c

# pointer-constraints-unstable-v1-protocol:
wayland-scanner client-header \
  < /usr/share/wayland-protocols/unstable/pointer-constraints/pointer-constraints-unstable-v1.xml \
  > pointer-constraints-unstable-v1-protocol.h
wayland-scanner private-code \
  < /usr/share/wayland-protocols/unstable/pointer-constraints/pointer-constraints-unstable-v1.xml \
  > pointer-constraints-unstable-v1-protocol.c

# relative-pointer-unstable-v1-protocol:
wayland-scanner client-header \
  < /usr/share/wayland-protocols/unstable/relative-pointer/relative-pointer-unstable-v1.xml \
  > relative-pointer-unstable-v1-protocol.h
wayland-scanner private-code \
  < /usr/share/wayland-protocols/unstable/relative-pointer/relative-pointer-unstable-v1.xml \
  > relative-pointer-unstable-v1-protocol.c

Which could be included like this

#include "xdg-shell-protocol.h"
#include "xdg-shell-protocol.c"
#include "pointer-constraints-unstable-v1-protocol.h"
#include "pointer-constraints-unstable-v1-protocol.c"
#include "relative-pointer-unstable-v1-protocol.h"
#include "relative-pointer-unstable-v1-protocol.c"

Would it make sense to let this code be autogenerated in a build script and reduced to those include lines? Or would that break with the single header philosophy...

Regenerating that code today generates a huge number of lines

------------------------------------------------------------------------------
File                                          blank  comment     code    Total
------------------------------------------------------------------------------
xdg-shell-protocol.h                            100     1615      612     2327
pointer-constraints-unstable-v1-protocol.h       43      438      186      667
xdg-shell-protocol.c                             21       28      134      183
relative-pointer-unstable-v1-protocol.h          26      174       97      297
pointer-constraints-unstable-v1-protocol.c       15       24       69      108
relative-pointer-unstable-v1-protocol.c          12       24       43       79
------------------------------------------------------------------------------
SUM                                             217     2303     1141     3661
------------------------------------------------------------------------------

which minus the comments I assume is identical to what is currently in the patch anyway.

@digitalsignalperson
Copy link

I made another branch to make maintaining a wayland fork easier: master...digitalsignalperson:sokol-custom:wayland-external

git diff master --stat
 README.md           |   38 +++
 sokol_app.h         |   94 ++++--
 sokol_app_wayland.h | 1752 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1857 insertions(+), 27 deletions(-)

This moved the whole wayland implementation into an external header so that if SOKOL_LINUX_CUSTOM is defined, then these functions can be implemented externally:

  • _SOKOL_PRIVATE void _sapp_linux_run(const sapp_desc* desc);
  • _SOKOL_PRIVATE void _sapp_linux_toggle_fullscreen(void);
  • _SOKOL_PRIVATE void _sapp_linux_update_cursor(sapp_mouse_cursor cursor, bool shown);
  • _SOKOL_PRIVATE void _sapp_linux_lock_mouse(bool lock);
  • _SOKOL_PRIVATE void _sapp_linux_update_window_title(void);
  • _SOKOL_PRIVATE void _sapp_linux_set_icon(const sapp_icon_desc* icon_desc, int num_images);
  • _SOKOL_PRIVATE void _sapp_linux_set_clipboard_string(const char* str);
  • _SOKOL_PRIVATE const char* _sapp_linux_get_clipboard_string(void);

So upstream changes to sokol_app.h are very easy to rebase this way. I'm also interested to try this pattern for implementing an SDL or GLFW backend.

FWIW with respect to the stuff about gnome/decorations, for myself I don't care about those. I don't need window chrome or what I'm creating needing to look "native". If I wanted that I'd use Qt. The whole point for me using sokol is to be able to make any custom GUI/graphics I want, so it's pretty much the opposite of wanting native look and feel. Also for window moving and resizing Super+LMB to drag or Super+RMB to resize is my expectation from the compositor. That said, I totally understand if for the general case all these things being deal breakers for merging.

By the way thank you @fleischie and @floooh for doing all this work/support in this PR to make wayland a possiblity!

@fleischie
Copy link
Author

@digitalsignalperson Hello. 👋

First of all: Thank you for your effort.

Second: I did make a mistake with the queue creation, which was hidden in previous versions of wayland (i.e. back when I first started with it). You cannot create an event-queue from a display and set it back onto the same display. In newer versions this is a segfault as you found out and your solution is also very much correct. I just never got around to fix it and update this PR.

Third: I also thought about separating the wayland-branch from sokol's main header. This way development would have been easier to continue while keeping sokol's main development unencumbered. But I overall stopped spending time on this PR unfortunately.

So again I really appreciate your effort. I don't know if I can muster the energy to update this PR itself after this long a time, or whether you want to take over. I'm still here and open for discussions if you want. 🙂

@digitalsignalperson
Copy link

Hi @fleischie, thanks for the message.

One technical question, I noticed when exiting the program it says warning: queue 0x62f6d5be5b40 destroyed while proxies still attached: and lists various buffers. This was mostly resolved by moving the queue destruction to be last
1ad934e but there's still a wl_data_offer and a wl_shm_pool I couldn't find where they are created.

warning: queue 0x62f6d5be5b40 destroyed while proxies still attached:
  wl_data_offer@4278190080 still attached
  wl_shm_pool@26 still attached

Any ideas where from?

@fleischie
Copy link
Author

fleischie commented May 6, 2024

@digitalsignalperson I don't know for sure. I have some ideas how this works, but I need to check again.

wl_data_offer is created for the copy/paste feature and wl_shm_pool is necessary for the cursor images. I think they are always created on the display object.

Edit: I lied, the wl_shm_pool is used to share data between compositor and client. My latest branch at least does not allocate that object itself. As you found out already (and for example is done here: libsdl-org/SDL@132b887 ) you are correct in moving the proxy-wrapper/queue destruction to the very end of the cleanup loop.

If you are very pedantic you could try to run the proxy-wrapper/queue destroy calls sequentially until you find the call, that triggers the warning you described. But practically if you already found a working solution I don't think it's necessary. 🤷

@digitalsignalperson
Copy link

Ignoring those cleanup errors for now since they aren't the biggest issue, when trying more complicated applications things crash when calling wl_display_dispatch_queue_pending() with

../wayland-1.22.0/src/wayland-client.c:233: wl_proxy_unref: Assertion `proxy->refcount > 0' failed.

Many sokol-samples I try work as expected (cube-sapp, shapes-sapp, sdf-sapp). I'm getting the assertion fail with imgui-sapp, cimgui-sapp, nuklear-sapp. They show the window and appear to render at least enough frames I can see the full UI before it crashes.

#0  0x00007ffff78b932c in ?? () from /usr/lib/libc.so.6
#1  0x00007ffff78686c8 in raise () from /usr/lib/libc.so.6
#2  0x00007ffff78504b8 in abort () from /usr/lib/libc.so.6
#3  0x00007ffff78503dc in ?? () from /usr/lib/libc.so.6
#4  0x00007ffff7860d46 in __assert_fail () from /usr/lib/libc.so.6
#5  0x00007ffff7e0739d in wl_proxy_unref (proxy=<optimized out>) at ../wayland-1.22.0/src/wayland-client.c:233
#6  0x00007ffff7e07d11 in destroy_queued_closure (closure=0x5555561f75f0) at ../wayland-1.22.0/src/wayland-client.c:288
#7  0x00007ffff7e08d89 in dispatch_event (display=<optimized out>, queue=<optimized out>) at ../wayland-1.22.0/src/wayland-client.c:1637
#8  0x00007ffff7e0913c in dispatch_queue (queue=0x55555578ed00, display=0x55555578aac0) at ../wayland-1.22.0/src/wayland-client.c:1777
#9  wl_display_dispatch_queue_pending (display=0x55555578aac0, queue=0x55555578ed00) at ../wayland-1.22.0/src/wayland-client.c:2019
#10 0x00005555555e24c3 in _sapp_linux_run (desc=0x7fffffffe140) at /mnt/0078/sokol-samples-scratch2/sokol/sokol_app_wayland.h:1743
#11 0x00005555555dcaec in main (argc=1, argv=0x7fffffffe3f8) at /mnt/0078/sokol-samples-scratch2/sokol/sokol_app.h:11189

I thought maybe this is a hint https://bugzilla.mozilla.org/show_bug.cgi?id=1685055 but I think we are doing all of wl_display_prepare_read_queue(), wl_display_dispatch_queue_pending(), and reading events in the same thread. Maybe the issue is similar to the need for wl_proxy_create_wrapper for setting up the queue. Which I also realized the lifetime of the wrapper only needs to be like this I think

    struct wl_display* wrapped_display = (struct wl_display*)wl_proxy_create_wrapper(_sapp_wl.display);

    _sapp_wl.event_queue = wl_display_create_queue(_sapp_wl.display);
    wl_proxy_set_queue((struct wl_proxy *) wrapped_display, _sapp_wl.event_queue);
    if (NULL == _sapp_wl.event_queue) {
        _sapp_fail("wayland: wl_proxy_set_queue() failed");
    }

    _sapp_wl.registry = wl_display_get_registry(wrapped_display);

    wl_registry_add_listener(_sapp_wl.registry, &_sapp_wl_registry_listener, NULL);
    wl_display_roundtrip_queue(_sapp_wl.display, _sapp_wl.event_queue);
    wl_proxy_wrapper_destroy(wrapped_display);

but I didn't get anywhere with my naive poking around

@fleischie
Copy link
Author

@digitalsignalperson shouldn't this be wl_display_rountrip queue(wrapped_display, _sapp_wl.event_queue)? Just a though from reading your comment.

@digitalsignalperson
Copy link

@fleischie searching for other usages, I find similar

e.g. https://github.com/flightlessmango/MangoHud/blob/55712618fab8b85bfba469353ee7218eb0154e0f/src/wayland_keybinds.cpp#L107

void update_wl_queue()
{
    wl_display_roundtrip_queue(wl_display_ptr, queue);
}

void init_wayland_data()
{
   if (!wl_display_ptr)
      return;

   struct wl_display *display_wrapped = (struct wl_display*)wl_proxy_create_wrapper(wl_display_ptr);
   queue = wl_display_create_queue(wl_display_ptr);
   wl_proxy_set_queue((struct wl_proxy*)display_wrapped, queue);
   wl_registry *registry = wl_display_get_registry(display_wrapped);
   wl_proxy_wrapper_destroy(display_wrapped);
   wl_registry_add_listener(registry, &registry_listener, NULL);
   update_wl_queue();
   update_wl_queue();
   keyboard = wl_seat_get_keyboard(seat);
   wl_keyboard_add_listener(keyboard, &keyboard_listener, NULL);
   update_wl_queue();
}

where the vanilla display pointer is used for everything except wl_proxy_set_queue and wl_display_get_registry, and destroyed immediately after.

Also interesting here https://github.com/intel/external-wayland/blob/7a4d77bc392e8a2225b8a52ec83959e206c6ab39/src/wayland-client.c#L1225 in wl_display_roundtrip_queue it creates a temporary wrapper.

/** Block until all pending request are processed by the server
 *
 * \param display The display context object
 * \param queue The queue on which to run the roundtrip
 * \return The number of dispatched events on success or -1 on failure
 *
 * This function blocks until the server has processed all currently issued
 * requests by sending a request to the display server and waiting for a
 * reply before returning.
 *
 * This function uses wl_display_dispatch_queue() internally. It is not allowed
 * to call this function while the thread is being prepared for reading events,
 * and doing so will cause a dead lock.
 *
 * \note This function may dispatch other events being received on the given
 * queue.
 *
 * \sa wl_display_roundtrip()
 * \memberof wl_display
 */
WL_EXPORT int
wl_display_roundtrip_queue(struct wl_display *display, struct wl_event_queue *queue)
{
	struct wl_display *display_wrapper;
	struct wl_callback *callback;
	int done, ret = 0;

	done = 0;

	display_wrapper = wl_proxy_create_wrapper(display);
	if (!display_wrapper)
		return -1;

	wl_proxy_set_queue((struct wl_proxy *) display_wrapper, queue);
	callback = wl_display_sync(display_wrapper);
	wl_proxy_wrapper_destroy(display_wrapper);

	if (callback == NULL)
		return -1;

	wl_callback_add_listener(callback, &sync_listener, &done);
	while (!done && ret >= 0)
		ret = wl_display_dispatch_queue(display, queue);

	if (ret == -1 && !done)
		wl_callback_destroy(callback);

	return ret;
}

That docstring also feels like it may have some useful hints, where we are also blocking until exhausting the event queue and using wl_display_dispatch_queue_pending per

        /* exhaust event queue */
        while (0 > wl_display_prepare_read_queue(_sapp_wl.display, _sapp_wl.event_queue)) {
            wl_display_dispatch_queue_pending(_sapp_wl.display, _sapp_wl.event_queue);
        }
        wl_display_flush(_sapp_wl.display);

        int event_count = epoll_wait(_sapp_wl.epoll_fd, _sapp_wl.events, _SAPP_WAYLAND_MAX_EPOLL_EVENTS, -1);
        for (int i = 0; i < event_count; i++) {
            if (_sapp_wl.event_fd == _sapp_wl.events[i].data.fd) {
                /* NOTE: check _sapp_wl_setup() for wl_proxy_set_queue()
                 * call, that sets the custom event_queue as proxy for
                 * default display, that's why the following call is not
                 * explicitly stating the queue to read events for/from */
                wl_display_read_events(_sapp_wl.display);
            }
        }

        _sapp_frame();
        eglSwapBuffers(_sapp_wl.egl_display, _sapp_wl.egl_surface);
        wl_surface_damage_buffer(_sapp_wl.surface, 0, 0, INT32_MAX, INT32_MAX);
        wl_surface_commit(_sapp_wl.surface);

        wl_display_dispatch_queue_pending(_sapp_wl.display, _sapp_wl.event_queue);

@digitalsignalperson
Copy link

digitalsignalperson commented May 8, 2024

woot, progress! disabling the data_device listener (comment out wl_data_device_add_listener) now imgui works digitalsignalperson@b3219bc. I suspected because wl_display_roundtrip_queue(_sapp_wl.display, _sapp_wl.event_queue); is called in the drop and selection callbacks that it might cause issues.

also I think that last code snippet I pasted starting at "exaust event queue" can be reduced to

        wl_display_dispatch_queue(_sapp_wl.display, _sapp_wl.event_queue);
        _sapp_frame();
        eglSwapBuffers(_sapp_wl.egl_display, _sapp_wl.egl_surface);
        wl_surface_damage_buffer(_sapp_wl.surface, 0, 0, INT32_MAX, INT32_MAX);
        wl_surface_commit(_sapp_wl.surface);

where wl_display_dispatch_queue() looks like it's doing the same things https://gitlab.freedesktop.org/wayland/wayland/-/blob/main/src/wayland-client.c?ref_type=heads#L1999
and it seems to work for me like this

Edit: eh not entirely sure about the last part. Just hacking around and breaking things trying to understand how things work..

@digitalsignalperson
Copy link

digitalsignalperson commented May 8, 2024

Fixed the issue with data_device listener (clipboard handling issue) digitalsignalperson@6d1b636

Now imgui demos work with copy/paste, and I tested droptest-sapp works too

Edit: Also fixed scrolling (x and y were backwards and inverted) and scaled them so one vertical mousewheel click scrolls 3 lines, and one horizontal mousewheel click scrolls about 5 columns. digitalsignalperson@30d94ff I'm curious for feedback if this is universal or just for my compositor and mouse

Summary of changes

Per: digitalsignalperson/sokol-custom@wayland-external-init...digitalsignalperson:sokol-custom:wayland-external (also this before refactor to seperate files master...digitalsignalperson:sokol-custom:wayland-pr-rebase)

  • use wrappped_display to create the event_queue
  • ensure event queue is destroyed last
    • TODO queue destroyed while proxies still attached: wl_data_offer, wl_shm_pool
  • consolidate code reading events from queue with wl_display_dispatch_queue
  • clipboard: read the fd as many times needed to read all the data (chunks of 64kB)
  • clipboard: only accept the offer when Ctrl+V is presesd
  • scrolling: x/y and +/- in correct directions
  • scrolling: add scale factors so that x/y scrolling ticks map to expected amounts (y: 3 lines per wheel click, x: 5 columns per wheel click)
    • TODO: scale factors are ad-hoc. Consistent for other systems? Is there a better solution?
  • call _sapp_timing_measure to get correct fps and frame timing measurements
  • input: don't pass modifiers to key event when it is a KEY_UP
  • cursors: ensure SAPP_MOUSCURSOR_{DEFAULT,ARROW} are "left_ptr"
    • TODO: what is the purpose of "right_ptr"?
  • clipboard: when copy-pasting within the application, detect the pipe loopback and decline the offer
  • cursors: set x,y offsets according to hotspot differences during cursor change

@digitalsignalperson
Copy link

Noting for window chrome, this could optionally be enabled in a really basic way with no dependencies in maybe a few hundred lines of C like in this example: https://github.com/danielwolbach/wayland-window/blob/main/source/main.c

Aside: it'd be a fun project to make the chrome itself with sokol_gfx and do something like glowing/animated with shaders :)

@frink
Copy link

frink commented Aug 14, 2024

Noting for window chrome, this could optionally be enabled in a really basic way with no dependencies in maybe a few hundred lines of C like in this example: https://github.com/danielwolbach/wayland-window/blob/main/source/main.c

That sounds like a lot!!! Sokol should be able to do a window titlebar in under a 100 sloc...

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.

9 participants