hexops/mach

Fork 0

glfw: build for wayland and X11 by default #374

Merged

PiergiorgioZagaria merged 6 commits from main into main

2022-06-30 03:54:04 +00:00

PiergiorgioZagaria commented

2022-06-28 10:54:41 +00:00

(Migrated from github.com)

Helps hexops/mach#374 and hexops/mach#376

By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

Helps hexops/mach#374 and hexops/mach#376 - [X] By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

PiergiorgioZagaria commented

2022-06-28 14:46:12 +00:00

(Migrated from github.com)

To get this error I set x11 = false in the build options, otherwise glfw would use X11 instead of wayland.
Also, all tests from 10 to 133 print mesa: for the --amdgpu-atomic-optimizations option: may only occur zero or one times!.

The errors are:

Test [46/138] Monitor.test "set_getGammaRamp" the documentation seems to be wrong, since on wayland the returned error is FeatureUnavailable and not PlatformError
Test [49/138] key.test "getName", the c function segfaults on wayland

Segmentation fault at address 0x80
???:?:?: 0x7f683544b9c4 in ??? (???)
upstream/glfw/src/input.c:713:12: 0x30a0a7 in glfwGetKeyName (/home/Zargio/Documents/Github/mach/glfw/src/sources_all.c)
    return _glfw.platform.getScancodeName(scancode);
           ^
/home/Zargio/Documents/Github/mach/glfw/src/key.zig:220:43: 0x248115 in key.test "getName" (test)
        const name_opt = cc.glfwGetKeyName(@enumToInt(self), @intCast(c_int, scancode));

Test [81/138] Window.test "setAspectRatio" returns error FeatureUnimplemented
Test [89/138] Window.test "show" like the first error, FeatureUnavailable is returned instead of PlatformError
Test [92/138] Window.test "requestAttention" returns error FeatureUnimplemented

To get this error I set `x11 = false` in the build options, otherwise glfw would use X11 instead of wayland. Also, all tests from 10 to 133 print `mesa: for the --amdgpu-atomic-optimizations option: may only occur zero or one times!`. The errors are: - `Test [46/138] Monitor.test "set_getGammaRamp"` the documentation seems to be wrong, since on wayland the returned error is `FeatureUnavailable` and not `PlatformError` - `Test [49/138] key.test "getName"`, the c function segfaults on wayland ``` Segmentation fault at address 0x80 ???:?:?: 0x7f683544b9c4 in ??? (???) upstream/glfw/src/input.c:713:12: 0x30a0a7 in glfwGetKeyName (/home/Zargio/Documents/Github/mach/glfw/src/sources_all.c) return _glfw.platform.getScancodeName(scancode); ^ /home/Zargio/Documents/Github/mach/glfw/src/key.zig:220:43: 0x248115 in key.test "getName" (test) const name_opt = cc.glfwGetKeyName(@enumToInt(self), @intCast(c_int, scancode)); ``` - `Test [81/138] Window.test "setAspectRatio"` returns error `FeatureUnimplemented` - `Test [89/138] Window.test "show"` like the first error, `FeatureUnavailable` is returned instead of `PlatformError` - `Test [92/138] Window.test "requestAttention"` returns error `FeatureUnimplemented`

emidoots commented

2022-06-28 17:51:03 +00:00

(Migrated from github.com)

We should update the tests to catch those errors and print them.

We do this in other places, e.g. here

We should update the tests to catch those errors and print them. We do this in other places, e.g. [here](https://sourcegraph.com/github.com/hexops/mach@b8a7c4ba9472737aedea130e14bbd4f5b95e5832/-/blob/glfw/src/Window.zig?L2777:77)

emidoots commented

2022-06-28 17:51:58 +00:00

(Migrated from github.com)

the c function segfaults on wayland

I suspect this is more undefined behavior being caught by Zig/UBSan, I can dig into it later. Presumably it works if you do a release-fast build

> the c function segfaults on wayland I suspect this is more undefined behavior being caught by Zig/UBSan, I can dig into it later. Presumably it works if you do a release-fast build

PiergiorgioZagaria commented

2022-06-28 17:54:07 +00:00

(Migrated from github.com)

What do we do about test 49? Since it segfaults there isn't anything to catch

emidoots commented

2022-06-28 18:00:32 +00:00

(Migrated from github.com)

Let's just comment it out for now and add:

// TODO: https://github.com/hexops/mach/issues/375

Let's just comment it out for now and add: ``` // TODO: https://github.com/hexops/mach/issues/375 ```

emidoots (Migrated from github.com) approved these changes

2022-06-29 04:49:07 +00:00

emidoots (Migrated from github.com) left a comment

Looks good, thanks a ton again for doing this!

emidoots commented

2022-06-29 04:53:42 +00:00

(Migrated from github.com)

CI shows this is failing to cross-compile from macOS/Windows:

It looks like linking to wayland-client is no longer necessary, though, because GLFW loads it at runtime now. I am pretty sure we can just remove that line entirely from build.zig?

CI shows this is failing to cross-compile from macOS/Windows: <img width="1256" alt="image" src="https://user-images.githubusercontent.com/3173176/176354314-7286e78a-f6c0-43bb-baf4-3c984105b2a8.png"> It looks like linking to `wayland-client` is no longer necessary, though, because GLFW loads it at runtime now. I am pretty sure we can just remove that line entirely from build.zig?

emidoots (Migrated from github.com) approved these changes

2022-06-30 03:53:54 +00:00

emidoots (Migrated from github.com) left a comment

LGTM, thanks! I did a git rebase -i origin/main and force pushed to clean up the history a bit, hope that's okay.

Awesome work!

LGTM, thanks! I did a `git rebase -i origin/main` and force pushed to clean up the history a bit, hope that's okay. Awesome work!