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

Fix #1956 merge channels (metallic/Roughness) : use default value #1968

Merged
merged 6 commits into from
Sep 6, 2023

Conversation

julienduroure
Copy link
Collaborator

Fix #1956
When we have to concatenate multiple channels into an image (for example MetallicRoughness).
If only 1 texture is provided, the other channel was filled with 1.0
Now this is filled with the default value of the socket

@julienduroure
Copy link
Collaborator Author

Tests still need to be updated, as default value from sockets are not taken into account in existing test files

@julienduroure julienduroure added this to the Blender 4.0 milestone Aug 16, 2023
@julienduroure julienduroure merged commit dd58f43 into main Sep 6, 2023
2 checks passed
@julienduroure julienduroure deleted the fix_1956_default_socket_value_merge_channels branch September 6, 2023 15:33
@maoliver-amd
Copy link

This doesnt seem correct to me. I noticed an issue after updating to blender 4.0 which seems connected to this PR.
From my understanding of the spec the rough/metal value in the presence of a texture is equal to the base factor multiplied by the texture value. Currently the same value is being written to the base factor ASWELL as now to the texture component (e.g. if we had a base roughness of 0.1 then the output gltf has 0.1 written into the texture and 0.1 written to roughnessFactor which results in incorrectly calculated final roughness = 0.1*0.1).

The default value in a compacted texture should be 1.0 when no texture is connected. We actually internally check for this and dump the entire texture channel during decoding to save memory (which no longer works after this PR)

From the spec:

The value for each property MAY be defined using factors and/or textures (e.g., baseColorTexture and baseColorFactor). If a texture is not given, all respective texture components within this material model MUST be assumed to have a value of 1.0. If both factors and textures are present, the factor value acts as a linear multiplier for the corresponding texture values.

@julienduroure
Copy link
Collaborator Author

@maoliver-amd
Do you mean that you have a Metallic texture and no roughness texture, but a value of 0.1 for roughness?
Can not reproduce in that case.

Can you please open a ticket with a sample .blend file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expected behavior of pbrMetallicRoughness's metallicRoughness textures
2 participants