floooh / sokol

minimal cross-platform standalone C headers
https://floooh.github.io/sokol-html5
zlib License
7.12k stars 501 forks source link

[sokol_app] basic X11 clipboard #998

Closed Dvad closed 2 months ago

Dvad commented 9 months ago

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 commented 9 months ago

@floooh here is my version regarding https://github.com/floooh/sokol/issues/995.

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

floooh commented 9 months ago

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.

floooh commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

floooh commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

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.

qwx9 commented 2 months ago

Not sure what the best form is on github, I opened a new PR to submit an amended patch. Thanks for your work!

floooh commented 2 months ago

@Dvad I'm closing this PR in favour of https://github.com/floooh/sokol/pull/1108. Unfortunately you didn't make it into the git history, I wonder if we could have handled that better. I did mention you in the changelog though.