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

Fixing Wayland cmake definitions and linkage #250

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

mtytel
Copy link

@mtytel mtytel commented Oct 3, 2024

In newer versions of cmake this line triggers a CMP0127 warning. Removing the quotes because they're not needed in older versions of cmake and doesn't trigger the warning in newer version of cmake.

@mtytel mtytel changed the title Fixing CMP0127 policy warning Fixing Wayland cmake definitions and linkage Oct 4, 2024
@mtytel
Copy link
Author

mtytel commented Oct 4, 2024

So 'Linux' actually wasn't matching CMAKE_SYSTEM_NAME and removing the single quotes makes it match so it turned on WAYLAND by default in the build above. That exposed some missing definitions and library linkage so I've fixed that here.

I also turned off wayland by default which is how it was behaving originally (because it never matched 'Linux')

@Gaspard--
Copy link

Disclaimer: not a maintainer.

So 'Linux' actually wasn't matching CMAKE_SYSTEM_NAME and removing the single quotes makes it match so it turned on WAYLAND by default in the build above. That exposed some missing definitions and library linkage so I've fixed that here.

I also turned off wayland by default which is how it was behaving originally (because it never matched 'Linux')

I think wayland is supposed to be turned on by default: #234
I haven't updated bgfx.cmake to test, so maybe #232 wasn't actually fixed. This would also explain #249

@bwrsandman
Copy link
Collaborator

There was a lot of push recently in the distro and DE space this year to ditch XOrg finally. I would prefer to keep Wayland as default as long as it's working.

@mtytel
Copy link
Author

mtytel commented Oct 13, 2024

It actually wasn't enabled by default before, it just looked like it was.
I turned it on by default it the last couple commits though and added some installation requirements for CI.

@mtytel mtytel force-pushed the master branch 2 times, most recently from 46ff81a to aa8d5cc Compare October 19, 2024 17:29
@mtytel
Copy link
Author

mtytel commented Oct 19, 2024

Alright squashed my changes.

It looks like the bgfx.cmake's bgfx amalgamated build doesn't work with wayland enabled but it compiles on newer bgfx. So I could update the bgfx ref.

@mtytel
Copy link
Author

mtytel commented Oct 25, 2024

I bumped bgfx version so the amalgamated build with wayland enabled works. CI is passing in my fork.

@mtytel
Copy link
Author

mtytel commented Nov 5, 2024

@bwrsandman this is ready to pull in.

Copy link
Collaborator

@bwrsandman bwrsandman left a comment

Choose a reason for hiding this comment

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

Will merge this after dependabot merges your spriv-opt fix

@bwrsandman bwrsandman merged commit b135fa3 into bkaradzic:master Nov 5, 2024
13 checks passed
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.

3 participants