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

layer manifest file format documentation does not match layers_schema.json #1756

Closed
Waffl3x opened this issue Oct 20, 2022 · 2 comments
Closed
Assignees

Comments

@Waffl3x
Copy link

Waffl3x commented Oct 20, 2022

The layers manifest documentation located here does not appear to match the layers_schema.json located here.
My specific issue is that it is not specified if the json object with name "layer" is optional, but there is an example that uses the "layers" node that lacks a "layer" node. The layers_schema.json appears to require a "layer" node despite this. If I am mistaken in my interpretation of the layers schema please let me know.
I request that the documentation is updated to be more specific in it's requirements, and if there's a more detail spec somewhere please let me know as I couldn't find one.

If it is that "layer" is mandatory and "layers" is optional, I also would like to know the correct way of handling it. In the Vulkan-Loader source code it's handled by checking first for a "layers" node, and falling back to checking for a "layer" node, however it also supports other violations so I would like clarification on what is actually required.

I intend to use the layers_schema to validate discovered layers, so I would prefer it doesn't fail on a layer manifest that is legal.
I recognize this isn't necessarily the right place to post this issue, but I figure that if the documentation is correct as is, the layers_schema.json needs to be changed to reflect it. Or perhaps I'm just reading the layers_schema.json wrong and there is more than one description of requirements. The lines in the schema that I am referring to can be found here.

edit:
The documentation states this here:

The ability to define multiple layers using the "layers" array was added. This JSON array field can be used when defining a single layer or multiple layers. The "layer" field is still present and valid for a single layer definition.

Given the behavior of the loader, I feel like this description is inaccurate, but at the very least, the schema should reflect that "layer" object is optional when a "layers" object is present.

edit2:
I have just noticed this comment in the loader source code

    // The following Fields in layer manifest file that are required:
    //   - "file_format_version"
    //   - If more than one "layer" object are used, then the "layers" array is
    //     required

this leads me to believe that it's acknowledged that "layer" and "layers" are mutually exclusive, it just needs to be formalized.

@charles-lunarg
Copy link
Contributor

This should be resolved by this PR in the Vulkan-Loader

My intent is to combine the two schema's (layer_schema.json and the ones I devised that fully describe the layer manifests as far as the loader is concerned).

Doing that should allow the layer_schema.json in this repo to be removed, since it is redundant.

@christophe-lunarg
Copy link
Contributor

I am closing this as it's being resolved in the Vulkan Loader repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants