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

Add 2025x spec version #36

Merged
merged 1 commit into from
Jun 25, 2024
Merged

Add 2025x spec version #36

merged 1 commit into from
Jun 25, 2024

Conversation

bsdorra
Copy link
Member

@bsdorra bsdorra commented Mar 12, 2024

  • Add thin-film interference
  • Add translucency color
  • Add 25x parameter list
  • Update localization files with placeholder

TODO:

  • User Guide update
  • Rough transmission energy compensation

@bsdorra bsdorra requested a review from proog128 March 12, 2024 07:37
Copy link
Member

@proog128 proog128 left a comment

Choose a reason for hiding this comment

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

This is really nice work and energy compensation looks great. Spec-wise I think it still requires a bit of work to flesh out all details and make the spec self-contained again, moreover it would be nice to explain the derivation of the energy compensation in more depth (similar to Section 6.2.1). Let me know if you need another review!

params-2025x.json Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Show resolved Hide resolved
params-2025x.json Outdated Show resolved Hide resolved
params-2025x.json Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved

Based on the thin-film layer weight $i$, we blend the original Fresnel with the thin-film reflectance for the specular core components
$$
OpaqueCore(...) = \mathbf{M}_r (1-i)F(...) + \mathbf{M}_r i R(...)...
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I don't understand it :-). What exactly is replaced? How can I read from this that specular and specular tint are not part of the thin film Fresnel? Thin film is rather invasive, and it probably affects Seq. 1 where we introduce the base BRDFs $\text{Metallic}$, $\text{ThinTransparentDielectric}$, $\text{VolumetricTransparentDielectric}$, and $\text{OpaqueDielectric}$.

spec-2025x.md.html Outdated Show resolved Hide resolved

Based on the thin-film layer weight $i$, we blend the original Fresnel with the thin-film reflectance for the specular core components
$$
OpaqueCore(...) = \mathbf{M}_r (1-i)F(...) + \mathbf{M}_r i R(...)...
Copy link
Member

Choose a reason for hiding this comment

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

The math should really be treated like code. It doesn't help if we simplify by leaving out details, redefining symbols (ambigous symbols), introducing unused functions or calling functions with the wrong number of arguments. Text is just comments, the math must give a correct program even without the text.


Based on the thin-film layer weight $i$, we blend the original Fresnel with the thin-film reflectance for the specular core components
$$
OpaqueCore(...) = \mathbf{M}_r (1-i)F(...) + \mathbf{M}_r i R(...)...
Copy link
Member

Choose a reason for hiding this comment

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

Ah, maybe I understand now what you mean. Is it the following?

$$ \begin{align} \text{ThinFilmMetallic} &= (1-i) \text{Metallic} + i M_r R \\ \text{ThinFilmOpaqueDielectric} &= (1-i) \text{OpaqueDielectric} + i (B + s M_r R) \\ \text{ThinFilmThinTransparentDielectric} &= (1-i) \text{ThinTransparentDielectric} + i (M_r R + \rho M_t (1 - R)) \\ \text{ThinFilmVolumetricTransparentDielectric} &= (1-i) \text{VolumetricTransparentDielectric} + (M_r R_d + \rho M_t (1 - R_d)) \\ \end{align} $$

This needs to be fleshed out (add all parameters, format it nicely etc.). Moreover, you need to update the definition of $\text{Core}$ (Eq. 13) and $\text{ThinCore}$/$\text{VolumetricCore}$, and everything that refers to it (Sheen, Appendix, ...):

$$ \begin{eqnarray} &\text{ThinFilmCore}(\rho, \rho_l, s, \eta, \rho_s, i, d_i, \eta_i) = \\ &\quad\quad m \text{ThinFilmMetallic}(\rho, i, d_i, \eta_i) \nonumber\\ &\quad\quad +(1-m)(1-t) \text{ThinFilmOpaqueDielectric}(\rho, \rho_l, s, \eta, \rho_s, i, d_i, \eta_i) \nonumber\\ &\quad\quad +(1-m)t \text{ThinFilmTransparentDielectric}(\rho, s, \eta, \rho_s, i, d_i, \eta_i) \nonumber \end{eqnarray} $$

This will cause quite a lot of non-straightforward refactoring work, I suppose, but it will be worth it. At the moment it's really unclear how exactly all those parts interact...

@bsdorra
Copy link
Member Author

bsdorra commented Apr 9, 2024

Yes, that's what I had in mind. I found the idea of explicitely rewriting all of the core components just too verbose to communicate the simple idea of replacing the microfacet/fresnel product with a weighted sum. From your reaction it becomes clear that it doesn't really work. I'll stick to the formal approach and rework this based on your suggestion. Thanks!

Copy link
Member

@proog128 proog128 left a comment

Choose a reason for hiding this comment

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

Thank you for this refactoring, I think it's a lot clearer now. Last time I thought it would require a much more invasive refactoring of the spec, but I think you found now a very good combination of keeping the changes local (inside the thin-film section) and describing its global effects.

spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Outdated Show resolved Hide resolved
spec-2025x.md.html Show resolved Hide resolved
- Add thin-film interference
- Add translucency color
- Add 25x parameter list
- Update localization
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.

2 participants