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

schema: Add layer settings schema for manifest validation #174

Closed
wants to merge 2 commits into from

Conversation

christophe-lunarg
Copy link
Contributor

@christophe-lunarg christophe-lunarg commented Dec 12, 2023

To go with the layer setting library, this file allows adding a test to validate the layer setting manifest is correct.

@charles-lunarg
Copy link
Collaborator

Why are we adding a schema to this repo that ostensibly looks copy pasted from VulkanTools instead of adding it to https://schema.khronos.org/vulkan/?

I know I'm a bit late on finishing the layer & driver schemas that also contain loader information, but I question if putting the schema here is the right choice.

@christophe-lunarg
Copy link
Contributor Author

Why are we adding a schema to this repo that ostensibly looks copy pasted from VulkanTools instead of adding it to https://schema.khronos.org/vulkan/?

I know I'm a bit late on finishing the layer & driver schemas that also contain loader information, but I question if putting the schema here is the right choice.

The idea is that part of this repository all layers will be able to pick up this schema and validate in a unit test their layer manifest.

Adding a Vulkan Tools dependence seems wrong for that. I am also planning in removing the copy in Vulkan Tools.

@charles-lunarg
Copy link
Collaborator

No my point is that you don't need to copy the schema everywhere if its located at a web URL

Not to mention most schema validators expect the schema to be available with a URL, making validating a given schema as simple as opening it in the validator.

@christophe-lunarg
Copy link
Contributor Author

christophe-lunarg commented Dec 12, 2023

Ermmmmmm what C++ validator take a URL? Not valijson that I am using. Not Python jsonschema either. Tell me how to do this if you have a solution.

And again the idea is not to copy the file everywhere, it's to move the file here so that it's available with our build system and dependences.

@charles-lunarg
Copy link
Collaborator

To comment what I said in a PM

I am mistaken about validation tools expecting or even using URL's for sources of schema's. While there is the concept of a URI in the schema itself, that is more for the schema referencing other schemas instead of the validator loading the schema from a URL. That, plus the validator extension in vscode I used was able to load schemas from online, caused my confusion.

That said, where the schema comes from isn't as relevant. Because all of our repos grab their dependencies through the internet, it stands to reason that our tests can load the schemas from https://schema.khronos.org/ instead of adding it to every repo. https://github.com/KhronosGroup/Khronos-Schemas is where these schemas are stored.

@christophe-lunarg
Copy link
Contributor Author

If we want to store multiple revisions of the layer settings manifest maybe. That repository was created to interface with James to update the multiple revision of the Profiles schema. Does the profiles repository use that storage? No.
It is useful for someone to grab these profiles yes, for example to figure out that Vulkan Header version is required by a profile.

On that present case :

  • Do we want to regularly upload layer manifest schema? no
  • Do the user of the layer setting library need to use different revisions of the layer manifest schema? no. The user only need to use the schema that match the behavior of the library so the same commit might be handy for that instead of figuring out what the correct revision.
  • Can we see value in adding the layer manifest revisions to https://schema.khronos.org/? Yes to document the new features in the schema and provide historisity.

The aspect were I can see a difficulty is that the Vulkan Loader could also benefit from having a JSON schema as it defines most of the specification of the layer manifest. We already encounter in the past this difficulty and I was thinking that the layer settings part should be versioned separately from the main block and be specified as undocumented and reserve for the layer settings in the loader layer manifest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants