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

Add platform::startup_notify for Wayland/X11 #2955

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

kchibisov
Copy link
Member

The utils in this module should help the users to activate the windows they create, as well as manage activation tokens environment variables.

The API is essential for Wayland in the first place, since some compositors may decide initial focus of the window based on whether the activation token was during the window creation.

Fixes #2279.

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

--

I'm not sure how to properly integrate it into the winit at this very moment, because we'll have a platform specific event. Well to be fair, we have them already for macOS backend and such (see gestures things).

@notgull do you want to look into impl of X11 part? I've linked the spec in the module, but I'm not sure how to implement it, there's a libstartup-notify, but I don't think we want it? Maybe x11rb could help us here?

@kchibisov
Copy link
Member Author

kchibisov commented Jul 12, 2023

Draft, because I can't add just Wayland for that, since the way to inform is .desktop file and missing X11 at least for startup could bring issues.

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

I'll make an X11 implementation after work today.

src/platform/startup_notify.rs Show resolved Hide resolved
src/platform/startup_notify.rs Outdated Show resolved Hide resolved
@psychon
Copy link
Contributor

psychon commented Jul 12, 2023

I've linked the spec in the module, but I'm not sure how to implement it, there's a libstartup-notify, but I don't think we want it?

I took a quick look at the spec and.... uhm... it's extensible. 😅 .

My TL;DR from reading the spec would be: If env variable DESKTOP_STARTUP_ID is set, unset this variable and set its content as property _NET_STARTUP_ID on "the right window" (whichever window that is). There is a comment about group leader windows, but I guess winit doesn't use one (I haven't checked).

And my TL;DR from looking at libstartup-notification: O


Looking at the libsn git repo, https://cgit.freedesktop.org/startup-notification/tree/test/test-launchee-xcb.c seems relevant. This does:

  • sn_xcb_display_new() (seems to be global setup; is also used by the launcher example)
  • sn_launchee_context_new_from_environment() + error checking (apparently this function fails if the right env var is not set?)
  • sleep(4)
  • sn_launchee_context_complete()
  • feed all received events through sn_xcb_display_process_event().

Sooo... what does each of these functions do?

  • sn_xcb_display_new() interns some atoms. Nothing interesting going on.
  • sn_launchee_context_new_from_environment() fetches DESKTOP_STARTUP_ID from the environment and saves it.
  • sn_launchee_context_complete() sends a remove message as explained in the spec:
/**
 * sn_launchee_context_complete:
 * @context: an #SnLauncheeContext
 *
 * Called by the launchee when it is fully started up and the startup
 * sequence should end.
 * 
 **/
void
sn_launchee_context_complete (SnLauncheeContext *context)
{
  char *keys[2];
  char *vals[2];
  char *message;
  
  keys[0] = "ID";
  keys[1] = NULL;
  vals[0] = context->startup_id;
  vals[1] = NULL; 

  message = sn_internal_serialize_message ("remove",
                                           (const char**) keys,
                                           (const char**) vals);

  sn_internal_broadcast_xmessage (context->display,
                                  context->screen,
                                  sn_internal_get_net_startup_info_atom(context->display),
                                  sn_internal_get_net_startup_info_begin_atom(context->display),
                                  message);

  sn_free (message);
}

I tried to rust-ify this. No guarantees and no testing, but this is how I understand things:

use std::os::unix::ffi::OsStrExt;
use x11rb::connection::Connection;
use x11rb::protocol::xproto::{
    ClientMessageEvent, ConnectionExt as _, CreateWindowAux, EventMask, WindowClass, WindowWrapper,
};

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let startup_id = match std::env::var_os("DESKTOP_STARTUP_ID") {
        Some(value) => value,
        None => {
            println!("No startup stuff going on");
            return Ok(());
        }
    };
    let startup_id = startup_id.as_bytes();

    let (conn, screen) = x11rb::connect(None)?;

    // Not shown here: Create a window, set the _NET_STARTUP_ID property on it, map it

    finish_startup(&conn, screen, startup_id)
}

fn finish_startup(
    conn: &impl Connection,
    screen_idx: usize,
    startup_id: &[u8],
) -> Result<(), Box<dyn std::error::Error>> {
    let mut message = "remove: ID=".as_bytes().to_vec();
    message.extend(startup_id);
    message.push(0);
    send_message(conn, screen_idx, &message)
}

fn send_message(
    conn: &impl Connection,
    screen_idx: usize,
    message: &[u8],
) -> Result<(), Box<dyn std::error::Error>> {
    let info_begin = conn
        .intern_atom(false, b"_NET_STARTUP_INFO_BEGIN")?
        .reply()?
        .atom;
    let info = conn.intern_atom(false, b"_NET_STARTUP_INFO")?.reply()?.atom;
    let mut message_type = info_begin;

    let screen = &conn.setup().roots[screen_idx];

    // Yay, the spec wants us to create a new window for each message
    let window = WindowWrapper::create_window(
        conn,
        screen.root_depth,
        screen.root,
        -100,
        -100,
        1,
        1,
        0,
        WindowClass::INPUT_OUTPUT,
        screen.root_visual,
        &CreateWindowAux::new()
            .override_redirect(1)
            .event_mask(EventMask::STRUCTURE_NOTIFY | EventMask::PROPERTY_CHANGE),
    )?;

    let mut remaining_message = message;
    while !remaining_message.is_empty() {
        let mut data_to_send = [0; 20];
        let to_send_len = remaining_message.len().min(data_to_send.len());
        data_to_send[..to_send_len].copy_from_slice(&remaining_message[..to_send_len]);
        remaining_message = &remaining_message[to_send_len..];

        conn.send_event(
            false,
            screen.root,
            EventMask::PROPERTY_CHANGE,
            ClientMessageEvent::new(8, window.window(), message_type, data_to_send),
        )?;
        message_type = info;
    }

    conn.flush()?;

    Ok(())
}

