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: Add build target for vkcube-display #1027

Closed
wants to merge 1 commit into from

Conversation

enunes
Copy link

@enunes enunes commented Sep 3, 2024

Add a new CMake target for vkcube-display, which the variant using VK_KHR_display.
This is a variant which already exists, but is currently hard to build as only one variant can be selected per build using CUBE_WSI_SELECTION. With this option, it is possible to build this variant in addition to other ones, similar to what is done for vkcube-wayland.

As of now this only takes effect in Linux (same as the wayland one below it) as this is the only platform I'm able to test on.

Add a new CMake target for vkcube-display, which the variant using
VK_KHR_display.
This is a variant which already exists, but is currently hard to build
as only one variant can be selected per build using CUBE_WSI_SELECTION.
With this option, it is possible to build this variant in addition to
other ones, similar to what is done for vkcube-wayland.

Signed-off-by: Erico Nunes <[email protected]>
@ci-tester-lunarg
Copy link

Author enunes not on autobuild list. Waiting for curator authorization before starting CI build.

1 similar comment
@ci-tester-lunarg
Copy link

Author enunes not on autobuild list. Waiting for curator authorization before starting CI build.

@CLAassistant
Copy link

CLAassistant commented Sep 3, 2024

CLA assistant check
All committers have signed the CLA.

@charles-lunarg
Copy link
Contributor

I want to state that while this PR is welcome, the solution is more of a band-aid than a long term fix. The root cause is vkcube being unable to choose the WSI platform at runtime.
I am already working on a PR which enables selection of xcb, xlib, wayland, directfb, and this PR's VK_KHR_display. I'd appreciate your review when I publish it in the coming days. I have the loading all happening, the last bit of work is polishing the selection logic to handle automatically choose an available WSI platform.

@enunes
Copy link
Author

enunes commented Sep 5, 2024

I agree that a refactor is better either by reworking the CMake logic or choosing the WSI at runtime. If you're already working on it then that will be better.

So I can help review the new PR when it arrives and maybe we can drop this one.
In the end all I'm looking for is a way to ship this cleanly in distributions so I don't have to build it over and over for a quick check for development on new installs. If the new PR would still take a long time, then I would still be interested in having this as a way to build this for distributions in the meanwhile.

@charles-lunarg
Copy link
Contributor

Closing in favor of #1029

@charles-lunarg
Copy link
Contributor

In the end all I'm looking for is a way to ship this cleanly in distributions so I don't have to build it over and over for a quick check for development on new installs. If the new PR would still take a long time, then I would still be interested in having this as a way to build this for distributions in the meanwhile.

That is exactly the reason that vkcube should have supported multiple WSI platforms at the same time, but alas it was easier to just copy-paste the cmake code and tweak the build than it was to actually make it work.
It has irked me that vkcube didn't handle that, so thank you for making this PR in order to encourage me to fix the underlying problem :D

@enunes
Copy link
Author

enunes commented Sep 10, 2024

@charles-lunarg Thanks! I'll test the new PR and post some review feedback there shortly.

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