vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
454 stars 72 forks source link

[Question] MESH_PRIMITIVE_TOO_FEW_TEXCOORDS #191

Closed mhwlng closed 1 month ago

mhwlng commented 11 months ago

I received a lot of GLB files (created by GLTFExporter, no idea what version) where :

GltfValidator.ValidationReport.Validate("test.glb"); returns this error:

MESH_PRIMITIVE_TOO_FEW_TEXCOORDS
meshes/0/primitives/0/material
Material is incompatible with mesh primitive: Texture binding '/materials/0/occlusionTexture' needs 'TEXCOORD_1' attribute.

(and the object is not shown correctly in three.js based viewers)

The various validation routines in SharpGLTF don't detect this error. (Nuget v1.0.0-alpha0030)

(In the past, this was not an error and was ignored by the various viewers. But: not anymore in newer versions...)

I found that this is the wrong data

   "materials":[
      {
         "name":"Default_Fabric",
         "occlusionTexture":{
            "index":2,
            "texCoord":1,
            "extensions":{
               "KHR_texture_transform":{
                  "scale":[
                     6,
                     6
                  ]
               }
            }
         }
      },

and following the instructions from

https://github.com/mrdoob/three.js/issues/25836#issuecomment-1509828959

this javascript fixes the problem:

for (const material of document.getRoot().listMaterials()) {
    const info = material.getOcclusionTextureInfo();
    if (info) info.setTexCoord(0);
}

which removes "texCoord":1, from the above .json

Validation = ValidationMode.TryFix also doesn't fix this.

Is it possible to let validate() return an error for this file and let ValidationMode.TryFix enumerate all the materials and fix this problem in the material\occlusionTexture using SharpGLTF ?

vpenades commented 11 months ago

Could it be possible to share one of these models?

Without seeing the models I suspect the issue is happening because the materials require more texture uv channels than what the mesh primitives provide

mhwlng commented 11 months ago

Try this file

https://habufa-res.cloudinary.com/image/upload/v1691148553/bad.glb

with this viewer

https://gltf-viewer.donmccurdy.com/

mhwlng commented 11 months ago

f.y.i. I edited the first post, because I initially misinterpreted the problem:

It seems that to fix the problem, you need to simply REMOVE "texCoord": 1 from the materials\occlusionTexture in the json

because there is no TEXCOORD_1 defined in any of the mesh primitives

vpenades commented 11 months ago

Well... the model is malformed, and if the library is unable to detect that it is malformed then there's a bug in the validator.

Another thing is if the library should fix it or decide whether the model is fixable.

In this particular case, most probably the material's occlusion requires its own UV channel, because that's how occlusion maps work, so fixing it by using the UVs of channel 0 would cause the occlusion maps to be rendered incorrecly... the only solution, as you mentioned, is to completely remove the occlusion from the materials.

mhwlng commented 11 months ago

I don't know if this is the best way to do it, but this is how I fixed the .glb file:

for (var i = 0; i < rootObject.LogicalMaterials.Count; i++)
{
  var material = rootObject.LogicalMaterials[i];

  var hasOcclusion = material.Channels
  .FirstOrDefault(o => o.Key == "Occlusion" && o.TextureCoordinate > 0).Key != null;

  if (hasOcclusion)
  {
    var materialBuilder = material.ToMaterialBuilder();
    materialBuilder.RemoveChannel(KnownChannel.Occlusion);
    materialBuilder.Name = "Repair_" + material.Name;

    var newMaterial = rootObject.CreateMaterial(materialBuilder);

    foreach (var mesh in rootObject.LogicalMeshes)
    {
      foreach (var meshPrimitive in mesh.Primitives)
      {
        if (meshPrimitive.Material == material)
        {
          meshPrimitive.Material = newMaterial;
        }
      }
    }
  }
}

rootObject.SaveGLB("test.glb", new WriteSettings()
{
  Validation = ValidationMode.TryFix
}
);

Of course, There are now one or more unused materials in the GLB. So, the new GLB file is bigger.

vpenades commented 11 months ago

Yes, right now that would be the way to do it.