How are managed material defines ?

[PeapBoy]

Hello everybody,

I face some errors in my app concerning material defines, I don't understand how they're managed.

To render a mesh, subMesh.getMaterial().isReadyForSubMesh() and subMesh.getMaterial().bindForSubMesh() functions are successively called each frame, right ?
Material defines are created at the beginning of the isReady function if subMesh doesn't have defines (so, if it's the first time this mesh is rendered, I assume).
Defines are stored in subMesh with the _materialDefines property.

Guess we begin with a standard material, we'll get StandardMaterialDefines on our subMesh and use the isReady function of StandardMaterial.
Now, I assign another material to my mesh (a PBR for example). We'll now use the isReady function of PBRMaterial.
In the isReady function, we'll only check if defines exist and we will work with StandardMaterialDefines instead of PBRMaterialDefines, no ?

This is what happens to me in some circumstances... 
PG: https://www.babylonjs-playground.com/#YNCHVR
(Open your console)

But then, there is a mechanism that handles this issue and this doesn't happen anymore. I don't understand this either.

Someone could enlight me please ? 

[GameMonetize]

You are right  this rich (who said complex? :D) mechanism is here to optimize code but could be tough to follow

The defines stored on every submesh are actually wiped out when you change the material: https://github.com/BabylonJS/Babylon.js/blob/master/src/Mesh/babylon.abstractMesh.ts#L158

[PeapBoy]

Shame on me, I didn't think about looking for "setEffect(null)"  Thanks

Mmh now, I understand the issue in the PG above.
First time setMaterial (and setEffect(null)) is called, (this._materialEffect === effect)  is (undefined === null) so we assign undefined to materialDefines and null to materialEffect.
Then we create StandardMaterialDefines at the beginning of the isReady function. If something is not ready yet, we don't call setEffect and materialEffect stays at null.
Second time setMaterial (and setEffect(null)) is called, (this._materialEffect === effect) is (null === null) so we return and don't assign undefined to materialDefines. Then I find myself with StandardMaterialDefines instead of PBRMaterialDefines or undefined. And boom.

setEffect(null) is not enough in the particular case where something isn't ready in material isReady function() and materialEffect is not replaced by a value.
Do you mind if we assign directly subMesh._materialDefines = undefined in the material set function ?

Moreover, it's been a long time I wonder why we don't use defines in bindForSubMesh function to save some if instructions. But I could open a new post for this. 

[GameMonetize]

Quote

Do you mind if we assign directly subMesh._materialDefines = undefined in the material set function ?

This is what this does: https://github.com/BabylonJS/Babylon.js/blob/master/src/Mesh/babylon.subMesh.ts#L14

When you call submesh.setEffect(null)

[PeapBoy]

As I show it in my PG, sometimes it happens that setEffect(null) is not enough.
In some cases, this._materialEffect is null. So, if you call setEffect(null), it will return immediately without setting the defines to undefined.

I have this issue in my app. I tried to reproduce it and make it simple in the PG.
https://www.babylonjs-playground.com/#YNCHVR

[GameMonetize]

ok makes sense!

so do you want to submit a PR to update the setEffect(null) to affect defines to null if _materialEffect is null?

[PeapBoy]

Of course ! 

However, I just realized that the issue I have in my app is not exactly the same as the one in the PG.
My bug appears because of my subMaterials.

I can see in this code that there's a complete mechanism when subMaterials are set :

Object.defineProperty(MultiMaterial.prototype, "subMaterials", {
  get: function () {
    return this._subMaterials;
  },
  set: function (value) {
    this._subMaterials = value;
    this._hookArray(value);
  },
  enumerable: true,
  configurable: true
});
MultiMaterial.prototype._hookArray = function (array) {
  var _this = this;
  var oldPush = array.push;
  array.push = function () {
    var items = [];
    for (var _i = 0; _i < arguments.length; _i++) {
      items[_i] = arguments[_i];
    }
    var result = oldPush.apply(array, items);
    _this._markAllSubMeshesAsTexturesDirty();
    return result;
  };
  var oldSplice = array.splice;
  array.splice = function (index, deleteCount) {
    var deleted = oldSplice.apply(array, [index, deleteCount]);
    _this._markAllSubMeshesAsTexturesDirty();
    return deleted;
  };
};

