Implement glTF extension KHR_materials_specular

[jjhembd]

Description

This PR implements the glTF extension KHR_materials_specular in physically-based rendering of models.

How it works (and how it affects the code)

Parsing the extension at load time

When the glTF is loaded, GltfLoader reads factors and loads textures from the extension. These are then attached to the material. See the updates to the Material class in ModelComponents.

Note that GltfLoader has been refactored to better handle multiple PBR extensions. See the methods loadMetallicRoughness and loadSpecularGlossiness which have been pulled out of loadMaterial, as well as the new method loadSpecular which handles the new extension. As part of this refactoring, I reworked many of the relevant private methods to use smaller function signatures (taking advantage of properties stored on the loader) and added docstrings.

Processing the extension properties in the pipeline

In MaterialPipelineStage, if material.specular is defined, its properties are processed by the new method processSpecularUniforms, which is very similar to the existing methods processSpecularGlossinessUniforms and processMetallicRoughnessUniforms.

The specular extension, if used, will extend the metallic roughness model. As a result, the check for the extension usually takes place inside the same code branch that handles metallic-roughness processing.

The specular extension has some naming conflicts with the old specular glossiness extension. To avoid confusion, I added "legacy" to the specular glossiness names. For example, the existing uniform u_specularFactor becomes u_legacySpecularFactor, and the corresponding define HAS_SPECULAR_FACTOR becomes HAS_LEGACY_SPECULAR_FACTOR.

Using the extension in the shader

