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.

Sign up for GitHub

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

Fix glTF property reference details #88

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix glTF property reference details #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10da1d3
Include minProperties and maxProperties in reference
javagl Oct 10, 2023
6cc6d00
Address wrong type stated for extension properties
javagl Oct 10, 2023
23648dd
Update reference files for new output
javagl Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lib/generateMarkdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,14 +400,21 @@ function createPropertiesDetails(schema, title, headerLevel, knownTypes, autoLin
if (defined(additionalPropertiesType)) {
// TODO: additionalProperties is really a full schema
let formattedType = style.typeValue(additionalPropertiesType);
if ((additionalProperties.type === 'object') && defined(property.title)) {
formattedType = style.linkType(property.title, property.title, autoLink);
if ((additionalProperties.type === 'object') && defined(additionalProperties.title)) {
formattedType = style.linkType(additionalProperties.title, additionalProperties.title, autoLink);
}

md += style.bulletItem(`${style.propertyDetails('Type of each property') }: ${ formattedType}`, 0);
}
}

if (defined(property.minProperties)) {
md += style.bulletItem(`${style.propertyDetails('Minimum number of properties') }: ${ property.minProperties}`, 0);
}
if (defined(property.maxProperties)) {
md += style.bulletItem(`${style.propertyDetails('Maximum number of properties') }: ${ property.maxProperties}`, 0);
}

const examples = property.examples;
if (defined(examples)) {
md += style.bulletItem(`${style.propertyDetails('Examples') }:`);
Expand Down
10 changes: 5 additions & 5 deletions test/test-golden/nested-embed.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure we want this change? The glTF extensions aren't just simple JSON objects, they do follow the Extension schema, which is what enables nesting. (For example, the "clearcoat" extension may have a "texture transform" extension nested inside of it).

Copy link
Contributor

Choose a reason for hiding this comment

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

Also it explicitly allows extras inside of extensions.

Copy link
Contributor Author

@javagl javagl Oct 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe I got confused here at some point, or am overlooking something.

An extension object is defined as a plain JSON object, as of the extension.schema.json. It is only an object, without additional constraints for its properties. Its additionalProperties are defined to be object, too.

From what you said about 'nesting', it sounds like the values in the extension dictionary should be constrained to be ~"something that can (optionally) contain another 'extensions' property" (and maybe an optional 'extras' property). So the values could be required to be a glTFProperty, but this 1. does not seem to be the case, and 2. (from the tip of my head) would not change anything in the meaning, compared to just using object - or does it?

Copy link
Contributor

Choose a reason for hiding this comment

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

You might be right. I've always been under the impression that the extensions dictionary object contains a dictionary of named Extension type objects, but it's been a while since I investigated this and I'm not sure.

Maybe @lexaknyazev can double check? This is a far-reaching change because every type of object in a glTF has an extensions dictionary on it, and I feel like the type of entry in that dictionary has changed here. At the very least, it will produce a lot of diffs across the entire section 5 "Properties Reference."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the nitpicking level, some confusion might be caused by the fact that the type of the extensions property is defined in the extension.schema.json. The schema actually does define the structure of the extensions property (and we might consider renaming that file, if this doesn't cause too much trouble - at least, it would match the name of the extras (not extra) schema file...)


=== bufferView.extras

Expand Down Expand Up @@ -235,7 +235,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== image.extras

Expand Down Expand Up @@ -316,7 +316,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== material.extras

Expand Down Expand Up @@ -442,7 +442,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== material.pbrMetallicRoughness.extras

Expand Down Expand Up @@ -548,7 +548,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== nestedTest.extras

Expand Down
10 changes: 5 additions & 5 deletions test/test-golden/nested-keyword.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### bufferView.extras

Expand Down Expand Up @@ -179,7 +179,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### image.extras

Expand Down Expand Up @@ -225,7 +225,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### material.extras

Expand Down Expand Up @@ -328,7 +328,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### material.pbrMetallicRoughness.extras

Expand Down Expand Up @@ -403,7 +403,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### nestedTest.extras

Expand Down
10 changes: 5 additions & 5 deletions test/test-golden/nested-linked.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

==== bufferView.extras

Expand Down Expand Up @@ -243,7 +243,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

==== image.extras

Expand Down Expand Up @@ -324,7 +324,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

==== material.extras

Expand Down Expand Up @@ -450,7 +450,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

==== material.pbrMetallicRoughness.extras

Expand Down Expand Up @@ -556,7 +556,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

==== nestedTest.extras

Expand Down
10 changes: 5 additions & 5 deletions test/test-golden/nested-linked.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

#### bufferView.extras

Expand Down Expand Up @@ -185,7 +185,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

#### image.extras

Expand Down Expand Up @@ -233,7 +233,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

#### material.extras

Expand Down Expand Up @@ -338,7 +338,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

#### material.pbrMetallicRoughness.extras

Expand Down Expand Up @@ -415,7 +415,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

#### nestedTest.extras

Expand Down
10 changes: 5 additions & 5 deletions test/test-golden/nested-remote.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== bufferView.extras

Expand Down Expand Up @@ -235,7 +235,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== image.extras

Expand Down Expand Up @@ -316,7 +316,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== material.extras

Expand Down Expand Up @@ -442,7 +442,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== material.pbrMetallicRoughness.extras

Expand Down Expand Up @@ -548,7 +548,7 @@ Dictionary object with extension-specific objects.

* **Type**: <<reference-extension,`extension`>>
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== nestedTest.extras

Expand Down
10 changes: 5 additions & 5 deletions test/test-golden/nested-remote.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### bufferView.extras

Expand Down Expand Up @@ -177,7 +177,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### image.extras

Expand Down Expand Up @@ -225,7 +225,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### material.extras

Expand Down Expand Up @@ -330,7 +330,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### material.pbrMetallicRoughness.extras

Expand Down Expand Up @@ -407,7 +407,7 @@ Dictionary object with extension-specific objects.

* **Type**: [`extension`](#reference-extension)
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### nestedTest.extras

Expand Down
10 changes: 5 additions & 5 deletions test/test-golden/nested-simple.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== bufferView.extras

Expand Down Expand Up @@ -237,7 +237,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== image.extras

Expand Down Expand Up @@ -316,7 +316,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== material.extras

Expand Down Expand Up @@ -440,7 +440,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== material.pbrMetallicRoughness.extras

Expand Down Expand Up @@ -544,7 +544,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

=== nestedTest.extras

Expand Down
10 changes: 5 additions & 5 deletions test/test-golden/nested-simple.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### bufferView.extras

Expand Down Expand Up @@ -179,7 +179,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### image.extras

Expand Down Expand Up @@ -225,7 +225,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### material.extras

Expand Down Expand Up @@ -328,7 +328,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### material.pbrMetallicRoughness.extras

Expand Down Expand Up @@ -403,7 +403,7 @@ Dictionary object with extension-specific objects.

* **Type**: `extension`
* **Required**: No
* **Type of each property**: Extension
* **Type of each property**: `object`

### nestedTest.extras

Expand Down
Loading