Right now there's a lot of Rc copying and RefCell borrowing in the examples. The worst offender of this is the pointer example, where the InputManager and OutputManager need a copy of the Rc just to pass it along to the structs they create.

In a real user program this could potentially get annoying quickly, with the only other viable alternative being lazy_static (with a performance hit on atomics of course).

This is issue for discussion of alternative ways of designing the library so that we can avoid this issue, if at all possible.

Should do the same as the wlroots example

No this isn't a typo, it's a real thing.

Documentation

Explain this confusing concept, see wlroots issues and source.

Events

• destroy

Functions

• pos or maybe coords return (x, y)
• output return Output
• layout_get Ensure ownership safety
• output_at
• output_coords

Should do the same thing as the wlroots example.

Will need others to test this though, since I don't have a touch screen

This is mostly wrapped though there's a few things left to do

Union Safety

• Make getting the type of the InputDevice safe by trusting the tag that's stored in the wlr_input_device. That way we don't have to deal with the union ever (and can just return an enum that is one of the input devices)

Misc functions

• name Convert the char* to a String and return that
• output_name Convert char* to a String and return that
• vendor i32 vendor id
• product i32 product id

Safety

• Ensure we don't need to use the destroy event / that we use it correctly.

Documentation

• Ensure that it's spelled out clearly that wlroots doesn't support tablets with more than one mode.

Events

• TabletPadButton
• TabletPadRing
• TabletPadStrip

TabletPad

• TabletPad struct that's passed in the tablet callbacks for the events. Doesn't hold any data by itself.

Enums

• TabletPadRingSource Used in the TabletPadRing event
• TabletPadStripSource Used in the TabletPadStrip event

Cleanup

• Cleanup in input_manager

Currently we have a PointerHandle but no Pointer state to pass to it. Here's what's needed in that struct (which will be similar to Keyboard):

Events

• PointerMotion
• PointerMotionAbsolute
• PointerAxis
• Button

Pointer struct

• Pointer struct doesn't actually hold anything (except some user data we may or may not use) but it should still exist in case more is added to it. For example, that might be a good place to also place the cursor information.

Misc

• pointer cleanup in input_manager

In wlroots there's a way to convert from an wlr_input_device to one of the concrete device types it refers to (e.g wlr_keyboard). It does this using a tagged union, so it's safe to use from that context.

The issue comes up is that we allow handles to these concrete device types, so it could be possible to have aliased mutable references to these types. This is made more complicated by the fact that we don't allow the user to store InputDevices to ensure this is upheld (because they aren't bound or checked in any way).

This issue proposes a safe way to do this conversion without causing mutable aliasing by utilizing these handle types.

We'd make a InputDeviceHandle, using the same mechanism we do for the concrete types. We'd have to store this handle within the concrete types, to ensure that it dies with the device. Then, to allow conversions we'd let you convert from an InputDeviceHandle to one of the concrete device handles (probably represented in an enum by the possible values it can return).

Currently, you cannot reconstruct a concrete device type using just the raw pointer to the device. E.g you can't make a valid KeyboardHandle using just the *mut wlr_keyboard because it doesn't contain the Rc/Weak information. To remedy this, we'd store a raw pointer to a into_rawed version of the Rc in the user data for all the concrete types. This would mean the reference count is always 2 while the device is valid. We'd use that when converting, because the wlr_input_device has access to this pointer and its user data. We'd then need to reclaim this Rc in the drop impl for all the concrete device types and ensure it's cleaned up along with the InputDevice Rc.

This makes the code much more complicated though, and it's possible I missed something that could break the safety of this. Looking over the code of rootston, there doesn't seem to be a necessary reason why you'd need to downcast from a InputDevice to a concrete device type, which is why this remains as a proposal rather than have an actual implementation in the code.

If there's any use cases that come up, they will be documented here and this will be implemented if it's necessary. Any arguments that this is actually unsafe is also welcome 😄.

Should do the same thing as the wlroots rotation example

Right now it's confusing because we have Output (as in the handle) and Output (as in the manager that holds the user data for an output).

To rectify this issue, a better name would be UserOutput which highlights what kind of data it stores.

Wrap wlr_list, as sometimes we don't want to pay the price to convert to a Vec

Make helper function to turn this into a Vec. Make it unsafe though, because now we can't be sure of the lifetime of the objects.

Right now there's a mix of as_ptr and to_ptr.

We should standardize on as_ptr (to_ptr makes it seem like ownership is transferred, which is not the case)

Let standardize it in fact with a trait:

trait PtrWrapper {
    type ptr_type;
    fn as_ptr(self) -> self::ptr_type;
    fn from_ptr(self::ptr_type) -> Self
}

Right now it's not an issue, but I want to provide the Keyboard from the callbacks and if you can access the Keyboard from both structures it breaks aliasing rules.

Right now for the MotionEvent and AbsoluteMotionEvent we expose access to the underlying InputDevice, which is an owned type that has a pointer to the device that could, at any time, become invalid.

Right now this only breaks one small function that's basically useless (InputDevice::dev_type), but never the less should not be possible (since it breaks memory safety).

I really don't want to make handle types for InputDevice (like I did for Pointer and Keyboard) because it could be possible to break mutable aliasing rules if I allow the InputDevice to be cast down into the concrete type (e.g you could have two Keyboards for the same underlying wlr_keyboard).

Though maybe it's possible if we modify InputDevice to have an enum that's one of the underlying types with the Rc information and we give out *Handle's to them... need to think about this.

Documentation

• This struct is state for a single client's bound wl_seat resource and is used to issue input events to that client. The lifetime of these objects are managed by the Seat.

A SeatClient is managed by a Seat, so it's probably best for the Seat to hold a SeatClient so that we can't have a dangling pointer. Since this is stored witin the wlroots' wlr_seat we need to probably just set and manage their raw pointer.

SeatClient

Functions

• resource unsafe
• client unsafe
• seat Double check this doesn't break mutable alias concerns, since this is a pointer to its parent!

Resources

• pointers
• keyboards
• touchs
• data_devices
• primary_selection_devices

Things will get complicated fast otherwise.

• Has a wayland global that needs to be a part of Compositor
• Should clean up state in it's Drop method (wlr_wl_shell_destroy) hmm that destroys the global.
• Should not be Clone or Copy
• For the events, there should be a ShellHandler trait similar to the I/O handlers. This will be passed into Compositor, just like the I/O managers.
• We might need to see if there's a way to deny shell spawning. This requires looking at the wayland spec to figure out. The best way to do that would be similar to OutputManager and InputManager (e.g have a function that Optionally returns a new user type that implements ShellHandler. If a Some is returned then the shell is created, other wise it's denied in whatever fashion is appropriate for the protocol).

To wrap:

• Functions
  • new
  • ping
  • set_geometry (wrapper for configure method)
  • popup_at (wrapper for wlr_wl_shell_surface_popup_at)

• Events

e.g in OutputManager and InputManager we should be cleaning up the signal listeners and the user data

Events

• TabletToolAxis
• TabletToolProximity
• TabletToolTip
• TabletToolButton (note uses same state as Button callback for Pointer)

TabletTool

• TabletTool struct that's passed in the callbacks, holds no data

Enums

• TabletToolAxisState Used in TabletToolAxis callback
• TabletToolProximityState Used in TabletToolProximity callback

Cleanup

• Cleanup in input_manager

Documentation

Documentation about pixman region

Only call is very unsafe, so no need to wrap unless this is expanded.

We have wrappers around Keyboard for example, but we don't check the type of the union. We need to do a sanity check when we try to access the union, otherwise it's UB.

We should also make a Rust enum, since the wlroots unions are tagged it's basically 1-1

Currently, when using one of the handles (e.g OutputHandle) it panics if you try to use it while already having a borrow to the structure it refers to. This is easy to do however, especially when in a callback that takes e.g a &mut Output and you try to borrow from an OutputHandle that may or may not alias to it.

The type signature will now look something like this:

pub fn run<F, R>(&mut self, runner: F) -> BorrowResult<Option<R>>

with BorrowResult being defined as:

pub type BorrowResult<T> = Result<T, BorrowErr>
pub struct BorrowErr;

impl ::std::error::Error for BorrowErr {
    fn description(&self) -> &str {
        "Tried to borrow a handle to a structure that is already borrowed"
    }
}

Using an OutputHandle one can construct full Outputs "safely" even though it's scheduled to be destroyed.

To fix this, we should deallocate the Rc<()> before we enter the callback, probably by utilizing ManuallyDrop.

This exists, but has issues.

Safety

Ensure that outputs live as long as they do in the layout. We don't currently do that!

Events

• added
• changed
• destroyed

Functions

• Make remove safe
• add Make adding an output safe, see above
• move
• outputs Vec of outputs
• closest_point
• get_box
• get_center_output

• Has a wayland global that needs to be a part of Compositor
• Should clean up state in it's Drop method (wlr_xdg_shell_v6_destroy)
• Should not be Clone or Copy
• For the events, there should be a XdgShellV6Handler trait similar to the I/O handlers. This will be passed into Compositor, just like the I/O managers.
• We might need to see if there's a way to deny shell spawning. This requires looking at the wayland spec to figure out. The best way to do that would be similar to OutputManager and InputManager (e.g have a function that Optionally returns a new user type that implements XdgShellV6Handler. If a Some is returned then the shell is created, other wise it's denied in whatever fashion is appropriate for the protocol).

To Wrap:

• Functions
  • new
  • ping
  • set_size
  • set_activated
  • set_maximized
  • set_fullscreen
  • set_resizing
  • close
  • popup_at

• Events

This is one of the globals that the compositor needs to create at startup.

Either it needs to be provided in the constructor (like the I/O managers) or it needs to be passed in later (like the extension protocols).

I'm up for either, leaning towards the latter since this is sort of an "extension" and it's possible that a compositor may not want to implement this interface (and we should take a page out of wlroots' book and let the user decide).

Either way, it needs to live as long as the compositor and destroyed before it disconnects.

Need to also ensure only one global is registered at a time of this type.

Functions

• send_selection Needs to only be possible if the DnD global has been made (so if an instance was made of the struct). Needs SeatClient to be made first.
• set_selection

DataOffer

Events

• destroy

Functions

• dnd_actions Return bitfield, please wrap in a bitfield!
• preferred_dnd_action`
• in_ask
• resource (unsafe)
• data_source (Return DataSource)

DataSource

Events

• destroy

Functions

• resource (unsafe)
• offer (Return DataOffer
• seat_client (Return SeatClient)
• mime_types (Wrap in Vec)
• accepted
• current_dnd_action
• dnd_actions Return bitfield, like in DataOffer
• compositor_actions Return bitfield, like in DataOffer
• actions_set

Global Callbacks

They should be callable, setting them may be unsafe but probably not. They are already initialized in global creation. Double check because this could be safe.

• accept
• send
• cancel

Global Cleanup

• Blocked by this issue clean up global

We are now having error cases now, and we should be using the new (and probably going to be standard) failure crate.

Stuff left to do:

Events

• Wrap all the data in KeyEvent (e.g time_msec
• wlr_key_state as an enum on Rust side

Keyboard

• Pass the keyboard in the keyboard callbacks. Currently we don't do this, and otherwise the current struct is unusable.
• keymap_fd (unsafe)
• keymap_size
• get_keymap (unsafe)
• get_xkb_state (unsafe)
• led_indexes (slice of the array, gotta store the size of LEDs and ensure they are always the same as in wlroots)
• mod_indexes (slice of the array, gotta store the size of the modifiers and ensure they are always the same as in wlroots)

Lots of these are unsafe, because it's unknown if it's worth wrapping them in the future. So the API surface might change to be more safe in the future.

Safety

Should be owned by Output to ensure memory safety

Functions

• size return (width, height)
• pos return (x, y)
• enabled
• hotspots return (hotspot_x, hotspot_y)
• texture
• surface ownership and mutable aliasing thought needed
• set_image
• set_surface
• move_to

Destructor

• set drop impl to wlr_output_cursor_destroy

I'm exposing raw pointers directly in many cases, because there are cases where we want to let the user get down and dirty with the raw wlroots state (the library should be usable 100% safely, but there might be use cases that are only possible with unsafe code and I want to facilitate that, providing the user annotates their code with unsafe).

However, when you have a public field like that you can easily do something like:

compositor.display = ::std::ptr::null_mut() // null pointer in safe code!

and then break everything.

To fix that, there should be the dreaded getters for the pointer types. We have those in most places, but there are a few places where this isn't done.

To fully test the library, we should recreate wlroot's rootston in 100% safe Rust.

Once this is done, and wlroots has stabilized (and started actually releasing versions), work will begin on porting Way Cooler

Prerequisites

Here are the prereqs needed to get rootston up to spec. There are links to relevant issues for each wlroots type where we outline how we will wrap them to ensure safety.

• Make basic examples first
  • Minimal
  • Simple
  • Pointer
  • Touch
  • Tablet
  • Output Layout
  • Multi-pointer
  • Rotation
• Types
  • Shells (possibly try to unify these?)
    • wlr_wl_shell
    • wlr_xdg_shell_v6
    • wlr_xdg_shell
  • Input
    • wlr_seat
    • wlr_seat_client
    • wlr_cursor
    • wlr_xcursor
    • wlr_xcursor_theme
    • wlr_data_device
    • wlr_input_device (is a union for the following):
      • wlr_keyboard
      • wlr_pointer
      • wlr_touch
      • wlr_tablet_pad
      • wlr_tablet_tool
  • Output
    • wlr_output
    • wlr_output_mode
    • wlr_output_cursor
    • wlr_output_layout
    • wlr_output_layout_output
  • Misc
    • wlr_box ("Area")
    • wlr_list
    • wlr_region
• Functionality wrapped
  • logging
  • rendering (still in flux, holding off)
  • Extensions

Rootston

• Configuration
  • Ini parsing
  • Xwayland configurable
  • Output layout
  • Cursor-output mapping
  • Per-device configuration for input
  • Keybinding
    • Meta key
    • Commands
      • exit
      • exec
      • close
      • next_window

Wrapper for wlr_box named Area to not clash with Rust's Box

Functions

• origin return (x, y)
• size return (width, height)
• closest_point
• intersection
• contains_point
• is_empty

Currently, it's very unergonomic (though very much safe) to use OutputLayout in conjunction with the two main types it deals with: namely Output and Cursor.

Currently, the user has construct an OutputLayout and then wrap it as Rc<RefCell<OutputLayout>> before passing it to Cursor. The actual implementation isn't complete yet either, and further use of the OutputLayout with e.g Output would require more clones and more confusion (because even if the user deletes her copy of the OutputLayout, there are probably other references still keeping it alive somewhere...not good).

So I'm proposing something that will make it much more ergonomic to use from the user perspective, but make it much, much harder to manage safely within the library.

We add another type: OutputLayoutHandle. It will be private, but function exactly the same as Output except that instead of being tied to the lifetime as dictated by wlroots it will be tied to the lifetime as dictated by the user. It will have a Weak<()> internally to an Rc<()> in the OutputLayoutHandle that will never go above 1.

This OutputLayoutHandle will live inside the user data for the Output, so that when we go to clean up the Output it will do one of the following:

1. The OutputLayout is no longer alive (if not, the user destroyed it). If so, no need to do cleanup because it did the appropriate clean up.
2. The OutputLayout is still alive. We need to clean up our output state from it (e.g call OutputLayout::remove(self))

The way we ensure the second case is not a double free (e.g when the user calls OutputLayout::remove(output) and then destroys the Output by e.g unplugging it) is by utilizing the signals on OutputLayout to automatically destroy the Weak<()> handle within the wlr_output user data when it's removed from the OutputLayout.

This will require massive changes to OutputLayout not the least of which is adding the ability to have signals. With this change though, we can also allow the user to hook in user-defined callbacks to run when an OutputLayout is modified (e.g an output is added, destroyed, or changes position). All the other signals do this by implementing using a trait object that's then wrapped in the type that actually has this information that can call the user-defined callbacks, so either it will have to be the same (e.g OutputLayoutHandler) or I'll find a different way to do it so that OutputLayout is still a struct.

This is wrapped, but there's still lots to do with this. The ownership requirements for this are tough, for example the XCursor is bound to it, the OutputLayout it can map to is shared amongst other structures, and wlroots plans to support multiple cursors. So this one will be a headache.

Ownership Safety

Ensure that it's not destroyed prematurely, since the drop is implemented to call the destructor on the pointer. This should be fine, except for the cases we hand out the wlr_cursor pointer.

Events

This has crossover with the input device events. Ensure that these are separate events, or that these listeners also need to be bound to the same events as the input device ones.

Button

• motion
• motion_event
• button
• axis

Touch

• touch_up
• touch_down
• touch_motion
• touch_cancel

TabletTool

• tablet_tool_axis
• tablet_tool_proximity
• tablet_tool_tip
• tablet_tool_button

Functions

• warp Should not take ownership of InputDevice, pass a reference
• warp_absolute Same as warp just call wlr_cursor_warp_absolute instead. Double check you can pass dev as null though, the documentation is unclear.
• move_to should allow optional InputDevice
• set_image Should be able to set arbitrary buffers
• set_surface ensure safety of the surface pointer, might need to keep a Rc to the surface to ensure it doesn't go out of scope.
• attach_input_dev Ensure safety that we don't lose the input device pointer handled by wlroots
• deattach_input_dev Probably can't pass an input_dev not associated with the Cursor, keep a list and disallow passing any not in it. Double check impl because that might be already handled in which case don't bother don't bother
• map_to_output Can only happen if cursor associated with output layout, check this before calling wlroots
• map_input_to_output Input device must be attached to this cursor, and the output must be in the attached output layout (like in map_to_output. Ensure before calling wlroots
• map_to_region
• map_input_to_region

Right now it takes an InputDevice which isn't "incorrect" but it does make it more difficult to use since you can't modify the pointer at all without having to do an ugly, unsafe cast from the device union.

Events

• TouchDown
• TouchUp
• TouchMotion
• TouchCancel

Struct

• Touch (doesn't hold anything, like Pointer would be good to provide. Maybe we can let the user use that data pointer for something (probably not)).

Cleanup

• Cleanup in input_manager

Should do the same thing as wlroots example

Safety

• make into_raw unsafe. It's not necessary, but it shows where unsafety can happen more clearly. unexposed, and renamed

Functions

• frame call wlr_xcursor_frame, make sure we can pass any time
• cursor_count
• cursors Do the same thing we did with XCursor images and return a slice, assuming it's safe
• name Rust String
• size

Right now they are all bunch up, and it makes it harder to read which is bad because it has the most complicated ownership requirements right now.

• screenshooter
• gamma-control
• server decoration
• gtk-primary-selection
• idle
• xwayland-keyboard-grab

I've established the swaywm GitHub org and moved wlroots there. We have discussed maintaining wlroots API wrappers within the swaywm organization - if you'd like, I can add you and we can host it there. Of course, the project would remain yours and you'd still have complete autonomy over it.

Functions

• flags bitfield on wl_output_mode, get from wayland-sys; add to that crate if it doesn't exist though
• size return (width, height)
• refresh

This might have to do with how we are handling the destruction event, as that's the last thing that shows up in the logs.

EDIT: According to the coredump, it fails during the pointer move event for the pointer example (note that the simple example works fine)

Mostly done, few things left to do.

The interface has changed, and now it must be re-supported.

Safety

• Double check that images slice is correct and that the lifetimes can't lead to unsafety.

Functions

• image_count
• name Return String
• total_delay Document it's the length of the animation in ms

The seat is a global object. It is assumed all compositors implement it, so this should be passed to Compositor during init. Note that it's not necessary for pointers and keyboard to work, but it's used to do libinput things.

Seat

• create
• drop impl calls wlr_seat_destroy
• set_capabilities Wrap capabilities in a bitfield!
• set_name Pass rust String
• pointer_surface_has_focus should be safe, but pass wrapped Surface
• enter_pointer Document that it sends a enter event to client and a leave event to the last focused client. Also that coords are surface-local. And to user notify_enter to change pointer focus to respect pointer grabs (TODO Should wlroots-rs do that? Is there a point where you wouldn't want to do that?)
• clear_focus
• send_motion Document that coords are surface-local, is sent to focused client, and that you should use pointer_notify_motion to send motion events (TODO Should wlroots-rs do that? Is there a pointer where you would not want to do that?)
• send_button Same as the other sends
• send_axis ditto
• pointer_start_grab Document: starts a grab of the pointer, the grabber is responsible for handling all pointer event until grab ends
• pointer_end_grab
• notify_enter See above for whether we should just do this ourselves, we probably shouldn't though
• notify_button
• notify_axis
• set_keyboard Ensure dev is a Keyboard, use the enum we will define for all input devices
• keyboard_start_grab similar docs to start pointer grab
• keyboard_end_grab
• keyboard_send_key Similar thing to pointer send stuff
• keyboard_notify_key
• keyboard_send_modifiers Similar thing to send_key
• keyboard_notify_modifiers
• keyboard_send_enter Similar thing to blah blah blah
• keyboard_notify_enter
• keyboard_clear_focus

Events

• set_cursor_event This is a struct but implies it's part of an event. Some investigation required.

There are certain types (e.g Output, Keyboard, etc.) that the user should not hold references to except in the limited contexts we provide (e.g usually callbacks when we know they are valid). We achieve this by only giving them borrows and not letting them clone them out.

To better encapsulate this, we should add some sort of suffix to the end of it. Handle is a good one, but I'm open to bike shedding.

• Keyboard -> KeyboardHandle
• Output -> OutputHandle

It's working, but still lots of things to wrap. All pretty straightforward.

Events

• frame
• mode
• enable
• scale
• transform
• swap_buffers
• destroy

Functions

• lx
• ly
• serial
• enabled
• scale
• transformation_matrix
• Make modes safe
• Remove events (unless it's necessary?)
• flags bitfield!
• refresh
• scale
• get_subpixel Wrap output?
• get_transform wrap output?
• needs_swap
• transform_matrix
• modes (return Vec)
• current_mode
• output_pos Document this, return lx and ly from wlr_output
• transform
• set_mode
• set_custom_mode
• set_position
• cursors // hardware cursors? safety issue
• hardware_cursor safety issue
• set_scale
• set_gamma
• get_gamma
• set_fullscreen_surface
• create_output_cursor // and associated functions, see output cursor ticket
• set_enable
• effective_resolution
• get_gamma_size
• wlr_output_create_global removed
• wlr_output_destroy_global

The output destroyed callback should immediately clean up the userdata and destroy the output once it has been called (without requiring the user to, for safety reasons).

This should be implemented in the Drop impl for the Output.

In order to do this, Output::from_ptr should now return ManuallyDrop<Output> instead of just Output. See here for info about manually drop. AFAIK, this won't cause it to leak any memory when we don't explicitly drop the Output in non-destroy callbacks.

Should do the same thing as the wlroots example.

Will need other to test, I don't have a touch screen computer available.