Shader: Add specular tint to Glass BSDF #114354

Alaska · 2023-11-01T06:53:04+01:00

Alaska commented

2023-11-01 06:53:04 +01:00

Add specular tint to Glass BSDF so it matches the transmissive
component of the Principled BSDF.

Specular tint behaves differently from before.
The F90 reflection is no longer tinted.

Add specular tint to Glass BSDF so it matches the transmissive component of the Principled BSDF. Specular tint behaves differently from before. The F90 reflection is no longer tinted.

Alaska added the

Module

Render & Cycles

label 2023-11-01 06:53:04 +01:00

Alaska changed title from ~~Shader: Add specular tint to Glass BSDF~~ to WIP: Shader: Add specular tint to Glass BSDF

2023-11-01 06:53:33 +01:00

Alaska changed title from ~~WIP: Shader: Add specular tint to Glass BSDF~~ to Shader: Add specular tint to Glass BSDF

2023-11-01 08:27:29 +01:00

Alaska commented

2023-11-01 08:27:35 +01:00

A few notes about this:

If you use the same colour for "Color" and "Specular Tint", this will be the closest match to how the shader worked prior to this pull request. The most noticable difference between that setup and the old version of the node is that F90 is now no longer tinted.
In EEVEE, the Glass BSDF now matches the transmissive material in the Principled BSDF.
In Cycles the Glass BSDF does not match the transmissive material in the Principled BSDF. This is because the Principled BSDF does sqrt(base_color) to match EEVEE while the Glass BSDF did not. How would you like this handled? sqrt() on the Cycles Glass BSDF as well? sqr() on the EEVEE Glass BSDF? Leave it as is? Something else?
Ideally for versioning, I would like to copy any animation data/drivers, etc from the "Color" input to the "Specular Tint" input. I don't know how to do this. Do you have any advice on how to do this? Do you think it's even required?

Here's some comparisons:

Main vs Pull request comparison:

Main	This Pull request

In the example above, versioning ensures that when older files are opened in Blender, the "Specular tint" is set to the "Color" of the Glass BSDF to try and ensure consistency between versions. Even with the versioning you will notice notice that the reflections are different between main and this pull request. Specifically F90 is tinted in main, but not in this pull request. So there is a rendering difference that comes from this change.

Cycles Principled BSDF vs Glass BSDF - Purple Glass, Orange Tint:

Principled BSDF	Updated Glass	Updated Glass with sqrt(color)

As you can see from the example above, the Glass BSDF needs to use sqrt(color) to match the Principled BSDF when using Cycles.

EEVEE Principled BSDF vs Glass BSDF - Purple Glass, Orange Tint:

Principled BSDF	Updated Glass	Updated Glass with sqrt(color)

As you can see from the example above, the Glass BSDF doesn't need sqrt(color) to match the Principled BSDF in EEVEE.

A few notes about this: - If you use the same colour for "Color" and "Specular Tint", this will be the closest match to how the shader worked prior to this pull request. The most noticable difference between that setup and the old version of the node is that F90 is now no longer tinted. - In EEVEE, the Glass BSDF now matches the transmissive material in the Principled BSDF. - In Cycles the Glass BSDF does not match the transmissive material in the Principled BSDF. This is because the Principled BSDF does `sqrt(base_color)` to match EEVEE while the Glass BSDF did not. How would you like this handled? `sqrt()` on the Cycles Glass BSDF as well? `sqr()` on the EEVEE Glass BSDF? Leave it as is? Something else? - Ideally for versioning, I would like to copy any animation data/drivers, etc from the "Color" input to the "Specular Tint" input. I don't know how to do this. Do you have any advice on how to do this? Do you think it's even required? --- Here's some comparisons: **Main vs Pull request comparison:** |Main|This Pull request| |-|-| |![Main - Cycles - Green Glass.png](/attachments/e10782ff-be23-4900-99bb-3aa97c6b9f64)|![PR - Cycles - Green Glass.png](/attachments/336630d1-6c07-4eaa-9a7d-461eadc7f46e)| In the example above, versioning ensures that when older files are opened in Blender, the "Specular tint" is set to the "Color" of the Glass BSDF to try and ensure consistency between versions. Even with the versioning you will notice notice that the reflections are different between main and this pull request. Specifically F90 is tinted in main, but not in this pull request. So there is a rendering difference that comes from this change. --- **Cycles Principled BSDF vs Glass BSDF - Purple Glass, Orange Tint:** |Principled BSDF|Updated Glass|Updated Glass with sqrt(color)| |-|-|-| |![Cycles - Purple-Orange Principled BSDF.png](/attachments/eb4fa8a0-d335-4dea-9a18-ed08bd0fd852)|![Cycles - Purple-Orange Updated Glass BSDF.png](/attachments/6e4cbf8c-bfda-457a-9f23-f1fae8a09ee4)|![Cycles - Purple-Orange Updated Glass BSDF - sqrt.png](/attachments/61c20b1c-ca4c-4955-8be8-5f525d9fee4c)| As you can see from the example above, the Glass BSDF needs to use `sqrt(color)` to match the Principled BSDF when using Cycles. --- **EEVEE Principled BSDF vs Glass BSDF - Purple Glass, Orange Tint:** |Principled BSDF|Updated Glass|Updated Glass with sqrt(color)| |-|-|-| |![EEVEE - Purple-Orange Principled BSDF.png](/attachments/fdadb9f5-61b2-4a12-8ff1-9816e3fca053)|![EEVEE - Purple-Orange Updated Glass BSDF.png](/attachments/0e521ced-bc11-4d12-b2c2-2bed6bb52609)|![EEVEE - Purple-Orange Updated Glass BSDF - sqrt.png](/attachments/9f5a8d10-efb0-48e7-8040-9c78657819f5)| As you can see from the example above, the Glass BSDF doesn't need `sqrt(color)` to match the Principled BSDF in EEVEE.