Most of the shader changes are in MaterialStageFS. The specularColorFactor (computed from both specularColorFactor and specularColorTexture is used to modify the f0 value in the czm_modelMaterial (which is somewhat unfortunately named material.specular).

The specularWeight (computed from specularFactor) is then passed through to pbrLighting and ImageBasedLightingStageFS.

Note that the implementation in ImageBasedLightingStageFS is incomplete, as the specularWeight factor is only used to scale down the specular component. The specularWeight factor should also modify the diffuse component in a more complicated way, but this will require some reworking of our image-based lighting calculations.

Other code style updates

Reduced nesting of compiler directives

Some of the material and lighting shader code was difficult to read due to deeply nested compiler directives. The readability problem was exacerbated by inconsistent indentation. For example, in MaterialStageFS, we had:

void materialStage(...)
{
    // ...
    #if defined(LIGHTING_PBR) && defined(USE_SPECULAR_GLOSSINESS)
        #ifdef HAS_SPECULAR_GLOSSINESS_TEXTURE
        vec2 specularGlossinessTexCoords = TEXCOORD_SPECULAR_GLOSSINESS;
          #ifdef HAS_SPECULAR_GLOSSINESS_TEXTURE_TRANSFORM
          specularGlossinessTexCoords = computeTextureTransform(specularGlossinessTexCoords, u_specularGlossinessTextureTransform);
          #endif

        vec4 specularGlossiness = czm_srgbToLinear(texture(u_specularGlossinessTexture, specularGlossinessTexCoords));
        vec3 specular = specularGlossiness.rgb;
        float glossiness = specularGlossiness.a;
            #ifdef HAS_SPECULAR_FACTOR
            specular *= u_specularFactor;
            #endif
// ...

One popular style guide recommends that all preprocessor directives start at the beginning of the line, and that the directives should not affect the indentation of the rest of the code. This would be a significant change to all our shaders, so I did not try to make that change now. Instead, I followed the example of the reference implementation, and tried to limit nesting to 2 levels, by adding subroutines. The above example from MaterialStageFS now becomes:

void setSpecularGlossiness(inout czm_modelMaterial material)
{
    #ifdef HAS_SPECULAR_GLOSSINESS_TEXTURE
        vec2 specularGlossinessTexCoords = TEXCOORD_SPECULAR_GLOSSINESS;
        #ifdef HAS_SPECULAR_GLOSSINESS_TEXTURE_TRANSFORM
            specularGlossinessTexCoords = computeTextureTransform(specularGlossinessTexCoords, u_specularGlossinessTextureTransform);
        #endif

        vec4 specularGlossiness = czm_srgbToLinear(texture(u_specularGlossinessTexture, specularGlossinessTexCoords));
        vec3 specular = specularGlossiness.rgb;
        float glossiness = specularGlossiness.a;
        #ifdef HAS_LEGACY_SPECULAR_FACTOR
            specular *= u_legacySpecularFactor;
        #endif
//...
void materialStage(...)
{
    // ...
    #if defined(LIGHTING_PBR) && defined(USE_SPECULAR_GLOSSINESS)
        setSpecularGlossiness(material);
    #elif //...

This affects both MaterialStageFS and ImageBasedLightingStageFS.

Cleaner loop structure

Some loops with pre-declared counters were cleaned up to better conform to the Coding Guide. This primarily affects GltfLoader.

Options object destructuring

While reading some functions with the arguments supplied as options objects, I found it easier to understand the arguments if I rewrote this:

  const gltfResource = options.gltfResource;
  const typedArray = options.typedArray;
  const releaseGltfJson = defaultValue(options.releaseGltfJson, false);
  const asynchronous = defaultValue(options.asynchronous, true);
  const incrementallyLoadTextures = defaultValue(
    options.incrementallyLoadTextures,
    true
  );
  // ...

as an object destructuring with default values, like this:

  const {
    gltfResource,
    typedArray,
    releaseGltfJson = false,
    asynchronous = true,
    incrementallyLoadTextures = true,
    // ...
  } = options;

This primarily affects GltfLoader, ResourceCache, and ResourceCacheKey.

Issue number and link

Resolves https://github.com/CesiumGS/cesium/issues/11938.

Testing plan

Load the development Sandcastle glTF PBR Specular and compare to the rendering in the reference implementation.

Author checklist

• [x] I have submitted a Contributor License Agreement
• [x] I have added my name to CONTRIBUTORS.md
• [x] I have updated CHANGES.md with a short summary of my change
• [x] I have added or updated unit tests to ensure consistent code coverage
• [x] I have update the inline documentation, and included code examples where relevant
• [x] I have performed a self-review of my code

Other TODO:

• [ ] Load test dataset for Sandcastle from server.
• [ ] Investigate color factor vs texture rendering differences
• [x] Add image comparisons

[github-actions[bot]]

Thank you for the pull request, @jjhembd!

:white_check_mark: We can confirm we have a CLA on file for you.

[jjhembd]

Here are a few image comparisons.

With extension disabled (as it would be rendered before this PR):

Extension enabled: Note the bottom row becoming more mirror-like.

Using procedural IBL, scaled up to exaggerate effect, with extension disabled:

Using procedural IBL, scaled up to exaggerate effect, with extension enabled:

Compare to the reference implementation:

[jjhembd]

@ggetz this is ready for a first look. I still need to load the Sandcastle dataset from a server, and also make a smaller dataset for the specs. But the code is all there.

[ggetz]

Thanks @jjhembd, this appears to be working well! A few top-level comments:

• Re: Reduced nesting of compiler directives, do you think it'd be appropriate to add this to the coding guide?
• The GltfPipeline folder is actually copied from https://github.com/CesiumGS/gltf-pipeline. Any updates in that folder will need to be reflected back there.
• Can you write up an issue (or link to an existing one) for the remaining IBL improvements we plan on making?

[jjhembd]

@ggetz this is ready for another look. For some of the points you raised:

• Nesting of compiler directives: I opened https://github.com/CesiumGS/cesium/issues/11992 to clarify what we want before I update the Coding Guide
• GltfPipeline changes: https://github.com/CesiumGS/gltf-pipeline/issues/660
• IBL improvements: I'm working on an issue.

@lilleyse could you take a first look at the shaders and see if the strategy makes sense? The key changes are in MaterialStageFS, pbrLighting, and ImageBasedLightingStageFS.

Please note that the IBL implementation is incomplete: it scales the specular component correctly, but we are missing a corresponding adjustment to the diffuse component. But our diffuse component needs work, so I think it makes sense to wait until the bigger IBL update.

[lilleyse]

@jjhembd at a quick glance the shader code looks good, it looks like it closely matches the Khronos reference implementation.

[ggetz]

Thanks @jjhembd! This looking good to me. Aside from the IBL modifications, any remaining work is summed up in your linked issues above.

Please open an issue for the IBL changes when you can.

As we talked about offline, once the initial extension support is complete for this, _materials_anisotropy, and KHR_materials_clearcoat, it makes sense to quickly follow up with the IBL improvements.