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 transmission prerender pass when using KHR_materials_transmission #8623

Merged
merged 9 commits into from
Sep 1, 2020

Conversation

MiiBond
Copy link
Contributor

@MiiBond MiiBond commented Jul 27, 2020

To better support transmission as defined in the glTF KHR_materials_transmission extension, here's an example of a Helper that can setup the rendering of the opaque scene behind the transmissive object(s).
To enable it, add this line:
var transHelper = new BABYLON.TransmissionHelper({renderSize: 1024}, scene);

Currently, transmissive surfaces just show the IBL through them like shown here:
Without helper

With the helper running, transmissive surfaces will show the opaque scene behind instead:
With helper

Looking for feedback. Is this even the right way to handle this issue?

@deltakosh deltakosh requested a review from sebavan July 28, 2020 14:43
Copy link
Member

@sebavan sebavan left a comment

Choose a reason for hiding this comment

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

It looks pretty cool :-)

I guess the next steps could be to fill the dispose in ensure that mesh.material.getRenderTargetTextures = null; is not necessarily set as I am afraid it might break some setups.

Then we could go in a deeper review ?

@sebavan
Copy link
Member

sebavan commented Jul 28, 2020

I am also wondering at what extend we could rely on our realtime filtering feature to ensure an accurate roughness ?

@sebavan
Copy link
Member

sebavan commented Jul 28, 2020

I also wonder if it should be derived into ScreenSpaceRefraction as we are doing for reflection as it can be extended to standard mat ?

I know it is a lot of questions but I love the idea very much !!!

@MiiBond
Copy link
Contributor Author

MiiBond commented Jul 28, 2020

I've added dispose and removed getRenderTargetTextures = null (I think it was necessary in an earlier version of this).
I didn't know that there was an option for realtime-filtering a texture. That sounds great but I'll have to dive into how it works. It looks like it's a flag on a material and not a texture right now and when I enable it, I get shader compile errors.

@sebavan
Copy link
Member

sebavan commented Jul 28, 2020

yup it is only at the material level as we did not split refraction from reflection in this mode which is smthg I could add later.

I bet the error comes from texture 2d not being supported at the moment (only cube) but it would be so awesome to introduce this part :-)

@MiiBond
Copy link
Contributor Author

MiiBond commented Jul 29, 2020

It would be great to have this for 2D refraction textures, for sure.
I have a version of this helper for our viewer's transparency code which is far more complicated (and horribly messy) to handle multiple layers of transmissive objects but, ultimately, it still generates a 2D refraction texture to be used by the material.

@deltakosh
Copy link
Contributor

We have some checks issue (mostly doc and linting)

@bghgary bghgary changed the title Initial test of a "transmission" helper Add transmission prerender pass when using KHR_materials_transmission Aug 11, 2020
Copy link
Contributor

@bghgary bghgary left a comment

Choose a reason for hiding this comment

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

Mostly nitpick comments.

Copy link
Contributor

@bghgary bghgary left a comment

Choose a reason for hiding this comment

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

This looks good to me. @sebavan Do you want to look at it further?

@deltakosh deltakosh merged commit 50172d6 into BabylonJS:master Sep 1, 2020
@sebavan
Copy link
Member

sebavan commented Sep 1, 2020

Actually, @MiiBond, what is calling dispose on the transmission helper ? I am wondering if it would not leak ?

@MiiBond
Copy link
Contributor Author

MiiBond commented Sep 1, 2020

@sebavan the helper has global scope and only one should ever be created. We don't want it to be cleaned up when the loader is and we don't want another one to be created when another loader is.

@sebavan
Copy link
Member

sebavan commented Sep 1, 2020

@MiiBond I was thinking on scene.dispose, should we add a scene.onDisposeObservable to call the helper dispose function ?

@MiiBond
Copy link
Contributor Author

MiiBond commented Sep 2, 2020

@sebavan Quite right. I'll open another PR.

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.

5 participants