kcoley / gltf2usd

command-line utility for converting glTF 2.0 models to USD
MIT License
263 stars 39 forks source link

Albedo, metalness, roughness factors ignored when using textures. #160

Closed ghost closed 5 years ago

ghost commented 5 years ago

glTF

Screenshot 2019-05-22 at 11 33 42 AM

USDZ

Screenshot 2019-05-22 at 11 33 53 AM

The blue ball uses a white texture with a factor of 0.5. The USDZ is not using the factor when creating new maps.

We use this a lot in our platform for the albedo (diffuseColor) texture. This allows us to have the same wood texture with a slightly different colour.

Would it be possible to add this multiplier in the texture conversion on your side and to get a USDZ as close as possible of the input glTF?

ghost commented 5 years ago

Edit: It seems USDZ supports it but the converter code defaults it to 1: https://i.imgur.com/G62rnqG.png

kcoley commented 5 years ago

Hi @sayduck-daniel, looks like multiplying factors by textures is not supported in UsdPreviewSurface. Though might be something you can bring up here: https://groups.google.com/forum/#!forum/usd-interest

ghost commented 5 years ago

Hi @kcoley, while testing we saw USD is not taking in account those factors.

Never the less, glTF does use it and when we convert a glTF using those factors, this ends up in a USD not looking the same.

Would it be possible on your side to multiply the image by the factor when you split the roughnessMetalsness map? This should be about multiplying GB channels from the roughnessMetalsness before saving it for USD use.

This would kinda work like the tiling we did earlier, this will allow us to keep USD and glTF from this convert identical until USD catches up.

kcoley commented 5 years ago

@sayduck-daniel actually, the factors are being set in the scale attribute of UsdUVTexture, and should be multiplied against the roughness and metalness maps. This might be a bug in Quicklook.

ghost commented 5 years ago

It seems it's not working that way what makes models totally wrong when the gltf is using a texture for roughness and a factor for the metalness.

I think you should move towards using the factors to change the image you generate until USDZ supports it correctly.

kcoley commented 5 years ago

@sayduck-daniel does this branch work for you?: https://github.com/kcoley/gltf2usd/tree/fixBlendShapes

ghost commented 5 years ago

Let me come back to you asap :)

ghost commented 5 years ago

Hi @kcoley, this is what I got from the person testing:

This seems to work. For example a product using only a roughnessMap and a metalnessFactor of 0 is no more fully metallic when converted to USDZ. However, the strength parameter is still ignored (ie a black occlusion map with strength 0 will still make the USDZ all black instead of having no effect)

ghost commented 5 years ago

I just tested myself with this gltf that looks like this:

Screenshot 2019-06-12 at 11 36 27 AM

We have 3 set of balls:

The output on master branch looks like this:

Screenshot 2019-06-12 at 11 37 43 AM

The output on the new branch looks like this:

Screenshot 2019-06-12 at 11 37 08 AM

As we can see, it solved the one top right.

I saw on your changelog you introduced the parameter --scale-texture but it's not working for us:

$ /.../gltf2usd/Source/gltf2usd.py --scale-texture -g model_with_local_uris.gltf -o model_branch_w_param.usdz
converted usd file extension from .usdz to .usdc: /var/folders/2q/82jdm_392rn_0lnshmt1r1_80000gn/T/tmpwugYlk/model_branch_w_param.usdc
Traceback (most recent call last):
  File "/.../gltf2usd/Source/gltf2usd.py", line 905, in <module>
    convert_to_usd(os.path.expanduser(args.gltf_file), os.path.abspath(os.path.expanduser(args.usd_file)), args.fps, args.scale, args.arkit, args.verbose, args.use_euler_rotation, args.optimize_textures, args.generate_texture_transform_texture, args.scale_texture)
  File "/.../gltf2usd/Source/gltf2usd.py", line 830, in convert_to_usd
    usd = GLTF2USD(gltf_file=gltf_file, usd_file=temp_usd_file, fps=fps, scale=scale, verbose=verbose, use_euler_rotation=use_euler_rotation, optimize_textures=optimize_textures, generate_texture_transform_texture=generate_texture_transform_texture, scale_texture=scale_texture)
  File "/.../gltf2usd/Source/gltf2usd.py", line 76, in __init__
    self.convert()
  File "/.../gltf2usd/Source/gltf2usd.py", line 798, in convert
    self._convert_materials_to_preview_surface_new()
  File "/.../gltf2usd/Source/gltf2usd.py", line 487, in _convert_materials_to_preview_surface_new
    usd_material.convert_material_to_usd_preview_surface(material, self.output_dir, material_name)
  File "/.../gltf2usd/Source/_gltf2usd/usd_material.py", line 23, in convert_material_to_usd_preview_surface
    usd_preview_surface = USDPreviewSurface(self._stage, gltf_material, self, output_directory, material_name, self._scale_texture)
  File "/.../gltf2usd/Source/_gltf2usd/usd_material.py", line 44, in __init__
    self._initialize_from_gltf_material(gltf_material)
  File "/.../gltf2usd/Source/_gltf2usd/usd_material.py", line 94, in _initialize_from_gltf_material
    self._set_khr_material_pbr_specular_glossiness(gltf_material)
  File "/.../gltf2usd/Source/_gltf2usd/usd_material.py", line 154, in _set_khr_material_pbr_specular_glossiness
    self._set_pbr_metallic_roughness(gltf_material)
  File "/.../gltf2usd/Source/_gltf2usd/usd_material.py", line 148, in _set_pbr_metallic_roughness
    self._set_pbr_metallic(pbr_metallic_roughness)
  File "/.../gltf2usd/Source/_gltf2usd/usd_material.py", line 259, in _set_pbr_metallic
    destination = metallic_roughness_texture.write_to_directory(self._output_directory, GLTFImage.ImageColorChannels.B, "metallic", scale_texture)
  File "/.../gltf2usd/Source/_gltf2usd/gltf2/Material.py", line 47, in write_to_directory
    return self._image.write_to_directory(output_directory, channels, texture_prefix, self._tt_offset, self._tt_scale, self._tt_rotation, scale_factor)
  File "/.../gltf2usd/Source/_gltf2usd/gltf2/GLTFImage.py", line 98, in write_to_directory
    value[0] = value[0] * scale_factor[0]
TypeError: 'tuple' object does not support item assignment
kcoley commented 5 years ago

@sayduck-daniel thanks, I merged an update to master for this