I have three questions about this part :
- Why adding a new material only affects textures and not everything ? (TexturesDirty flag)
- How are the materialDefines assigned to undefined ? (They aren't, right ? That's my issue in my app, I think)
- If we add a subMaterial using subMaterials[materialIndex] = new Material() instead of using subMaterials.push() function, is the dirty flag rised ?

[PeapBoy]

Hello again,

When calling subMesh.getMaterial() function, we actually set the new subMaterial to the subMesh.
I think we should reset the defines here.
 

public getMaterial(): Material {
  var rootMaterial = this._renderingMesh.material;

  if (rootMaterial && (<MultiMaterial>rootMaterial).getSubMaterial) {
    var multiMaterial = <MultiMaterial>rootMaterial;
    var effectiveMaterial = multiMaterial.getSubMaterial(this.materialIndex);

    if (this._currentMaterial !== effectiveMaterial) {
      this._currentMaterial = effectiveMaterial;
      if (this._materialDefines) {
        this._materialDefines.markAllAsDirty();
      }
    }

    return effectiveMaterial;
  }

  if (!rootMaterial) {
    return this._mesh.getScene().defaultMaterial;
  }

  return rootMaterial;
}

Instead of this  :

if (this._materialDefines) {
  this._materialDefines.markAllAsDirty();
}

We could do this :

this._materialDefines = undefined;

Because if we assign a new material the line before, materialDefines are not the good ones anymore (except if you change the material from a PBR to a PBR or from a Standard to a Standard).

Since I'm sure I don't understand everything so I may miss some important processes, I prefer asking here if it makes sense rather than submitting a PR immmediately.
 

[GameMonetize]

I think it is a great fix! You can go ahead

[PeapBoy]

Hello,

I still have a case where defines are wrong 
But I found why.
 

PushMaterial.prototype.isReady = function (mesh, useInstances) {
  if (!mesh) {
    return false;
  }
  if (!mesh.subMeshes || mesh.subMeshes.length === 0) {
    return true;
  }
  return this.isReadyForSubMesh(mesh, mesh.subMeshes[0], useInstances);
};
PushMaterial.prototype.bind = function (world, mesh) {
  if (!mesh) {
    return;
  }
  this.bindForSubMesh(world, mesh, mesh.subMeshes[0]);
};

 

In these two functions, we use mesh.subMeshes[0] without knowing if this subMesh does use the material.
These function are called by Scene._checkIsReady() function, that's why I faced this case.

For example. I have a MultiMaterial which has two subMaterials : a Standard and a PBR.
MultiMaterial.isReady(mesh) will call PushMaterial.isReady(mesh) for each subMaterial.
First, it will go through StandardMaterial.isReadyForSubMesh() function, create StandardMaterialDefines and set them to mesh.subMeshes[0].
Then, it will go through the PBRMaterial.isReadyForSubMesh() function with StandardMaterialDefines instead of PBRMaterialDefines.
Because we called the two functions with the same subMesh.


I understand it's necessary for retrocompatibility (there was only isReady function before, right ?). So I can't simply put the defines to undefined after calling the isReadyForSubMesh function...

[GameMonetize]

The fix could be in the isReady function of MultiMat. It should call isReadyForSubMesh instead of isReady if it exists

[PeapBoy]

Something like this ?

MultiMaterial.prototype.isReady = function (mesh) {
  if (!mesh) {
    return false;
  }
  if (!mesh.subMeshes || mesh.subMeshes.length === 0) {
    return true;
  }
  for (var index = 0; index < mesh.subMeshes.length; index++) {
    var subMesh = mesh.subMeshes[index];
    var subMaterial = this.subMaterials[subMesh._materialIndex];
    if (subMaterial) {
      if (!material.isReadyForSubMesh(mesh, subMesh)) {
        return false;
      }
    }
  }
  return true;
}

But if we do this, you're not guaranteed to verify every material of subMaterials array. So it could return true without being really ready.
It only returns true if every used subMaterial is ready.

[GameMonetize]

sounds good to me

[PeapBoy]

Seems like you did the last change, thanks !