Fix x264 build and GNOME window decorations, linux#1128
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses two Linux-related issues: improving SDL window decorations under GNOME/Wayland by preferring X11/Xwayland when appropriate, and adding a custom vcpkg overlay for x264 to resolve build/versioning issues.
Changes:
- Add GNOME-on-Wayland detection and prefer X11/Xwayland via SDL video-driver hinting; also log the chosen SDL video driver.
- Update build documentation examples to show
--helpand a more realistic-d/-oinvocation. - Introduce a vcpkg overlay port for x264 (portfile + patches) to stabilize builds across platforms.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/visualizer/window/window_manager.cpp | Detect GNOME Wayland sessions and set SDL driver preference to improve window decorations; log selected SDL driver. |
| docs/building_and_distribution.md | Update example invocations for native and portable runs. |
| .github/overlays/x264/vcpkg.json | Define an x264 overlay port manifest (features, defaults, metadata). |
| .github/overlays/x264/portfile.cmake | Implement the x264 overlay build logic via vcpkg_from_gitlab and vcpkg_make_configure. |
| .github/overlays/x264/version.diff.in | Patch x264 version.sh to embed vcpkg revision/ref info. |
| .github/overlays/x264/configure.patch | Patch x264 configure to be more portable and respect CPPFLAGS/CFLAGS in checks. |
| .github/overlays/x264/allow-clang-cl.patch | Allow clang-cl to be treated like MSVC cl in configure detection. |
| .github/overlays/x264/parallel-install.patch | Adjust install target generation to be more parallel-safe. |
| .github/overlays/x264/srcpath-relative-path.patch | Fix relative path computation logic in x264 configure for certain layouts. |
| .github/overlays/x264/uwp-cflags.patch | Remove forced -MD for WinRT/UWP builds in x264 configure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # With a git clone, this can be determined by running `versions.sh`. | ||
| # With vcpkg_from_gitlab, we modify `versions.sh` accordingly. |
There was a problem hiding this comment.
The x264 versioning comment refers to versions.sh, but the script being patched/used is version.sh. This is likely to confuse future maintainers; update the comment to the correct script name (and wording if needed).
| # With a git clone, this can be determined by running `versions.sh`. | |
| # With vcpkg_from_gitlab, we modify `versions.sh` accordingly. | |
| # With a git clone, this can be determined by running `version.sh`. | |
| # With vcpkg_from_gitlab, we modify `version.sh` accordingly. |
|
|
||
| vcpkg_make_configure( | ||
| SOURCE_PATH "${SOURCE_PATH}" | ||
| DISABLE_CPPFLAGS # Build is not using CPP/CPPFLAGS |
There was a problem hiding this comment.
vcpkg_make_configure() is called with DISABLE_CPPFLAGS and a comment that the build isn't using CPPFLAGS, but this port also applies configure.patch which explicitly prepends ${CPPFLAGS} to CFLAGS/ASFLAGS. Consider removing DISABLE_CPPFLAGS (or updating the patch/portfile) so CPPFLAGS from the toolchain actually flow into the build as intended.
| DISABLE_CPPFLAGS # Build is not using CPP/CPPFLAGS |
| REPO videolan/x264 | ||
| REF "${ref}" | ||
| SHA512 707ff486677a1b5502d6d8faa588e7a03b0dee45491c5cba89341be4be23d3f2e48272c3b11d54cfc7be1b8bf4a3dfc3c3bb6d9643a6b5a2ed77539c85ecf294 | ||
| HEAD_REF master |
There was a problem hiding this comment.
The portfile comment says the pinned ref is from the stable branch, but vcpkg_from_gitlab() uses HEAD_REF master. If --head builds are expected to track stable, consider setting HEAD_REF stable (or adjust the comment to match the intended branch).
| HEAD_REF master | |
| HEAD_REF stable |
|
Thanks for contributing. We had some discussions here: #1070 |
Thanks! Will take a look at that discussion. |
|
Please split this pr. The window-manager/docs change and the x264 overlay are independent. The former is fine. I think we should not introduce custom overlays. This issue should be fixed upstream as it is not an LFS issue. |
Ubuntu 25.10 x86_64
GNOME