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

vendor:vulkan Add support for xlib and xcb #4639

Merged
merged 3 commits into from
Jan 5, 2025

Conversation

flga
Copy link
Contributor

@flga flga commented Dec 30, 2024

We need forward declarations for the WSI types, but xlib doesn't use any prefixes.
To make it clear and consistent with the other WSI types, I've added the Xlib prefix:

Display => XlibDisplay
Window => XlibWindow
VisualID => XlibVisualID

However, one could argue keeping the names consistent with xlib would be better, so let me know your thoughts on that.

@laytan
Copy link
Collaborator

laytan commented Dec 30, 2024

Could you not use vendor:x11/xlib for the types?

@flga
Copy link
Contributor Author

flga commented Dec 31, 2024

Done.

I thought that importing x11/xlib would imply xlib gets linked in, but anecdotally that seems to not be the case, the dependency gets elided if no procedures are called.

Can this be relied on will things like reflect get in the way of this "optimization"?

xlib has build constraints that have to be either matched or lifted. I chose the former for no other reason than trying to keep it consistent with win32.
This does mean that they need to be kept in sync but I suspect that's not a big deal.

PS: there's a 3rd option here, which would be to match vulkan and separate out WSI stuff into independent sub packages, but that's both breaking and more intrusive

@laytan
Copy link
Collaborator

laytan commented Jan 5, 2025

Yeah foreign dependencies don't get linked if none of their procs are called.

vendor/vulkan/structs.odin Outdated Show resolved Hide resolved
@laytan laytan merged commit f566c5e into odin-lang:master Jan 5, 2025
7 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.

2 participants