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

[build.zig] GLFW Platform Detection Support #4150

Merged
merged 3 commits into from
Jul 11, 2024

Conversation

InventorXtreme
Copy link
Contributor

While tinkering with Zig and raylib, I noticed that the Zig-compiled version of raylib was running through XWayland. I determined that the build.zig file would, by default, only support X11, and could be manually switched to Wayland, but could not use both via runtime platform detection.

I resolved this problem by adding a new value, "Both" to the LinuxDisplayBackend enum and replacing the switch statement that was responsible for including the libraries with a set of if statements. This change allows for both backends to run off of one executable compiled through Zig. I also changed the default support mode from X11 only to both.

I tested the changes on my machine and was able to run the same binary natively in both Wayland and X11.

Apologies if anything is funky with my submission, this is my first time contributing code to another project.

Made formating fit within raylib standards and changed the default option to support both X11 and wayland on Linux.
@Gamer-Kold
Copy link
Contributor

This looks good to me on initial glance but I'll probably wanna try building this myself to see if this works on my machine.

A little bit surprised this is all it takes for runtime detection of display protocol. Does it automatically fall back to X11 if Wayland support is enabled?

@InventorXtreme
Copy link
Contributor Author

It seems that as long as both X11 and Wayland are enabled for glfw and the appropriate libraries are linked, it figures it out. I roughly copied this from the make file, which I knew had worked in the past so I think this is all it takes.

If the build is compiled with the "both" option, it will automatically fall back to X11, but if it is only compiled with Wayland support, it will not be able to fall back because the X11 support will not be enabled and the libraries will not be linked. I tested the same executable compiled with the new "both" option and it ran natively on both platforms without a recompile. The user can of course still choose to support only one platform if they would prefer.

@Gamer-Kold
Copy link
Contributor

Tried it; got the same executable working on both X11 and Wayland natively. This looks good to me.

@raysan5 raysan5 merged commit 44c6cd2 into raysan5:master Jul 11, 2024
@raysan5
Copy link
Owner

raysan5 commented Jul 11, 2024

@InventorXtreme thanks for the improvement!

@Gamer-Kold thanks for the review!

@lnc3l0t
Copy link
Contributor

lnc3l0t commented Aug 6, 2024

This introduces a problem on X11-only systems, because it assumes both X11 and Wayland are installed by default:

linux_display_backend: LinuxDisplayBackend = .Both,

Without wayland-scanner installed this line:

const client_step = b.addSystemCommand(&.{ "wayland-scanner", "client-header" });
will cause waylandGenerate() to fail.

I propose adding a check, to prevent errors and to suggest changing linux_display_backend in the user's build.zig.

Notes

  • The same issue might apply to Wayland-only system missing X11 dependencies
  • The problem isn't limited to wayland-scanner, other wayland-* libraries can also cause build failures without helpful logs.

lnc3l0t added a commit to lnc3l0t/raylib that referenced this pull request Aug 6, 2024
raysan5#4150 introduced a default value for linux_display_backend,
which makes X11-only systems fail to build.
raysan5 pushed a commit that referenced this pull request Aug 6, 2024
#4150 introduced a default value for linux_display_backend,
which makes X11-only systems fail to build.
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.

4 participants