Material refactoring

[javagl]

Addresses #133

As mentioned in the issue: The MaterialModelV2 class (and MaterialModelV1) have been attempts to handle glTF 1.0 and glTF 2.0, with their main difference being in the material models. The MaterialModelV2 tried to offer a "flattened" access to the information that is stored in the glTF PBR material model. This "flattening" consisted of omitting the TextureInfo structures, which turned out to not be ideal: The TextureInfo actually carries certain extensions, which consequently could not be modeled nicely with this class.

This PR is a first draft of a possible refactoring here. This will involve some breaking changes - specifically, for those who explicitly used these classes by creating instances of them. The changes for clients will roughly fall into the categories of 1. using a different constructor and 2. creating the (new) PBR-based materials including the structures for the TextureInfo.

As ... some sort of justification: Some of this was already on the radar. The MaterialModelV2 class carried a note

/**
 * ...
 * Note: This class might be renamed to "PbrBasedMaterialModel" and moved to
 * a different package in the future.
 */

Simiarly, the MaterialModelV1 class carried a note

/**
 * ...
 * Note: This class is actually no longer specific for glTF 1.0. It might
 * be renamed to "TechniqueBasedMaterialModel" and moved to a different
 * package in the future.
 */

This is essentially what is done here.

The MaterialModelV1 is relatively simple (and hardly anyone is using that anyhow). It has been renamed to DefaultTechniqueMaterialModel, and implements a new interface TechniqueMaterialModel which just offers the technique-based material representation.

There are more changes for the MaterialModelV2. There now are structures that more closely resemble the actual structure of the materials in glTF 2.0.:

• PbrMaterialModel, corresponding to glTF material
• PbrMetallicRoughnessModel, corresponding to glTF pbrMetallicRoughness
• The "texture info" structures:
  • TextureInfo, corresponding to glTF textureInfo
  • NormalTextureInfo (extending TextureInfo), corresponding to glTF normalTextureInfo
  • OcclusionTextureInfo (extending TextureInfo), corresponding to glTF occlusionTextureInfo

as well as Default... implementations for all of them.

Accessing these structures can be a bit inconvenient, compared to the MaterialModelV2 that flattened all this out. Previously, the MaterialModelV2 offered functions like
TextureModel texture = material.getBaseColorTexture();

The new structures would make this rather tedious:

TextureModel texture = null;
PbrMetallicRoughnessModel metallicRoughness = material.getPbrMetallicRoughnessModel();
if (metallicRoughness != null) {
    TextureInfoModel textureInfo = metallicRoughness.getBaseColorTextureInfoModel();
    if (textureInfo != null) {
        texture = textureInfo.getTextureModel();
    }
}

🤕

So there are some convenience functions for that, via default methods in the interface, meaning that it is still possible to do
TextureModel texture = material.getBaseColorTexture();

But I'm hesitating and going back and forth about which of convenience functions should be offered. I could offer nearly all the functions that have been in MaterialModelV2. Maybe I'll do this to make the change "less breaking"...

The question is even more important for the case of creating a material: The MaterialModelV2 offered convenient functions like
material.setBaseColorFactor(values);
Trying to offer similar functions in the new implementation raises a few questions about consistencies.

For both (reading and creating materials), some questions about the handling of default values still have to be sorted out.

Hopefully, people preferred the MaterialBuilder, which should be largely unaffected by all this.

---

Done. One TODO for me: The extensions and extras from the TextureInfo-related structures are not yet passed to the model or copied accordingly. Some functions that are involved here still require a cleanup.

[javagl]

Some questions about the handling of default values still have to be sorted out.

A bit more specific information about this point: The current MaterialModelV2 class offers some properties as primitive types, and assigns default values. For example, the metallicFactor is currently a float, and set to 1.0f in the constructor. As part of the general transition from float to double, it could/should become double. But I think that it could be beneficial for consistency to offer it as Double instead. Similarly, float[] arrays that are currently initialized with defaults can remain null.

I'm hesitating a bit about this one. When a client wants to obtain the metallicFactor, and receives a null for that, then the client has to deal with this, and has to "know" that the default value should be 1.0. But I think that one could try to strive for some form of consistency here. For example, functions like the Camera getAspectRatio already do return the boxed type and null if no value was set. More generally, most places until now do not try to assign default values. Trying to consistently assign them leads to tricky questions elsewhere. (For example, a Node rotation and matrix could both have default values, but it doesn't make sense to try and return them). So I'm leaning towards not providing default values in some places, to have more consistency in the API, particularly given that this PR already does involve a breaking change.

[javagl]

Tagging @Nadwey (because you interacted with this PR), and @LocutusV0nB0rg (because of your general interest in JglTF - even though you might be less interested in details of the material than in other things).

I just marked this as "Ready For Review". Note that this is only supposed to go into the v3.0.0-dev ("staging") branch! It may not make sense to try and "review" it on a code level. But maybe someone wants to have a look, and/or try it out, and/or provide some feedback.

