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

[sokol_app] basic X11 clipboard #998

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Dvad
Copy link

@Dvad Dvad commented Feb 23, 2024

Implement clibpard feature using Xlib. This is a basic version which only supports utf8 target and the clibpoard selection. (not the selection + middle click workflow).

Simplified adaptation of glfw in https://github.com/glfw/glfw/blob/master/src/x11_window.c

@Dvad
Copy link
Author

Dvad commented Feb 23, 2024

@floooh here is my version regarding #995.

By the way, is there a code style or code formatter I should use on the code when submitting PR?

Implement clibpard feature using Xlib. This is a basic version which only supports utf8 target and the clibpoard selection. (not the selection + middle click workflow).

Simplified adaptation of glfw in https://github.com/glfw/glfw/blob/master/src/x11_window.c
@floooh
Copy link
Owner

floooh commented Feb 23, 2024

That looks reasonably simple :) Somehow and suddenly there's a wave of PRs coming in, it might be a couple of days before I have time to properly look at the PR.

In the meantime, have a look at the failed CI build please:

https://github.com/floooh/sokol/actions/runs/8015098805/job/21903872711?pr=998

Unused variable warnings should be supressed with _SOKOL_UNUSED(var), and C functions without args must have an explicit void arg (e.g. const char* _sa_sapp_x11_get_clipboard_string(void)).

(the CI builds have extremely picky warning settings)

PS: also somehow, there's X11 code that leaked into the Android and iOS builds (search for error:):

Android: https://github.com/floooh/sokol/actions/runs/8015098802/job/21903872342?pr=998

iOS: https://github.com/floooh/sokol/actions/runs/8015098802/job/21903873030?pr=998

You can test locally whether the Linux version compiles with the strict CI warning settings by running ./test_linux.sh in the tests subdirectory.

sokol_app.h Show resolved Hide resolved
sokol_app.h Show resolved Hide resolved
@floooh
Copy link
Owner

floooh commented Feb 23, 2024

PPS:

By the way, is there a code style or code formatter I should use on the code when submitting PR?

I don't have clang-format setup yet, you should mainly check that the opening { does not go into its own line (there's a couple of places in the PR where { is currently on its own line), and the else branches look like this } else { (the latter I only changed a little while ago, so there may still be some places with the old else-style where the } was on its own line and else started a new line.

But please don't run a formatter over the code, that tends to bury the actual changes in formatting noise.

@Dvad Dvad force-pushed the x11_clipboard branch 2 times, most recently from 77a51b9 to 228b073 Compare February 23, 2024 19:11
@Dvad
Copy link
Author

Dvad commented Feb 23, 2024

Updated. Hopefully all failure are gone.

Somehow and suddenly there's a wave of PRs coming in, it might be a couple of days before I have time to properly look at the PR.

All good, thanks in the first place for considering them.

@floooh
Copy link
Owner

floooh commented Mar 2, 2024

FYI I'll start testing this PR next, but not sure yet if I'll get around to merge today or tomorrow.

@floooh
Copy link
Owner

floooh commented Mar 2, 2024

Hmm, when testing, pasting from the system clipboard into the sokol application works, but not pasting out of it into the system clipboard (e.g. sapp_set_clipboard_string() seems to be a no-op.

You can test this by running imgui-sapp, go to Widgets => Text Input => Multi-line text input (see screenshot below) mark some text, press Ctrl-X or Ctrl-C. Then go into a regular desktop application and try to paste there (on KDE, there's also a Klipper app which lets you inspect the clipboard content). The cut/copied text from the sokol-app doesn't make it into the system clipboard (the function _sapp_x11_set_clipboard_string is being called though).

Looking at the GLFW code just leaves me confused TBH :D We should try to get this fixed though before merging (but I suspect the code won't be quite as simple afterwards, I wonder if GLFW is actually the most simple code for writing text into the X11 clipboard).

(I tested on an Ubuntu 23.10 laptop with KDE desktop session)

image

@Dvad
Copy link
Author

Dvad commented Mar 3, 2024

Thanks for testing!

Hum, I need to investigate, both direction are definitely working on my machine. (Fedora 38, Gnome running on X11).
Yeah GLFW is a bit confusing, I simplified a bit by cutting the select + middle-click flow. I wonder if I have simplified too much and dropped support for some cases without noticing.

@floooh
Copy link
Owner

floooh commented Mar 4, 2024

Interesting that it works on your side, can you describe step by step how you cut/copy out of an sokol-app application into the system? Maybe I messed something up or encountered an edge case.

@Dvad
Copy link
Author

Dvad commented Mar 4, 2024

Ctrl+X/Ctrl+C from the Multi-line text input then Ctrl+V my terminal worked (I use tilix).
Since Xorg clipboard is a peer to peer protocol, I was thinking maybe my code does not play nice with what the clipboard manager do? Or it could be a Wayland/Mir vs X11 thing?

@Dvad
Copy link
Author

Dvad commented Mar 24, 2024

BTW, I did not abandon this PR, but I am lacking time to test on a ubuntu machine to debug currently. My attempt to use a virtual machine was not successfull so far.

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

2 participants