Skip to content

[rcore][GLFW] Fix GetClipboardImage() creating new connection under X11#5871

Merged
raysan5 merged 3 commits into
raysan5:masterfrom
steampuker:clipboard-fix
May 19, 2026
Merged

[rcore][GLFW] Fix GetClipboardImage() creating new connection under X11#5871
raysan5 merged 3 commits into
raysan5:masterfrom
steampuker:clipboard-fix

Conversation

@steampuker

@steampuker steampuker commented May 15, 2026

Copy link
Copy Markdown
Contributor

As of now, GetClipboardImage creates a new connection and a ghost window to retrieve image data and then unloads it all. This adds unnecessary overhead for this function caused by establishing and terminating connection.

This PR stores this clipboard connection and other related data in the global platform struct but is also lazy loaded (so a first call would have the overhead and subsequent ones would not). The clipboard handling data is unloaded upon ClosePlatform.

Update 18/05/26: Now this PR makes the use of the main raylib connection without creating new ones at all.

Tested with ubench:

UBENCH_EX(raylib, clipboard_image) {
    Image images[10];

    UBENCH_DO_BENCHMARK() {
        for(int i = 0; i < 10; ++i)
            images[i] = GetClipboardImage();
    }

    for(int i = 0; i < 10; ++i)
        UnloadImage(images[i]);
}

Without the fix, the result is:

[       OK ] raylib.clipboard_image (mean 4.350ms, confidence interval +- 1.332339%)

With the fix:

[       OK ] raylib.clipboard_image (mean 499.261us, confidence interval +- 1.124542%)

Comment thread src/platforms/rcore_desktop_glfw.c Outdated

// Request the data: "Convert whatever is in CLIPBOARD to image/png and put it in RAYLIB_CLIPBOARD_MANAGER"
XConvertSelection(dpy, clipboard, targetType, property, win, CurrentTime);
XConvertSelection(platform.cbDisplay, platform.cbAtom, platform.cbTargetAtom, platform.cbPropertyAtom, platform.cbWindow, CurrentTime);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ColleagueRiley's reference also calls XSync after this, do we need to add it here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not really required because you later wait for the SelectNotify event, but it may still be good to have.

The XSync() function flushes the output buffer and then waits until all requests have been received and processed by the X server.

@steampuker

Copy link
Copy Markdown
Contributor Author

I just looked into commit history, it seems RGFW is also affected? Do I port these to RGFW platform as well?

@ColleagueRiley

Copy link
Copy Markdown
Contributor

There should generally be possible to use the original GLFW X Display and X Window rather than creating your own internal display.

But, in my opinion, hacky backend fixes like these don't really belong in Raylib, as they make Raylib harder to maintain. This is exactly why the RGFW backend exists in Raylib, because the RGFW is more actively worked on and the implementation is easier to understand and modify. Features can be added directly into RGFW rather than being forced into each backend. Of course, that means that GLFW users will have to wait for GLFW to get updated, but I think that's for the best since for Raylib shouldn't have to maintain these messy hacks throughout each backend.

The rcore_win32 backend was made essentially for this purpose, hacks for win32 can be added directly to it instead of to each backend, but even then it still means that Raylib/Raysan will have to maintain it and I don't think it's worth it.

I have also seen plenty of these sorts of hacks being the result of AI generated code. Ray isn't as familiar with each low-level backend as someone like me, or a GLFW maintainer and I don't believe Ray directly tests for Linux or OSX. So dubious low level backend hacks will create hard to find bugs directly in Raylib, rather than being caught filtered out by RGFW/GLFW.

@steampuker

steampuker commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

There should generally be possible to use the original GLFW X Display and X Window rather than creating your own internal display.

But, in my opinion, hacky backend fixes like these don't really belong in Raylib, as they make Raylib harder to maintain. This is exactly why the RGFW backend exists in Raylib, because the RGFW is more actively worked on and the implementation is easier to understand and modify. Features can be added directly into RGFW rather than being forced into each backend. Of course, that means that GLFW users will have to wait for GLFW to get updated, but I think that's for the best since for Raylib shouldn't have to maintain these messy hacks throughout each backend.

The rcore_win32 backend was made essentially for this purpose, hacks for win32 can be added directly to it instead of to each backend, but even then it still means that Raylib/Raysan will have to maintain it and I don't think it's worth it.

