Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export: KHR_materials_specular and KHR_materials_ior #1158

Closed
wants to merge 9 commits into from

Conversation

proog128
Copy link

Adds support for the specular, specular tint and ior properties of the Principled BSDF node. The values are converted to KHR_materials_specular (KhronosGroup/glTF#1719) and KHR_materials_ior (KhronosGroup/glTF#1718). The conversion is explained in this comment.

This is more a proof-of-concept prototype than an actual implementation. Baking several properties into textures doesn't seem to fit nicely into the codebase at the moment, it probably needs to be refactored a bit.

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2020

CLA assistant check
All committers have signed the CLA.

@julienduroure
Copy link
Collaborator

@proog128 Did you see that some automatic tests failed?

if no_texture:
if specular != 0.5 or specular_tint != 0.0:
specular_ext_enabled = True
normalized_base_color = [bc / luminance(base_color) if luminance(base_color) > 0 else 0 for bc in base_color]
Copy link
Contributor

@scurest scurest Aug 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base_color can be None here (but only if specular_tint==0, so its value doesn't matter).

@julienduroure julienduroure added enhancement New feature or request exporter This involves or affects the export process Material labels Aug 10, 2020
This issue occurs if base color is a texture and specular and specular tint are greater 0.
# Conflicts:
#	addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py
#	addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_texture_info.py
@remi-barney
Copy link

I was going to post an issue for this feature and found this PR. We're missing this specular feature on our Blender > glTF workflow. And now it's supported in ThreeJS, it would be great to be able to export it from Blender. 😃

PS: @julienduroure I see that you're the main contributor to this project. So sorry I can't help with the code, python and Blender's API are not my strong suit. Tell me if I can help in any other way. ✊ Anyway, thank you all for this useful plugin.

@repalash
Copy link

Would be great having these extensions natively in the addon. I can help with fixing any issues in this. Let me know if interested.

@julienduroure
Copy link
Collaborator

Hello @repalash ,
Help is always welcome :)
@proog128 are you available to rebase your work on master?

Everyone is welcome to

  • review code
  • test some file and publish some screenshots
  • provide some files to be added to test folder

Thanks !

@proog128
Copy link
Author

I rebased to master. It should work, but it still lacks testing and refactoring.

Especially test scenes with different combinations of textures on/off per slot would be nice. This is definitely a weak spot in the current (hacky, proof-of-concept) implementation.

So far, the exporter had to rewrite texture channels to match the glTF spec (e.g., metallic into B channel, roughness into G, and so on). With the new glTF PBR extensions, this becomes more complicated. It's not only about copying channels, but also applying some math and doing that conditionally based on whether a texture slot is filled or empty. This is (obviously) not well supported in the current architecture of the exporter, so a refactoring from someone more familiar with the exporter code base would be highly appreciated! I am happy to answer any questions.

@julienduroure
Copy link
Collaborator

@proog128

So far, the exporter had to rewrite texture channels to match the glTF spec (e.g., metallic into B channel, roughness into G, and so on)

Do you mean that there is currently some case where we export some non valid glTF files, not following the spec? Do you have some example?

@proog128
Copy link
Author

proog128 commented Nov 7, 2021

Do you mean that there is currently some case where we export some non valid glTF files, not following the spec? Do you have some example?

No, the exporter writes spec-conformant glTF files, without and with this pull request. It just becomes more complicated when taking specular and IOR into account. Reason is that the mapping from Blender's specular parameters to glTF's specular parameters involves some computations, not just channel mapping.

For the non-textured case, the mapping is rather simple, it can be found in gltf2_blender_gather_materials.py, line 434. For the textured case the mapping is in gltf_blender_gather_images.py, where I added the FillSpecularColor class for this purpose. The textures are collected here and later, in the encode step, the same equations as before are applied to the textures via numpy, see line 289.

@missphyrgames
Copy link

Hey, I would also love for this to be added! Is there any time frame for getting this in and is there anything I can do to assist? Thanks!

@julienduroure
Copy link
Collaborator

Hello,
Some update from my side, and some questions:

  • I just wrote a generic way to manage complex numpy calculation. Currently, there is only one way to manage texture : Mapping channels from source to destination. I added some generic way to get data from source (Image and/or scalar), and use them to generate an image. Regarding list of sockets, solution used (mapping or specific) is chosen. (code not pushed yet)
  • Next step is now to fit the specular calculation into this generic system.

Some question about @proog128 's implementation:

  • I can see specularColorFactor and specularColorTexture, but what about specularFactor and specularTexture ?

    • Seems there are not implemented in current PR
    • Because specularColorTexture is RGB and specularTexture is A, is that a good idea to export a single RGBA texture?
  • Let's open the discussion about how we are going to manage importing KHR_materials_specular extension ...

@proog128
Copy link
Author

proog128 commented May 6, 2022

@julienduroure Really great that you are picking this up!

I can see specularColorFactor and specularColorTexture, but what about specularFactor and specularTexture ?
Seems there are not implemented in current PR

Both specular and specular color are implemented, see for example here and here. But there were multiple changes in the spec while and after I wrote the code. Therefore, there could be naming issues (e.g. sometimes specular color is called specular tint).

Btw. here is a comment in which I wrote up how to convert from Principled to glTF. Maybe it helps in understanding what the code does. Not sure if it is still up-to-date, but it definitely captures the general idea.

Because specularColorTexture is RGB and specularTexture is A, is that a good idea to export a single RGBA texture?

Splitting the texture into specular and specular color textures was a late addition to the spec. Nevertheless, even though the spec allows to have separate textures, it encourages the use of a single texture if both specular color and specular are textured. This is possible because specular is stored in A and specular color in RGB channels.

@julienduroure
Copy link
Collaborator

Thanks @proog128 for feedback.
What I mean about "not implemented" is that, in your current draft implementation, Blender specular, Blender specular tint, Blender IOR and Blender transmission are used to calculate glTF specularColorFactor and glTF specularColorTexture.
But it seems that glTF specularFactor and/or specularTexture are never exported.
I am not really confident about my understanding of rendering equations/conversion, so I am not sure exactly how it works, and I would like to understand why these 2 parameters are not exported.

For info, here is the PR with current implementation (with basic conversion based on your own PR) : #1646

@proog128
Copy link
Author

Blender Principled BSDF does not have an equivalent for glTF specular. Therefore, the exporter keeps glTF specular at its default of 1.

specular in Blender Principled BSDF modifies f0 of the Fresnel equation.

specular in glTF modifies the overall (directionally independent) strength of the reflection effect. When using a Schlick Fresnel (as it is done in most glTF PBR implementations), this corresponds to modifying f0 and f90 simultaneously.

glTF also has the option to modify only f0, but it is called specular color. Thus, Blender specular is mapped to glTF specular color.

@julienduroure
Copy link
Collaborator

Ok, thanks @proog128 .
You answer leads to another question: How to handle import :

  • glTF can use RGB specular color, where Blender uses only tint. So there will no 100% compatibility. What I did currently:
    • For texture, use RGB to BW node (I didn't check how this is done internally)
    • For Factor, use luminence to convert from Vec3 to scalar
  • SpecularColor import conversion probably need some update to be more accurate ?
  • How to handle glTF specularFactor and glTF specularTexture?
    • Is there a way to manage it correcty, or how can we approximate as best as possible?
    • Some example : How to manage specularFactor 0.0 ?

@proog128
Copy link
Author

I created a notebook showing the conversions here (Interactive notebook on Colab). Maybe it helps!

SpecularColor import conversion probably need some update to be more accurate ?

Using luminance is ok, I guess. Maybe it's possible to optimize the perceived conversion result, but I haven't thought about it yet.

How to handle glTF specularFactor and glTF specularTexture?

In the general case: not possible. For some edge cases it can be approximated, for example if specular_gltf == 0 and transmission_gltf == 0, then specular_blender = 0. But that's only a special case if specular is near zero. In the general case, specular_gltf cannot be mapped to Blender.

Special handling for these edge cases breaks round-trip conversion and may look strange in combination with textures, so I am not sure if it's a good idea...

@julienduroure
Copy link
Collaborator

Thanks a lot. I will have a look soon

@julienduroure
Copy link
Collaborator

We can now close this PR, as specular is now managed in master by merging #1646

@julienduroure julienduroure mentioned this pull request Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exporter This involves or affects the export process Material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants