-
-
Notifications
You must be signed in to change notification settings - Fork 36.2k
Improved transparent rendering #25819
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
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
| return vec4( ( 1.0 - F ) * attenuatedColor * diffuseColor, transmittedLight.a ); | ||
| float transmittanceFactor = ( transmittance.r + transmittance.g + transmittance.b ) / 3.0; | ||
| return vec4( ( 1.0 - F ) * attenuatedColor, 1.0 - ( 1.0 - transmittedLight.a ) * transmittanceFactor ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the key to my new hack, which I feel is more defensible than what it replaces. As before, when transmittedLight.a == 1 (the scene is opaque), no change to the proper PBR computation occurs. As this transmissive material gets lighter and the scene behind gets more transparent, output alpha decreases to blend in more of the unknown CSS background color.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure this is needed.
It is best not to hack the physical model if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is addressing a specific opacity bug with your alternative approach: #25881 (comment)
In particular, without this, 20% of the background shows through even if the transmissive material is absorbing all or nearly all of the transmitted light. This hack still has no effect when the final rendered output is opaque.
In theory, it can't, without breaking normal blending. The three.js normal blending function outputs premultiplied values, hence
They are completely different properties, with different purposes. There is no reason why they should be the same. |
|
@WestLangley Interesting, thanks! I just read up on premultiplied alpha again - makes me think I should try and tweak this approach so that I can keep that I just need to get the background cleared to (1, 1, 1, 0) - might it be acceptable to add a |
src/renderers/shaders/ShaderChunk/transmission_pars_fragment.glsl.js
Outdated
Show resolved
Hide resolved
|
@WestLangley pulling in your changes we can now check your live example with my updates: https://raw.githack.com/elalish/three.js/transparency/examples/webgl_materials_physical_transmission.2.html Note that |
|
I like the 80% minimum opacity for transmissive objects (pulled from #25881) - it's pretty much the only arbitrary constant in here now and it makes a pretty nice compromise between retaining object saturation and still showing the background: |
|
@WestLangley Further improvements would be great! Would you mind responding to the main feature I added compared to yours? #25881 (comment) |
|
@WestLangley Good point, but this is an unavoidable tradeoff of our blend function restrictions. I'm going to focus only on our difference in clear color, since my other adjustment to allow dark transparent objects to approach opaqueness is orthogonal and only further improves the results (would love to discuss on a separate thread if you're unconvinced). I'm proposing a compromise:
|
|
Noting that the user's specified clear color is ignored and we are trying to choose one set of parameters for all possible backgrounds ... perhaps it is possible to use the given clear color in the transmissive pass, even if clear alpha is zero? As a user, if I have a CSS background that is renderer.setClearColor( 0xE96143, 0.0 ); |
|
@donmccurdy This |
|
@elalish oops sorry! I noticed my mistake and edited my comment while you were responding.
I'm really thinking of cases where the CSS background is not a solid color, but where you can still squint and say it's broadly such and such a color. Certainly if my CSS background is black-and-white checkers, I'm out of luck in any case. |
|
@donmccurdy That's an interesting idea. I was trying to make this PR minimally-invasive, so no one would have to change their code or add anything to get the benefit. Having configuration might be nice, but probably something to discuss in a follow-on PR? I actually found that picking the clear alpha to match the overall lightness of the color gave decent results too. Any thoughts @mrdoob? |
|
Sure, I don't mean to block either PR! It looked like we were stuck in an argument about whether to choose one-size-fits-all settings that look good on dark backgrounds or on bright backgrounds, so I thought perhaps passing through the user's clear color to the transmissive pass might unblock things... 😇 |
I agree, it doesn't. The use case my PR was developed for one in which the entire background is transparent, and the camera background or CSS shows through. |
| _this.getClearColor( _currentClearColor ); | ||
| _currentClearAlpha = _this.getClearAlpha(); | ||
| if ( _currentClearAlpha < 1 ) _this.setClearColor( 0xffffff, 0.5 ); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a change from my PR. Are you aware that the render target will have premultiplied values?
So the two values of interest in this PR are 0xffffff * 0.5 = 0x7f7f7f and alpha = 0.5.
In my PR, the values are 0x7f7f7f * 0.8 = 0x666666 and alpha = 0.8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, we talked offline. I think we're at consensus now. Correct me if I'm wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a subjective decision, and can be changed later.
|
Do you mind removing the builds from the PR? |
Oh, good call; done! |














This is a follow-on to #22425 and #22473 to make rendering transparent objects onto a transparent canvas work better so they can be composited onto DOM or AR content behind and look reasonably seamless. This is required as DOM-to-texture is not and will not be allowed due to security, and WebXR does not expose the camera feed by default, meaning those backgrounds are not available within our shaders. It also means what I'm doing is fundamentally a non-PBR hack, since blend functions can't represent light transport properly, and it's not possible to handle transmission roughness or refraction. However, we can improve significantly:
Before / Current (including #25881)




(for historical reference) 1st draft / 2nd draft:
Before / After:






And it works with other CSS background colors since the canvas is transparent:
It does require two extra steps to make it work in addition to this PR:1) The renderer must havepremultipliedAlpha: false2) Must callrenderer.setClearColor(new Color(0xFFFFFF), 0);I haven't included these in the spirit of avoiding breaking changes and because this unblocks model-viewer. However, it might be worth looking at these defaults, as materials defaultpremultipliedAlphatofalse, while the renderer defaults totrue. Also, if you setrenderer.setClearColor(new Color(0xFFFFFF));, it will ignore thealphaof your context and clear toalpha = 1, which is a bit confusing. Currently the background clears to black by default, but white seems just as reasonable?