I have also seen plenty of these sorts of hacks being the result of AI generated code. Ray isn't as familiar with each low-level backend as someone like me, or a GLFW maintainer and I don't believe Ray directly tests for Linux or OSX. So dubious low level backend hacks will create hard to find bugs directly in Raylib, rather than being caught filtered out by RGFW/GLFW.

Hello, Riley, thanks for your insight. At some point, I also thought of using raylib's main connection for this but then one would have to deal with event loop management. What if a Key or a Mouse event fires in the middle of this process? I suppose that's why it was made like that in the first place (even though I don't like it personally).

I agree that this is a hacky solution, having platform-specific code in a platform-agnostic backend is not a good idea. I suppose for Windows it worked because it's a few lines of code but for Linux, the situation is different (considering, there's also Wayland support wanted). If RGFW supports this out of the box, will be awesome but I'm also concerned about the GLFW backend.

My main issue with this whole thing is that because of this, projects using static raylib have to be explicitly linked to X11. The way it was before, GLFW would dynamically load necessary X11 functions and you would simply link raylib without additional flags (maybe aside from -lm), so the very first claim:

Would be true. Not sure about AI generated but this does feel like a PoC code pushed into the upstream. So now that raylib does require X11 linkage, I want the implementation to be somewhat usable in real projects.

@ColleagueRiley

ColleagueRiley commented May 17, 2026

Copy link
Copy Markdown
Contributor

I think your first point is somewhat true. Though, I'm pretty sure you are able to resolve this by peeking the event, seeing if it's the one you want and if not, put it back in the queue.

Yeah, I don't think it's even a good idea for winapi code due to maintainability issues. But you bring up some other points that I didn't mention.

Generally for RGFW I have two methods for handling wayland/x11, either the user has to choose one at compile time, making it incompatible with the other, or you can actually compile for both backends, and then choose the preferred one on runtime. It will also fallback to X11 if wayland fails.

While that does lead to a larger binary, I think that's generally worth it as it will ensure both platforms are supported by default. But Raylib can handle that however it sees fit.

I don't really consider X11 or Wayland to be an external dependency as they're core parts of the Linux platform. Dynamically loading doesn't necessarily fix the problem, as it requires the headers, but RGFW has a way of doing that too via XDL. But I would still consider dynamic loading to be a dependency.

I have seen AI code in other PRs too, but specially with the original changes for this feature @CrackedPixel and I saw some tell tale signs of AI. For example, how there were a ton of comments all over the code that were later removed.

Comment thread src/platforms/rcore_desktop_glfw.c
@raysan5 raysan5 changed the title [rcore][GLFW] Fix GetClipboardImage creating new connection under X11. [rcore][GLFW] Fix GetClipboardImage() creating new connection under X11 May 18, 2026
@raysan5 raysan5 added the platform: Linux (X11) Linux platform - X11 windowing label May 18, 2026
@steampuker

Copy link
Copy Markdown
Contributor Author

@ColleagueRiley, I found a way to use the main connection to handle the clipboard event without disrupting the event queue, I want your opinion. Tested with clipboard image example, theoretically it should work.

@ColleagueRiley

ColleagueRiley commented May 18, 2026

Copy link
Copy Markdown
Contributor

That looks good, since it also removed the SelectionNotify event form the queue.

Although there is no reason to use while-do rather than just doing while (XCheckTypedEvent(...) == False);

@steampuker

steampuker commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

Done. I need more reviews for this one, then I may port this to RGFW backend until it gets native support for clipboard image processing.

@ColleagueRiley

Copy link
Copy Markdown
Contributor

It would be fairly easy for me to add it to RGFW, the only problem is that I would want to implement each backend as well and I'm not sure how long that would take.

Although, I already have a plan for how the API would look.

@raysan5 raysan5 merged commit be56f2c into raysan5:master May 19, 2026
@raysan5

raysan5 commented May 19, 2026

Copy link
Copy Markdown
Owner

@steampuker thank you very much for the improvement and thanks @ColleagueRiley for the review

@steampuker

Copy link
Copy Markdown
Contributor Author

@raysan5, please note that RGFW still has the suboptimal code, I could port this to the RGFW backend (at least for now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform: Linux (X11) Linux platform - X11 windowing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants