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 tests for VK_EXT_conditional_rendering #131

Merged

Conversation

werman
Copy link
Contributor

@werman werman commented Oct 17, 2018

All functions affected by VK_EXT_conditional_rendering are tested:

  • vkCmdDraw, vkCmdDrawIndexed, vkCmdDrawIndirect, vkCmdDrawIndexedIndirect
  • vkCmdDrawIndirectCountKHR, vkCmdDrawIndexedIndirectCountKHR (VK_KHR_draw_indirect_count)
  • vkCmdDispatch, vkCmdDispatchIndirect, vkCmdDispatchBase
  • vkCmdClearAttachments

The only limitation of these tests is that combination of different functions in one render
pass are not tested - only several calls of the same functions. Due to the different hardware
specific support of conditional rendering some implementations may have exhibit issues in such cases,
especially when combining conditional rendering with functions from VK_KHR_draw_indirect_count.

I have run the tests on AMD GPU with Mesa and on Intel GPUs with Mesa (in this case I have tested my implementation of this extension). Unfortunately there wasn't a Windows installation in my vicinity to run the tests.

@CLAassistant
Copy link

CLAassistant commented Oct 17, 2018

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@alegal-arm alegal-arm left a comment

Choose a reason for hiding this comment

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

Please run python scripts/check_build_sanity.py -r check-all and fix the issues

@werman werman force-pushed the vulkan/VK_EXT_conditional_rendering branch 2 times, most recently from b5f4264 to 4c47606 Compare October 18, 2018 08:57
@werman werman force-pushed the vulkan/VK_EXT_conditional_rendering branch from 4c47606 to c0ea855 Compare October 19, 2018 10:04
Copy link
Contributor

@vkushwaha-nv vkushwaha-nv left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good.

@alegal-arm
Copy link
Contributor

In the internal review now

3 similar comments
@alegal-arm
Copy link
Contributor

In the internal review now

@alegal-arm
Copy link
Contributor

In the internal review now

@alegal-arm
Copy link
Contributor

In the internal review now

@werman werman force-pushed the vulkan/VK_EXT_conditional_rendering branch from c0ea855 to 8890e20 Compare October 30, 2018 10:42
@werman werman force-pushed the vulkan/VK_EXT_conditional_rendering branch from 8890e20 to a46acfe Compare October 30, 2018 17:04
All functions affected by VK_EXT_conditional_rendering are tested:
 - vkCmdDraw, vkCmdDrawIndexed, vkCmdDrawIndirect, vkCmdDrawIndexedIndirect
 - vkCmdDrawIndirectCountKHR, vkCmdDrawIndexedIndirectCountKHR (VK_KHR_draw_indirect_count)
 - vkCmdDispatch, vkCmdDispatchIndirect, vkCmdDispatchBase
 - vkCmdClearAttachments

The only limitation of these tests is that combination of different functions in one render
pass are not tested - only several calls of the same functions. Due to the different hardware
specific support of conditional rendering some implementations may have exhibit issues in such cases,
especially when combining conditional rendering with functions from VK_KHR_draw_indirect_count.

Components: Vulkan

New Tests: dEQP-VK.conditional_rendering.*

Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
@werman werman force-pushed the vulkan/VK_EXT_conditional_rendering branch from a46acfe to 2126307 Compare October 31, 2018 11:20
@alegal-arm alegal-arm merged commit 54e546a into KhronosGroup:master Nov 6, 2018
@werman
Copy link
Contributor Author

werman commented Nov 6, 2018

Thank you for all your help!

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.

None yet

5 participants