Main - Cycles - Green Glass.png

909 KiB

Cycles - Purple-Orange Updated Glass BSDF.png

938 KiB

PR - Cycles - Green Glass.png

948 KiB

EEVEE - Purple-Orange Principled BSDF.png

656 KiB

EEVEE - Purple-Orange Updated Glass BSDF.png

656 KiB

Cycles - Purple-Orange Updated Glass BSDF - sqrt.png

1001 KiB

Cycles - Purple-Orange Principled BSDF.png

1001 KiB

EEVEE - Purple-Orange Updated Glass BSDF - sqrt.png

678 KiB

Alaska requested review from Brecht Van Lommel 2023-11-01 08:27:40 +01:00

Alaska requested review from Lukas Stockner 2023-11-01 08:27:48 +01:00

Alaska force-pushed update-glass-bsdf from 132e5ad26d to b063073879

2023-11-05 09:09:36 +01:00

Compare

Brecht Van Lommel commented

2023-11-06 10:16:06 +01:00

@blender-bot build

Alaska commented

2023-11-06 12:14:01 +01:00

The build bot failed. Specifically with one render test involving glass. T45609

Prior to the change, the entire reflection would be tinted by the "Color" input. After the change, and after applying the versioning code, only the F0 of the reflection will be tinted by the old Color input, while F90 is white.

In the case where "Color" was (1.0, 1.0, 1.0), the reflection will appear the same as older versions of Blender. If the Color is not that, then the reflection will be different as you deviate from the F0 reflection.

In the T45609 test, the color is (0.8, 0.8, 0.8) and is expected to have differences.

If the test library included more tests with non (1.0, 1.0, 1.0) glass, then more tests would fail. Maybe extra tests should be added?

The build bot failed. Specifically with one render test involving glass. T45609 Prior to the change, the entire reflection would be tinted by the "Color" input. After the change, and after applying the versioning code, only the F0 of the reflection will be tinted by the old Color input, while F90 is white. In the case where "Color" was (1.0, 1.0, 1.0), the reflection will appear the same as older versions of Blender. If the Color is not that, then the reflection will be different as you deviate from the F0 reflection. In the T45609 test, the color is (0.8, 0.8, 0.8) and is expected to have differences. ![114354 - Failed Test.png](/attachments/9d58a789-a5a8-4c4b-a043-e8d65bb57ec7) --- If the test library included more tests with non (1.0, 1.0, 1.0) glass, then more tests would fail. Maybe extra tests should be added?

114354 - Failed Test.png

209 KiB

Alaska added 1 commit 2023-11-07 05:53:21 +01:00

5d992b8f68 Merge branch 'main'

Conflicts:
	source/blender/blenloader/intern/versioning_400.cc

Alaska added 1 commit 2023-11-07 06:35:58 +01:00

f9309a78df Fix incorrect merge with main

Alaska added 1 commit 2024-01-10 05:57:50 +01:00

17cc72a685 Merge branch 'main'

Conflicts:
	source/blender/blenkernel/BKE_blender_version.h
	source/blender/blenloader/intern/versioning_400.cc
	source/blender/gpu/shaders/material/gpu_shader_material_glass.glsl

