Vulkan-Samples icon indicating copy to clipboard operation
Vulkan-Samples copied to clipboard

Update validation layer

Open JoseEmilio-ARM opened this issue 1 year ago • 3 comments

Description

The current default version of the validation layers have a bug that prints an incorrect error message in a new sample I am working on. This change updates the template so that we fetch the most recent version of the layers available today (with this, and possibly a few other bugs, fixed).

These changes only affect the template, so for them to take effect in an existing Android build, you need to follow the instructions in https://github.com/KhronosGroup/Vulkan-Samples/blob/670f52353dc9cc422ab535fa5b66d2eeb04a70db/bldsys/cmake/template/gradle/download_vvl.gradle#L29-L32

Note that the Validation Layers repository does not consistently name their binaries or URL paths, so those had to be updated as well, and might need to be changed again in the future.

General Checklist:

Please ensure the following points are checked:

  • [x] My code follows the coding style
  • [x] I have reviewed file licenses
  • [x] I have commented any added functions (in line with Doxygen)
  • [x] I have commented any code that could be hard to understand
  • [x] My changes do not add any new compiler warnings
  • [x] My changes do not add any new validation layer errors or warnings
  • [x] I have used existing framework/helper functions where possible
  • [x] My changes do not add any regressions
  • [x] I have tested every sample to ensure everything runs correctly
  • [x] This PR describes the scope and expected impact of the changes I am making

Note: The Samples CI runs a number of checks including:

  • [x] I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
  • [x] My changes build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions

JoseEmilio-ARM avatar Feb 22 '24 17:02 JoseEmilio-ARM

bldsys/cmake/template/gradle/app.build.gradle.in is already a template that gets configured here. If you changed bldsys/cmake/template/gradle/download_vvl.gradle to also be a template then in the future you'd only need to make changes to create_gradle_project.cmake to update the validation layer version.

jherico avatar Feb 22 '24 19:02 jherico

This is an Android-only change, so I can't test it I'm afraid.

gary-sweet avatar Feb 23 '24 14:02 gary-sweet

bldsys/cmake/template/gradle/app.build.gradle.in is already a template that gets configured here. If you changed bldsys/cmake/template/gradle/download_vvl.gradle to also be a template then in the future you'd only need to make changes to create_gradle_project.cmake to update the validation layer version.

Hi @jherico , I looked into this, but I think it might not be worth having a template for download_vvl.gradle, because this is the only parameter that needs adjusting, and in reality it is never used, it is only a fallback, if the caller of the script does not define their own version:

https://github.com/KhronosGroup/Vulkan-Samples/blob/670f52353dc9cc422ab535fa5b66d2eeb04a70db/bldsys/cmake/template/gradle/download_vvl.gradle#L39-L42

Perhaps we could remove this safeguard, to make it more explicit that the caller (build.gradle) needs to provide a layer version?

Also they don't need to always match. Ideally I would not have needed to change the fallback version, but unfortunately the download URL changed and did not work with the previous version anymore 😞

JoseEmilio-ARM avatar Feb 23 '24 14:02 JoseEmilio-ARM