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

KHR_materials_thinfilm extension branch. #1742

Closed

Conversation

UX3D-nopper
Copy link
Contributor

No description provided.

@bhouston
Copy link
Contributor

bhouston commented Feb 7, 2020

Could we simplify the parameters for this to just thinFilmThickness and thinFilmIOR?

I notice this is the parameterization used in the main reference paper here: https://hal.archives-ouvertes.fr/hal-01518344/document

It is also the parameterization used in Autodesk Standard Surface:

thin_film_thickness* | float | 0 | thickness of the film (in nanometres
thin_film_IOR* | float | 1.5 | refractive index of the film

https://autodesk.github.io/standard-surface/

Why was this more specific parameters chosen rather than the more physical parameterization that is also what everyone else is using? I highly advocate for the more physical ones as this allows for implementations to improve their physicality and we do not have to change the specification.

@UX3D-nopper
Copy link
Contributor Author

The parametrization - in general for glTF materials - is to feed the GPU directly. So the parameters above are sufficient for a path tracer, but not for the GPU - depending of course which algorithms etc. we use.
Internally, you can use Standard Surface or Enterprise PBR. From these parameters, you need to generate the data for real-time - as glTF is a transmisson format.

@UX3D-nopper
Copy link
Contributor Author

We can also discuss this in the break ;-)

@bhouston
Copy link
Contributor

bhouston commented Feb 7, 2020

I created a branch here with my very minor proposed changes:

I can not make a PR against your fork apparently.

@donmccurdy
Copy link
Contributor

As this PR is more recent than KHR_materials_transmission, I have to assume there are reasons you prefer it — would you mind expanding on the motivation? Am I correct in understanding that this extension (if ratified) would provide the features of both KHR_materials_transmission and KHR_materials_volume (which would then not be ratified)?

@UX3D-nopper
Copy link
Contributor Author

The KHR_materials_transmission is a different visual effect and from my point of view do not intersect. The same is for KHR_materials_volume .

@emackey
Copy link
Member

emackey commented Jun 11, 2020

Would it be reasonable to rename this extension to something more like KHR_materials_iridescence?

I ask because when I look at the long list of proposed PBR Next extensions, there are many that talk about "thin" and "coat" and IOR and transmission. I feel that this extension's name should distance itself from all those other proposed extensions.

@mosra mosra mentioned this pull request Jul 23, 2020
58 tasks
@emackey emackey mentioned this pull request Aug 21, 2020
@UX3D-nopper
Copy link
Contributor Author

@bhouston
Copy link
Contributor

There is still intent from Threekit for pushing this towards a full standard. It is something that arises often.

@UX3D-nopper
Copy link
Contributor Author

Yes, it is the roadmap for PBR Next. Maybe just join the Monday call.

@UX3D-nopper UX3D-nopper deleted the extensions/KHR_materials_thinfilm branch September 20, 2021 11:52
@UX3D-nopper
Copy link
Contributor Author

We continue here #2027

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

Successfully merging this pull request may close these issues.

5 participants