Skip to content

Conversation

@elalish
Copy link
Contributor

@elalish elalish commented Apr 12, 2023

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)
imageimage
(for historical reference) 1st draft / 2nd draft:
imageimage

Before / After:
imageimage
And it works with other CSS background colors since the canvas is transparent:
imageimageimageimage

It does require two extra steps to make it work in addition to this PR:
1) The renderer must have premultipliedAlpha: false
2) Must call renderer.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 default premultipliedAlpha to false, while the renderer defaults to true. Also, if you set renderer.setClearColor(new Color(0xFFFFFF));, it will ignore the alpha of your context and clear to alpha = 1, which is a bit confusing. Currently the background clears to black by default, but white seems just as reasonable?

@github-actions
Copy link

github-actions bot commented Apr 12, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
642.3 kB (159 kB) 642.6 kB (159.1 kB) +258 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
432.2 kB (104.7 kB) 432.4 kB (104.7 kB) +258 B

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 );
Copy link
Contributor Author

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.

Copy link
Collaborator

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.

Copy link
Contributor Author

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.

@WestLangley
Copy link
Collaborator

The renderer must have premultipliedAlpha: false

In theory, it can't, without breaking normal blending.

The three.js normal blending function outputs premultiplied values, hence renderer.premultipliedAlpha must remain true so that the drawing buffer is composited with the canvas background correctly.

However, it might be worth looking at these defaults, as materials default premultipliedAlpha to false, while the renderer defaults to true.

They are completely different properties, with different purposes. There is no reason why they should be the same.

@elalish
Copy link
Contributor Author

elalish commented Apr 12, 2023

@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 true. What blocked me is that transmission samples from the drawing buffer to determine incoming light and transparency. On the first pass, all that's there is the clear color, which we need to have alpha == 0 if our canvas is transparent. However, the renderer's premultipliedAlpha: true setting means no matter what we use for setClearColor, we always get black, since it's multiplied by zero.

I just need to get the background cleared to (1, 1, 1, 0) - might it be acceptable to add a premultipliedAlpha override input to setClearColor?

@elalish
Copy link
Contributor Author

elalish commented Apr 26, 2023

@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 material.transparent is only turned on now when opacity < 1, as this makes a fairly significant rendering difference on its own.

@elalish
Copy link
Contributor Author

elalish commented Apr 26, 2023

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:
image

@mrdoob
Copy link
Owner

mrdoob commented Apr 27, 2023

Adding screenshots for my own reference in the future.

This PR WL's PR
Screenshot 2023-04-27 at 11 57 06 Screenshot 2023-04-27 at 11 57 12
Screenshot 2023-04-27 at 11 58 23 Screenshot 2023-04-27 at 11 58 20
Screenshot 2023-04-27 at 11 57 28 Screenshot 2023-04-27 at 11 57 34

@mrdoob mrdoob added this to the r153 milestone Apr 27, 2023
@WestLangley
Copy link
Collaborator

Here is an example with a CSS background (not a skybox).

WestLangley's PR

westlangley

This PR

elalish

@elalish
Copy link
Contributor Author

elalish commented May 5, 2023

@WestLangley Further improvements would be great! Would you mind responding to the main feature I added compared to yours? #25881 (comment)

@elalish
Copy link
Contributor Author

elalish commented May 10, 2023

@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: setClearColor( 0xffffff, 0.5 ), which I think is a decent middle ground between working for light backgrounds and working for dark backgrounds. For dark backgrounds, reducing either the color or the alpha moves the result in a good direction. For light backgrounds, the opposite is true, but reducing the color has a more extreme negative impact than reducing the alpha. I also like this compromise because it only involves 1 arbitrary constant rather than two (since multiplying by #ffffff is a no-op).

setClearColor( 0xffffff, 0.5 ) (this PR):
imageimage
setClearColor( 0x7f7f7f, 0.8 ) (#26003):
imageimage
Regarding your example (or as close as I could manage): agreed that my compromise is doesn't do as well for it, but it's at least improved a bit and I still think it is outweighed by the examples above.
image

@donmccurdy
Copy link
Collaborator

donmccurdy commented May 10, 2023

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 #E96143, it would be convenient to be able to set this once and move on:

renderer.setClearColor( 0xE96143, 0.0 );

@elalish
Copy link
Contributor Author

elalish commented May 10, 2023

@donmccurdy This setClear is actually buried in the renderer and not available in the public API. The idea is that this compromise value does a decent job for all CSS backgrounds (though obviously not perfect). If you know your background ahead of time, you're probably better off just applying it as a real three.js background so the rendering is really correct. This PR allows it to still work pretty well when the 3D developer and the CSS developer don't know about each other.

@donmccurdy
Copy link
Collaborator

donmccurdy commented May 10, 2023

@elalish oops sorry! I noticed my mistake and edited my comment while you were responding.

If you know your background ahead of time...

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.

@elalish
Copy link
Contributor Author

elalish commented May 10, 2023

@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?

@donmccurdy
Copy link
Collaborator

donmccurdy commented May 10, 2023

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... 😇

@WestLangley
Copy link
Collaborator

WestLangley commented May 10, 2023

Regarding your example (or as close as I could manage): agreed that my compromise is doesn't do as well for it,

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 );

Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

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.

@elalish elalish requested a review from WestLangley May 10, 2023 22:11
@mrdoob
Copy link
Owner

mrdoob commented May 11, 2023

Do you mind removing the builds from the PR?
And reverting the changes in webgl_loader_gltf_transmission?

@elalish
Copy link
Contributor Author

elalish commented May 11, 2023

Do you mind removing the builds from the PR? And reverting the changes in webgl_loader_gltf_transmission?

Oh, good call; done!

@mrdoob mrdoob merged commit f7dff10 into mrdoob:dev May 12, 2023
@mrdoob
Copy link
Owner

mrdoob commented May 12, 2023

@elalish FYI

79cc93c#diff-6f5dde4e2eee5725efad5ce991ebe25cbf3c000fa3e26f84a11292fe788f661fL147-L152

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