Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add install target and integration tests #8

Closed
wants to merge 1 commit into from

Conversation

aqnuep
Copy link

@aqnuep aqnuep commented May 23, 2024

This solution is far from perfect, as the project lacks a proper directory structure, but at least allows for using the repository as a dependency in ecosystem components.

If preferred, we can rework the directory structure too.

@aqnuep aqnuep requested a review from dgkoch May 23, 2024 15:09
@aqnuep aqnuep force-pushed the add-cmake-install-target branch 2 times, most recently from 95578b1 to b6b4769 Compare May 23, 2024 15:10
@aqnuep aqnuep force-pushed the add-cmake-install-target branch from b6b4769 to b2684d6 Compare May 23, 2024 15:12
@aqnuep aqnuep marked this pull request as draft May 23, 2024 15:17
@aqnuep
Copy link
Author

aqnuep commented May 23, 2024

I spoke too soon, I just realized that this code depends on the vulkan_sc_core.hpp, i.e. the C++ header, which is quite unfortunate, because it makes it incompatible with the ecosystem components in general, which do not generate combined C++ headers of any sort.

@dgkoch
Copy link
Collaborator

dgkoch commented May 23, 2024

I spoke too soon, I just realized that this code depends on the vulkan_sc_core.hpp, i.e. the C++ header, which is quite unfortunate, because it makes it incompatible with the ecosystem components in general, which do not generate combined C++ headers of any sort.

Does it really though? All it really needs is what's defined in fake/vulkan/vulkan_sc_core.hpp which is just the pipeline cache structures, which should be the same as those in the C header.

@aqnuep
Copy link
Author

aqnuep commented May 23, 2024

I guess where I'm trying to get to is that probably we should do some more intrusive changes and put together something more robust, one of them being the chamge to the used includes, but sure, you're right.

@dgkoch
Copy link
Collaborator

dgkoch commented May 23, 2024

I don't mind if you want to do it more robustly. I made the fake header thing long before we had the VulkanSC-Headers repo that we could pull in easily.

@aqnuep
Copy link
Author

aqnuep commented May 31, 2024

Closing this MR for now, we will figure out the long-term plan for this first.

@aqnuep aqnuep closed this May 31, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants