KhronosGroup/glTF-Validator

Warn on redundant property values

lexaknyazev opened this issue · 5 comments

Some properties of the material extensions can make the whole extension useless when set to zero. For example:

  • KHR_materials_clearcoat
    • clearcoatFactor disables the extension.
    • clearcoatRoughnessFactor disables the associated texture.
  • KHR_materials_sheen
    • sheenColorFactor disables the extension.
    • sheenRoughnessFactor disables the associated texture.
  • KHR_materials_transmission
    • transmissionFactor disables the extension.
  • KHR_materials_volume
    • thicknessFactor disables the extension.

Note that the same applies to the base spec's emissiveFactor that has to have a non-default value for the emissive texture to have any effect.

On top of that, there's no need to include, e.g., an IOR extension with a value of 1.5 as it wouldn't change the material appearance but may reduce asset portability. We could go even further and flag all glTF properties that have schema-default values.

This approach may get slightly outdated with the upcoming animation pointer extension.

@emackey @javagl @bghgary WDYT?

(I don't know the technicalities of the specific properties that you mentioned, so this is rather about the question of flagging schema-default values in general )


The animation pointer was also the first thing that came to my mind. What could be the medium-term path here? A property should probably not be flagged for having its default value when the property is animated. So there are roughly two options:

  • Flag these values now, but don't flag them after the animation pointer extension was ratified.
  • Add support for the animation pointer extension in the validator, and only warn when the default-valued property is not animated.

The first one would make many assets "invalid" (at least, contain warnings) for a short period of time, and then they turn "valid" again. That doesn't sound right.

The second one could be doable. I assume that if/when the animation pointer extension is ratified, the validator will sooner or later support it anyhow, to check the validity of the pointer/path, for example. But it would still cause potentially many assets to suddenly have potentially many warnings.

(Do you have a rough idea about how how many of the sample models contain such redundant properties?)


More generally: I see the point of this question, of course. Imagine an asset with 10000 nodes, where each node contains the identity matrix as its transform, but is redundantly written out as a full-blown 16-element-array. That would be far from optimal.

But on the other hand, consider some pseudocode of an asset generator:

const asset = createBasicAsset();
const meshPrimitive = createMeshPrimitive();
asset.nodes[0].meshes[0].primitives.push(meshPrimitive);

const fileContent = JSON.stringify(asset, null, 2);
write(fileContent);

So far, so simple.

But implementing asset generators in a way that they are aware of the default values can be really, really non-trivial. In order to avoid potential warnings in the above example, it would be necessary to build a tremendously complex infrastructure that does things like

if (asset.nodes[0].meshes[0].primitives[0].mode === 4) {
    asset.nodes[0].meshes[0].primitives[0].mode = undefined; // 4 is the default....
}

for each and every property (not to mention cases like "the default value is X if condition Y holds"). So requiring to not write out default values may be a considerable burden for asset generators. And if the goal was to create an asset generator that is "guaranteed" do generate warning-free assets, this burden might be prohibitively large.

  • I think there's some value in flagging default values in extensions, particularly those that make such extensions useless. Exporters may unconditionally write assets with all material extensions present even when the actual materials do not go beyond the base spec PBR MR.
  • Another reason is to catch incorrect extension usage when the "enabler"-property (like clearcoat factor) remains unintentionally unset.
  • For historical reasons, we do have a dedicated validation message about identity matrix. Maybe we should supersede it with something more generic.

The upcoming iridescence extension gives couple more domain-specific examples:

  • iridescenceThicknessMinimum has no effect when iridescenceThicknessTexture is undefined.
  • iridescenceThicknessTexture has no effect when iridescenceThicknessMinimum == iridescenceThicknessMaximum.

I do think that these two unique cases should produce dedicated validation issues.


As the next step, we may start flagging all factor / texture pairs that yield no-ops.

I would vote not to flag all redundant values as warnings (like primitive.mode === 4), for the reasons @javagl mentions. Flagging as hints at most, perhaps.

But the cases @lexaknyazev describes here seem OK to me as warnings. 👍🏻

Is the reason the animation pointer extension would affect these warnings just that you may intend to keep a "disabled" extension or texture for later animation? Or something else?

The philosophy in the design of all these PBR-related extensions is that the extension itself is always useless. The extension, and indeed the entire JSON shell of the glTF, is read once during glTF parsing / loading, and is thrown out prior to runtime rendering.

An application that supports clearcoat for example, will have a clearcoat internal API parameter on every single material, not just materials that happen to carry a particular glTF extension. Blender is a good example of this: There's a clearcoat input on every PBR material that imports from glTF, not just materials that use clearcoat. After import from glTF, the user is free to edit the clearcoat input, turning the effect on or off independently from the presence of the glTF JSON or its extensions. The Blender glTF exporter will decide anew at export time whether it's appropriate to write a clearcoat extension for each material.

Blender screenshot

If and when some future version of glTF or any other 3D format decides to incorporate clearcoat as a core parameter, it will be completely compatible with this behavior: The parameter will always be implicitly present, on every material, with a default value that leaves its effects turned off by default, ready to be activated by a user (or perhaps an animation) at any moment. And when exporting such a thing to glTF 2.0, the clearcoat extension is just a shell that needs to be written whenever a particular material's settings don't match the default clearcoat settings at the time of export.

Getting back to validation behavior, I would suggest:

  • If the validator finds any default value explicitly specified, be it in an extension or in core, it could raise a message that the value is not needed. Similar to UNUSED_OBJECT, this would likely be a severity 2 informational message.

  • If the validator finds a texture assigned in a spot where the factor is zero (in extension or in core), I would expect this to be a severity 1 "warning" message. I wouldn't go as far as failing the validation, because glTF is sometimes used for asset exchange (for example unused texture coordinates are often repurposed after glTF import by some other tool, so we don't always want to strip them away). The possibility of animated factors complicate this check.

  • I agree with the Iridescence checks proposed in the comment above. Those might also be warnings, not validation errors, as the parameters could be animated by a vendor extension or controlled at runtime by some process unknown to us.