@psychon
Copy link
Contributor

psychon commented Jul 12, 2023

After having written the above, I took another look at the current state of the PR.

  • Apparently I was wrong in looking only at the launchee. This code also wants to be the launcher, apparently?
  • On Wayland, this works quite different than on Wayland. I don't know the Wayland protocol, but it feels like you cannot easily abstract over the two.
  • Apparently, on Wayland one has to request such a token. I read the startup spec as "you just invent a token" on X11:
All messages must include these keys:

  ID
 
          uniquely identifies a startup sequence; should be some globally 
          unique string (for example, hostname+pid+"_TIME"+current time).
          The string should be in the form of <unique>_TIME<timestamp>,
          where the timestamp is the X server timestamp of the user
          action that caused the launch. See the TIMESTAMP key
          for details.

@kchibisov
Copy link
Member Author

@psychon inventing is not an issue, because inventing token and sending it back to user is fine, Wayland is more strict here.

@kchibisov
Copy link
Member Author

On Wayland, this works quite different than on Wayland. I don't know the Wayland protocol, but it feels like you cannot easily abstract over the two.

I think you can since GTK does that just fine, as well as firefox. They both use both protocols via the same internal C++ interfaces. It's true that X11 API is a bit weird here, but Wayland protocol links to X11 one for some stuff.

It's true though, that I don't quite understand what X11 really wants here, I've tried to understand the GTK thing, but got confused what leader window is. Wayland spec is more clear here, you ask for token you activate with that token, dead simple.

For X11 we at least need handling for startup case, as in where we pass the variable from the env to first winit window and then create it. From what I've read we must set do (taken from gtk, you've mentioned that in your rust code):

      XChangeProperty (display_x11->xdisplay,
                       display_x11->leader_window,
                       gdk_x11_get_xatom_by_name_for_display (display, "_NET_STARTUP_ID"),
                       gdk_x11_get_xatom_by_name_for_display (display, "UTF8_STRING"), 8,
                       PropModeReplace,
                       (guchar *)startup_id, strlen (startup_id));

We may leave request of the code alone, but I don't see any issue with inventing and sending it back via the event to the user keeping the async-ness of the API, since doing it sync on Wayland is not really an option (though, don't you need to broadcast your invented token somehow?).

@psychon
Copy link
Contributor

psychon commented Jul 12, 2023

but got confused what leader window is.

I think that refers to ICCCM window groups: https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.11

A set of top-level windows that should be treated from the user's point of view as related (even though they may belong to a number of clients) should be linked together using the window_group field of the WM_HINTS structure.
One of the windows (that is, the one the others point to) will be the group leader and will carry the group as opposed to the individual properties. Window managers may treat the group leader differently from other windows in the group. For example, group leaders may have the full set of decorations, and other group members may have a restricted set.

It is not necessary that the client ever map the group leader; it may be a window that exists solely as a placeholder.

(I don't think I ever saw any WM treat group leaders specially, but that doesn't mean much; I'm not WM-switching often)

(though, don't you need to broadcast your invented token somehow?).

Yup, looking again at the tests for libstartup-notification, there is a call to sn_launcher_context_initiate() which broadcasts a message starting with new (via the equivalent of what is send_message() in my rust hack). This event can have many, many properties (e.g. NAME, DESCRIPTION, APPLICATION_ID). Always set are ID and SCREEN, all other ones seem to be optional. The ID is generated via:

  snprintf (s, len, "%s/%s/%d-%d-%s_TIME%lu",
            canonicalized_launcher, canonicalized_launchee,
            (int) getpid (), (int) sequence_number, hostbuf,
            (unsigned long) timestamp);
  ++sequence_number;

where canonicalized comes from replacing all / with | in the caller-provided arguments of similar names. sequence_number is a static counter, hostbuf the host name of the system and timestamp is also caller-provided.

So, that would be something like send_message("new: ID=your-chosen-id SCREEN=0") (SCREEN is the X11 screen number, but realistically it will always be zero ;-) ).

@kchibisov kchibisov force-pushed the activation-token branch 2 times, most recently from 2f5a69a to 3acc71c Compare July 13, 2023 05:36
@kchibisov kchibisov force-pushed the activation-token branch 2 times, most recently from 22670a8 to 34cb588 Compare July 20, 2023 12:10
@kchibisov kchibisov marked this pull request as ready for review July 20, 2023 12:11
The utils in this module should help the users to activate the windows
they create, as well as manage activation tokens environment variables.

The API is essential for Wayland in the first place, since some
compositors may decide initial focus of the window based on whether
the activation token was during the window creation.

Fixes rust-windowing#2279.
@kchibisov kchibisov merged commit f7a84a5 into rust-windowing:master Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Window activation support
4 participants