Lukas Stockner reviewed 2024-01-29 00:49:22 +01:00

Lukas Stockner left a comment

Hm, this is a tricky one.

On the one hand, while the existing Glass node isn't really physically-based anyways (transmission tint should be volumetric absorption), this seems like an improvement in realism to me.

On the other hand, the Glass BSDF (like Glossy) is also sort of intended as a building block for more complex shader designs, and for that the simpler behavior (where the entire BSDF reacts linearly to the Color input) might be preferred. For users who want a material that behaves intuitively and realistically out of the box, we have Principled.

This also affects things like whether we should apply sqrt() - from a "intuitive use" perspective it makes sense, but from a "building block" perspective it is nicer without it.

On the code level, this seems fine to me. Copying the animation data is tricky, there only are helpers for moving it, but tbh i think it would be fine to just not bother here.

But on the functionality level, I'd leave this up to @brecht.

Hm, this is a tricky one. On the one hand, while the existing Glass node isn't really physically-based anyways (transmission tint should be volumetric absorption), this seems like an improvement in realism to me. On the other hand, the Glass BSDF (like Glossy) is also sort of intended as a building block for more complex shader designs, and for that the simpler behavior (where the entire BSDF reacts linearly to the Color input) might be preferred. For users who want a material that behaves intuitively and realistically out of the box, we have Principled. This also affects things like whether we should apply `sqrt()` - from a "intuitive use" perspective it makes sense, but from a "building block" perspective it is nicer without it. On the code level, this seems fine to me. Copying the animation data is tricky, there only are helpers for moving it, but tbh i think it would be fine to just not bother here. But on the functionality level, I'd leave this up to @brecht.

Alaska commented

2024-01-29 02:05:32 +01:00

@LukasStockner One of the reasons for creating this pull request (and others like the conductor BSDF, and adding the roughness input to the Subsurface Scattering Node) is to allow users to recreate the Principled BSDF from smaller nodes without having to use OSL. This allows users to experiment with replacing, removing, adding, or re-ordering layers in the Principled BSDF (Obvious a layering option is still required).

So I would personally like to see the specular tint added to a smaller node, even if that's in the form of a separate "Generalized Schlick" node, or option on the Glass BSDF, or something similar.

The sqrt issue isn't exclusive to this pull request. The glass already has this issue in main resulting in Cycles and EEVEE rendering differently. So that's still something to consider even if we decide not to implement the specular tint changes in the Glass BSDF.

@LukasStockner One of the reasons for creating this pull request (and others like the [conductor BSDF](https://projects.blender.org/blender/blender/pulls/114958), and adding the [roughness input to the Subsurface Scattering Node](https://projects.blender.org/blender/blender/pulls/114499)) is to allow users to recreate the Principled BSDF from smaller nodes without having to use OSL. This allows users to experiment with replacing, removing, adding, or re-ordering layers in the Principled BSDF (Obvious a layering option is still required). So I would personally like to see the specular tint added to a smaller node, even if that's in the form of a separate "Generalized Schlick" node, or option on the Glass BSDF, or something similar. --- The `sqrt` issue isn't exclusive to this pull request. The glass already has this issue in main resulting in Cycles and EEVEE rendering differently. So that's still something to consider even if we decide not to implement the specular tint changes in the Glass BSDF.

❤️ 1

Clément Foucault commented

2024-01-30 09:41:13 +01:00

As you can see from the example above, the Glass BSDF doesn't need sqrt(color) to match the Principled BSDF in EEVEE.

It is difficult to judge with different background pattern. The issue is that EEVEE doesn't do 2 refraction events or just does an approximation. For now, consider the color applied as two refraction event (so current behavior) and later we will make it work depending on the refraction event count.

> As you can see from the example above, the Glass BSDF doesn't need sqrt(color) to match the Principled BSDF in EEVEE. It is difficult to judge with different background pattern. The issue is that EEVEE doesn't do 2 refraction events or just does an approximation. For now, consider the color applied as two refraction event (so current behavior) and later we will make it work depending on the refraction event count.

This pull request has changes conflicting with the target branch.

intern/cycles/kernel/svm/closure.h
source/blender/blenkernel/BKE_blender_version.h
source/blender/blenloader/intern/versioning_400.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u update-glass-bsdf:Alaska-update-glass-bsdf

git checkout Alaska-update-glass-bsdf

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Shader: Add specular tint to Glass BSDF #114354

Checkout