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

Implement getting X11 clipboard contents #1805

Merged
merged 4 commits into from
Jul 3, 2021

Conversation

psychon
Copy link
Contributor

@psychon psychon commented May 23, 2021

This PR adds support for getting the clipboard contents. I only tested this with strings, but would expect that other kinds of data should work as well. There is still lots of TODO left here: Only get_string and get_format are implemented. Of course all the put_* are missing, but also available_type_names and preferred_format are missing since I ran out of time for now.

Edit: Updated to also implement available_type_names and preferred_format. put_* is best left for another PR, I think, because that requires keeping some state and my best idea for that so far is to change Clipboard into Rc<ClipboardState> so that the clipboard contents can be kept in ClipboardState.

@psychon psychon force-pushed the x11-clipboard branch 2 times, most recently from 324577f to bfad91d Compare May 23, 2021 15:32
@psychon
Copy link
Contributor Author

psychon commented May 23, 2021

I forgot to mention in any of the commits: This is a first step towards #937. (Now that I left this here, GH will surely link the issue to this PR)

Copy link
Collaborator

@maan2003 maan2003 left a comment

Choose a reason for hiding this comment

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

Thanks! I have tested this for strings. Not sure to test with other formats.

druid-shell/src/platform/x11/clipboard.rs Outdated Show resolved Hide resolved
druid-shell/src/platform/x11/clipboard.rs Outdated Show resolved Hide resolved
@psychon
Copy link
Contributor Author

psychon commented May 23, 2021

Oh and: Somewhen, this might need timeouts. If the transfer did not finish after [time], abort and return None.

However, now is not the time for that. (As in, I don't have the time right now.)

@psychon
Copy link
Contributor Author

psychon commented May 31, 2021

I updated this to also implemented available_type_names() and preferred_format().

available_type_names() is just a transfer of type TARGETS. This produces a list of Atoms (i.e.: Vec<u32>). This list then just has to go through self.connection.get_atom_name (element-wise).

preferred_format() finds the first entry in the result of available_type_names(), which seems to be what the gtk backend is doing.

I tested this with:

fn main() {
    let app = druid_shell::Application::new().unwrap();
    println!("{:?}", app.clipboard().available_type_names());
    println!("{:?}", app.clipboard().preferred_format(&["FOOBAR", "UTF8_STRING"]));
}

Output is:

["TIMESTAMP", "TARGETS", "MULTIPLE", "_VIMENC_TEXT", "_VIM_TEXT", "UTF8_STRING", "COMPOUND_TEXT", "TEXT", "STRING"]
Some("UTF8_STRING")

With the gtk backend I get:

["TIMESTAMP (133)", "TARGETS (129)", "MULTIPLE (132)", "_VIMENC_TEXT (136)", "_VIM_TEXT (137)", "UTF8_STRING (70)", "COMPOUND_TEXT (138)", "TEXT (139)", "STRING (31)"]
Some("UTF8_STRING")

Dunno why the gtk backend also includes the number of the atom. That does not seem useful to me and would break my implementation of preferred_format(), so I did not do that.

@psychon psychon force-pushed the x11-clipboard branch 2 times, most recently from 1c8b3c0 to 6e09b50 Compare May 31, 2021 07:52
@cmyr
Copy link
Member

cmyr commented Jun 29, 2021

Taking a look at this, and just wanted to make sure that the changes related to the screen number and timestamps is intended to be part of this PR?

@maan2003
Copy link
Collaborator

timestamp is needed for the clipboard, screen_num looks like a unrelated refactor

@psychon
Copy link
Contributor Author

psychon commented Jul 1, 2021

To clean up this PR, I rebased it on current master and squashed all the commits (since you use squash merges anyway). I also wrote a hopefully good commit message for this new commit.

Edit:

screen_num looks like a unrelated refactor

It is indeed unrelated. If you want, I can remove it from this PR.

This commit implements X11 clipboard transfers as specified in ICCCM.
This allows to get the contents of the clipboard.

X11/ICCM call the underlying mechanism "selections". This works with
ConvertSelection requests. The X11 server forwards these requests to the
selection owner which then uses SendEvent requests to answer. Thus, this
requires some way to blockingly wait for events. For this purpose, a
FIFO queue (VecDeque) of pending events is introduced. When waiting for
the "right" event, "wrong" events are pushed to this queue for later
processing.

Doing selection transfers requires an up-to-date-ish X11 timestamp.
Thus, the Application now tracks a timestamp and updates it whenever it
gets a newer timestamp.

As an unrelated refactor, this changes the X11 screen number to be saved
as usize instead of i32. This saves a couple of unnecessary casts. I
didn't want to do this too much in this commit, so screen_num() still
returns i32 instead of usize, even though I think it should be an usize.

Besides the above, the actual selection transfer is fully contained in
clipboard.rs.

The basic steps for getting the selection contents are:

- create an invisible window (used for the reply)
- send a ConvertSelection request with this window
- wait for a SelectionNotify event (at this point, the selection owner
  set a property on the window)
- get the contents of the property from the window
- in case the selection contents are larger than allowed for window
  properties, the property as type INCR. This indicates an "incremental
  transfer" which works as follows:
  - every time the property is deleted, the selection owner creates a
    new property with the next chunk of the property
  - thus, we have to wait for PropertyNotify events for our special
    window and react to them by getting the next piece of data

Signed-off-by: Uli Schlachter <psychon@znc.in>
@cmyr
Copy link
Member

cmyr commented Jul 1, 2021

@psychon if that's the only unrelated change then no worries, but if there's other stuff in here that's unrelated breaking them out into a separate PR will definitely make review easier.

Copy link
Collaborator

@maan2003 maan2003 left a comment

Choose a reason for hiding this comment

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

Looks good! Just some minor comments

druid-shell/src/platform/x11/clipboard.rs Outdated Show resolved Hide resolved
druid-shell/src/platform/x11/clipboard.rs Outdated Show resolved Hide resolved
druid-shell/src/platform/x11/clipboard.rs Show resolved Hide resolved
psychon and others added 3 commits July 3, 2021 18:56
@maan2003 turn `for` into functional!

Co-authored-by: Manmeet Maan <49202620+Maan2003@users.noreply.github.com>
Signed-off-by: Uli Schlachter <psychon@znc.in>
Signed-off-by: Uli Schlachter <psychon@znc.in>
@maan2003 maan2003 merged commit 1d55330 into linebender:master Jul 3, 2021
@psychon psychon deleted the x11-clipboard branch July 4, 2021 05:17
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

3 participants