Some details and justifications

Again: This is not final. There may still be caveats and things that have to be adjusted when further (or even "all") (PBR) extensions are supposed to be implemented. But the general approach taken here seems to go in the right direction. The proof-of-concept usage of this state in the implementations of KHR_materials_variants and KHR_texture_transform and KHR_materials_clearcoat in #135 did not reveal anything that is fundamentally wrong.

A basic summary of the approach was already given in the initial post of this PR: The idea is to more closely reflect the actual structures of glTF materials on the "model" side. And I think that reflecting some of these structures (like the TextureInfo) seems to be necessary, given that extensions may be attached to these things. Trying to let "the things that contain multiple texture infos" somehow "aggregate" the extension information from each of them doesn't seem to be feasible, and could not sensibly and consistently be done for extensions of extensions.

Some of the changes here do (in their current form) make aspects of the material handling a bit less convenient. The former MaterialModelV2 class did aggregate many of its internal structures (like the PBR material and the other texture infos). With the more fine-grained structures, and given that the MaterialModel itself is a "tagging type" that has to cover glTF 1.0 and 2.0 materials, some operations involve multiple steps and ... yeah, pretty "ugly" ... casts. For example, setting the base color texture of a material right now looks roughly like this:

PbrMaterialModel materialModel = (PbrMaterialModel)gltfModel.getMaterialModel(0);
DefaultPbrMetallicRoughnessModel pbr = 
  (DefaultPbrMetallicRoughnessModel) materialModel.getPbrMetallicRoughnessModel();
DefaultTextureInfoModel tim = (DefaultTextureInfoModel)pbr.getBaseColorTextureInfoModel();
tim.setTextureInfoModel(example);

There are two options that I'm still considering for improving or simplifying this. (This may happen at a later point, maybe as a new PR into the v3.0.0-dev branch).

Option 1: Covariant return types.

The current structure here follows the structure of other model classes, namely that of read-only interfaces, and modifiable Default implementations. For example, for the materials

interface PbrMaterialModel extends MaterialModel
{
  PbrMetallicRoughnessModel getPbrMetallicRoughnessModel();
}
interface PbrMetallicRoughnessModel
{
    TextureInfoModel getBaseColorTextureInfoModel();
}
interface TextureInfoModel
{
    TextureModel getTextureModel();
}

With the corresponding Default... classes that contain the corresponding 'setters'. (This structure has its justifications. And it could make much more sense under slightly different starting conditions. But I'll skip the history of the glTF 1.0-to-2.0 transition here). In practice, nearly all the code can (or even has to) assume that these elements can be casted to their Default counterparts. This is the sequence of casts that is shown in the code snippets above. But as a rule of thumb, these casts should only be done when it's absolutely necessary.

One way of avoiding these casts would be to assume that the Default-implementations also return Default implementations. The DefaultPbrMaterialModel could not return a PbrMetallicRoughnessModel, but a DefaultPbrMetallicRoughnessModel, like this:

class DefaultPbrMaterialModel implements PbrMaterialModel
{
    // ...

    @Override
    public DefaultPbrMetallicRoughnessModel getPbrMetallicRoughnessModel()
    {
        return pbrMetallicRoughnessModel;
    }
}

(Siimilarly, for all other types down the hierarchy).

With this, the actual types are "pinned" to the Default ones after the first cast:

// Only cast once...
DefaultPbrMaterialModel materialModel = 
  (DefaultPbrMaterialModel)gltfModel.getMaterialModel(0);

// No additional cast necessary here
DefaultPbrMetallicRoughnessModel pbr = materialModel.getPbrMetallicRoughnessModel();
DefaultTextureInfoModel tim = pbr.getBaseColorTextureInfoModel();
tim.setTextureInfoModel(example);

This would somehow defeat the (little) purpose of the interfaces. I'm hesitating whether that's a sensible trade-off. But I'm deferring that change, because changing the return type to the Default versions would always be possible, Changing them back from the Default versions to the interfaces would be a breaking change.

Option 2: Convenience functions

There could always be some convenience functions, either in the classes themself, or in some static utility functions. There's nothing preventing code that allows changing the given snippet to something like this

// Unavoidable cast:
DefaultPbrMaterialModel materialModel = (DefaultPbrMaterialModel)gltfModel.getMaterialModel(0);

// Convenience function to hide the casts and/or instanceof-checks:
materialModel.setBaseColorTextureInfoTextureModel(example);

or even

MaterialModel materialModel = gltfModel.getMaterialModel(0);

// Convenience function to hide the casts and/or instanceof-checks:
MaterialModels.setPbrBaseColorTextureInfoTextureModel(
  materialModel, example);

But similar to Option 1: These can be added later, without breaking anyhing.

If the (common) usage patterns turn out to be too cumbersome and repetitive, and if/when I have a clearer idea about what the best structure of these convenience functions should be, I'll consider to add them.