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

Add get_pipeline_cache_data #166

Merged
merged 1 commit into from
Dec 4, 2018
Merged

Conversation

zakorgy
Copy link
Contributor

@zakorgy zakorgy commented Dec 4, 2018

This change is Reviewable

@MaikKlein
Copy link
Member

I think we should just return a Vec<u8> here for convenience. Although the spec says:

Any data written to pData is valid and can be provided as the pInitialData member of the VkPipelineCacheCreateInfo structure passed to vkCreatePipelineCache.

I am not sure when you would want to only partially retrieve the pipeline cache. @Ralith Any thoughts?

@zakorgy
Copy link
Contributor Author

zakorgy commented Dec 4, 2018

@MaikKlein I have updated the PR according to your advice. I can squash the commits if you want.

pipeline_cache: vk::PipelineCache,
) -> VkResult<Vec<u8>> {
let data_size: *mut usize = mem::uninitialized();
let data: *mut c_void = mem::uninitialized();
Copy link
Member

Choose a reason for hiding this comment

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

I think you should initialize them explicitly to a null ptr.

);

match err_code {
vk::Result::SUCCESS => Ok(Vec::from_raw_parts(data as _, *data_size, *data_size)),
Copy link
Member

Choose a reason for hiding this comment

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

You have to allocate the buffer to data yourself. You can do this with Vec::with_capacity and set_len. And you need to call get_pipeline_cache_data two times to get the size, and then to fill the buffer.

@@ -1281,27 +1281,25 @@ pub trait DeviceV1_0 {
pipeline_cache: vk::PipelineCache,
) -> VkResult<Vec<u8>> {
let data_size: *mut usize = ptr::null_mut();
Copy link
Member

@MaikKlein MaikKlein Dec 4, 2018

Choose a reason for hiding this comment

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

Sorry I overlooked that the first time. You probably want to allocate that on the stack like let mut data_size = 0 and then write to it with &mut data_size

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

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

Thanks, I've been wanting this!

@MaikKlein MaikKlein merged commit 8d67600 into ash-rs:master Dec 4, 2018
@kvark
Copy link
Collaborator

kvark commented Jan 10, 2019

@MaikKlein any chance of releasing a version with this?

@MaikKlein
Copy link
Member

@kvark Oh I can't believe I haven't pushed a new version. I fix up a few PRs and I publish a new version later today.

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