Skip to content

General improvements #48

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

Merged
merged 5 commits into from
May 2, 2018
Merged

General improvements #48

merged 5 commits into from
May 2, 2018

Conversation

sjfricke
Copy link
Contributor

@sjfricke sjfricke commented Apr 24, 2018

  • removed redundent setting .allocationSize
  • added VK_MEMORY_PROPERTY_HOST_COHERENT_BIT to vertex memory
    • Most android devices has HOST_VISIBLE memory that isn't COHERENT
  • vkMapMemory with allocation size instead of vector/struct size
  • Vulkan Debug Message -> Vulkan-Debug-Message so it can be regex search in logcat easier
  • Closed all AAsset* with AAsset_close

sjfricke added 5 commits April 22, 2018 08:50
From the Vulkan spec we need to add HOST_COHERENT as it says: If a memory object does not have the VK_MEMORY_PROPERTY_HOST_COHERENT_BIT property, then vkFlushMappedMemoryRanges must be called in order to guarantee that writes to the memory object from the host are made visible to the VK_ACCESS_HOST_WRITE_BIT access type, where it can be further made available to the device by synchronization commands
The good practice is to map allocation size, this is also done for the texutre data so keepit consistent across all map memory
@ggfan ggfan merged commit b741014 into googlesamples:master May 2, 2018
@ggfan
Copy link
Contributor

ggfan commented May 2, 2018

thanks for the PR. The changes around mem_req are needed indeed; the validation layer handling is outdated: Vulkan merged device and instance layers ( I am pretty sure about it ) so there is no need to do device layers anymore. You may try to delete the device layer handling and confirm it still would work.
Thank you!

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