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

Added an option to get the current vk instance #292

Merged
merged 1 commit into from
Jun 21, 2022

Conversation

COM8
Copy link
Contributor

@COM8 COM8 commented Jun 21, 2022

In order to debug compute shaders using RenderDoc and its in application API, one needs access to the vk::Instance object held by the kp::Manager. This PR introduces a new getter for retrieving the std::shared_ptr<vk::Instance> via the Manager::getVkInstance()call.

@axsaucedo
Copy link
Member

Nice one @COM8 - u was actually thinking of doing a codebase refractor that ensures "open by default" and exposes all underlying vulkan components and likewise allows to use external Vulkan components - one request that has come up in the past is to initialise vkbuffers with external tensors. Will open an issue for this.

PR looks good - seems you need to sign commits and we'll be able to merge

Copy link
Member

@axsaucedo axsaucedo left a comment

Choose a reason for hiding this comment

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

Also added a small comment

src/Manager.cpp Outdated Show resolved Hide resolved
@COM8
Copy link
Contributor Author

COM8 commented Jun 21, 2022

Yes, this is something I was thinking about as well since it would allow one to extend the functionality of kompute by accessing raw Vulkan objects if needed and if there is no alternative available in kompute.

Looks like pulling the docker image failed for the cpp-tests.

Signed-off-by: Fabian Sauter <sauter.fabian@mailbox.org>
@axsaucedo
Copy link
Member

OK sounds like something that's something worth exploring - I'll put together a design doc and thsi can be something interesting to do iteratively once your current pr is merged

Strange, seems it now worked with the rerun - merging

@axsaucedo axsaucedo self-requested a review June 21, 2022 07:14
@axsaucedo axsaucedo merged commit 5dc47b1 into KomputeProject:master Jun 21, 2022
@COM8 COM8 deleted the get_instance branch August 16, 2022 10:28
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