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

Improve MeshPhysicalMaterial transmission support #21884

Merged
merged 10 commits into from
May 26, 2021

Conversation

takahirox
Copy link
Collaborator

@takahirox takahirox commented May 24, 2021

Related issue: #16977, #21000

Description

Sorry for the late but this is a PR for improving MeshPhysicalMaterial transmission support.

The screenshot of a new glTF transmission example.

image

Changes

  • Add thickness, thicknessMap, attenuationDistance, and attenuationColor properties to MeshPhysicalMaterial.
  • Add webgl_loader_gltf_transmission example.
  • Add transmission_(pars_)fragment.glsl.js which are primarily based on glTF-Sample-Viewer shader codes. Integrate transmissionmap_(pars_)fragment.glsl.js with them.
  • Add transmissive objects render path to WebGLRenderer. Refer to Intent to implement/contribute Enterprise PBR improvements #16977 (comment) for the detail.

Notes

  • This PR doesn't include KHR_materials_ior and KHR_materials_volume extension supports in GLTFLoader. I would make a follow up PR if this change is merged.
  • Please feel free to make follow up PRs for cleaning up, optimization, and improvement. /cc @whatisor
  • Regarding the shader codes, I almost just copied from glTF-Sample-Viewer shader codes. I applied this part but haven't this part yet (because I haven't perfectly understood that part yet). Please feel free to make a follow up PR to apply that part.
  • I haven't finely compared with the reference screenshot yet. I would be happy if you help.

Limitations

  • I haven't checked yet but probably it doesn't work with XR yet. Follow up PR is welcome.

@WestLangley
Copy link
Collaborator

@takahirox Thanks for this! :-)

The previous transmission implementation -- which was just a stub -- looked like this when transmission was set to 1.

Screen Shot 2021-05-24 at 7 53 28 AM

Now, it darkens when transmission is set to 1... More light should transmit, not less, no?
Screen Shot 2021-05-24 at 7 53 39 AM

@cx20
Copy link
Contributor

cx20 commented May 24, 2021

I used raw.githack.com to run this sample. However, the result was not as shown in the thumbnail on the left.

https://rawcdn.githack.com/takahirox/three.js/d6df4b39033dcf42c139dcef9c700b18880774d1/examples/index.html?q=gltf#webgl_loader_gltf_transmission
image

The environment tested was Windows 10 + RTX 2060 + Chrome 90.

@mrdoob
Copy link
Owner

mrdoob commented May 24, 2021

@cx20 raw.githack.com doesn't work in this case. I'll try to merge this ASAP.

// transmission

let _transmissionRenderTarget = null;
const _transmissionSamplerSize = new Vector2( 1024, 1024 ); // Should be configurable?
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be the same as the output render dimensions? Without refraction, the transmission lines up exactly with the viewport.

const transparentObjects = currentRenderList.transparent;

if ( opaqueObjects.length > 0 ) renderObjects( opaqueObjects, scene, camera );
if ( transmissiveObjects.length > 0 ) renderTransmissiveObjects( opaqueObjects, transmissiveObjects, scene, camera );
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there some way we could not have to render the opaque objects twice? It seems like we could just copy the previous buffer into the tranmissionSamplerMap.

Copy link
Owner

Choose a reason for hiding this comment

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

With WebGL2 we'll be able to avoid rendering twice, but with WebGL 1 we can't have antialias when rendering into a render target 😕

Copy link
Contributor

@elalish elalish May 24, 2021

Choose a reason for hiding this comment

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

Well, wouldn't we still be able to take the normal anti-aliased framebuffer we just rendered the opaque objects into and copy that into the transmissionSamplerMap for the next pass? Instead of rendering it all over again? This would also remove the question around what size the samplerMap should be.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can WebGL 1 generate mipmaps for non-power-of two textures? The current implementation uses the lower level mips for roughness transmission.

Copy link
Owner

Choose a reason for hiding this comment

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

AFAIK, it can't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, forgot about that. Could we sample it into a power-of-two texture, instead of a pure copy?

vec3 f0 = vec3( pow( ior - 1.0, 2.0 ) / ( pow( ior + 1.0, 2.0 ) ) );
vec3 f90 = vec3( 1.0 );

vec3 f_transmission = totalTransmission * getIBLVolumeRefraction(
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, this isn't just tranmission support, it's also volume and ior? Awesome!

if ( json.transmission !== undefined ) material.transmission = json.transmission;
if ( json.thickness !== undefined ) material.thickness = json.thickness;
if ( json.attenuationDistance !== undefined ) material.attenuationDistance = json.attenuationDistance;
if ( json.attenuationColor !== undefined && material.attenuationColor !== undefined ) material.attenuationColor.setHex( json.attenuationColor );
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. This can support the glTF volume and ior extensions (which are just going in now for ratification), but we haven't yet added these to GLTFLoader. That will be exciting!

@takahirox
Copy link
Collaborator Author

takahirox commented May 24, 2021

@WestLangley

Thanks for the report. Yeah, I think transmission = 1 should look more transmissive. I'll take a look...

@WestLangley
Copy link
Collaborator

WestLangley commented May 24, 2021

@takahirox I understand your use of the glFT fidelity test case, but IMHO, I'd leave that for glTF, and instead focus on enhancing the GUI (and model, if necessary) in the current three.js transmission test case.

By being able to vary the parameters, one can see if the new shader is working correctly.

@takahirox
Copy link
Collaborator Author

Looking better?

image

@mrdoob
Copy link
Owner

mrdoob commented May 26, 2021

Excellent work @takahirox!

I'm going to merge this and try using renderer.copyFramebufferToTexture() to avoid drawing the opaque list twice (thanks @elalish!)

@mrdoob mrdoob merged commit bbe6f1e into mrdoob:dev May 26, 2021
@mrdoob
Copy link
Owner

mrdoob commented May 26, 2021

Thanks!

@cx20
Copy link
Contributor

cx20 commented May 26, 2021

I tried the latest version of the three.js library and glTF Loader with gltf-test.
I think the result is very good.

Click on the screenshot below to see a sample.

model three.js r128 + GLTF Loader result three.js r129dev + GLTF Loader result
Transmission Test image image
Toy Car image image

@takahirox takahirox deleted the Transmission branch May 26, 2021 14:57
@WestLangley
Copy link
Collaborator

@takahirox Can you recheck the metalness = 1 case? Pure metals are not transparent, so in that case, the value of transmission should have no effect.

@mrdoob
Copy link
Owner

mrdoob commented May 26, 2021

@elalish

I tried using copyFramebufferToTexture but seems like the render target needs to be the same size as the framebuffer (which is not power of 2 and everything breaks...)

I guess we'll need WebGL 2 for that?

@mrdoob
Copy link
Owner

mrdoob commented May 26, 2021

@takahirox Seems like Safari is not liking the glsl in this PR...

Screenshot 2021-05-26 at 19 38 51

Any ideas?

@takahirox
Copy link
Collaborator Author

takahirox commented May 26, 2021

@mrdoob

Safari doesn't support WebGL2 and GLSL3 yet, does it? It seems I should have written texture2D and texture2DLodEXT instead of texture and textureLod in the shader for WebGL1 and GLSL1.

And we need to add the following lines if we use texture2DLodEXT.

#extension GL_EXT_shader_texture_lod : enable
#extension GL_OES_standard_derivatives : enable

https://developer.mozilla.org/en-US/docs/Web/API/EXT_shader_texture_lod

@mrdoob
Copy link
Owner

mrdoob commented May 26, 2021

@takahirox Thanks! 90abd4f

@takahirox
Copy link
Collaborator Author

takahirox commented May 26, 2021

@WestLangley

@takahirox Can you recheck the metalness = 1 case? Pure metals are not transparent, so in that case, the value of transmission should have no effect.

Good catch. In the current shader, I first apply the transmissionSamplerMap to diffuseColor.

And then material.diffuseColor will be calculated from diffuseColor but it will be vec3(0.0) if metalness is 1.

The problem is diffuseColor has an effect to material.specularColor even if metalness is 1.

Perhaps material.specularColor should be calculated from the raw diffuseColor?

@mrdoob
Copy link
Owner

mrdoob commented May 27, 2021

@cx20 Curious to see how these examples looks like after #21894 🤔

@whatisor
Copy link
Contributor

@takahirox I tried to figure out how transparent Plastic material work.
For plastic material, environment reflection should be almost zero. Can the current transmission model work if environment reflection intensity is almost 0?

@Mugen87 Mugen87 added this to the r129 milestone May 27, 2021
@cx20
Copy link
Contributor

cx20 commented May 27, 2021

@mrdoob I compared the results before and after the fix in #21894.
I think the metal band is no longer transparent.

model before after
Transmission Test image image

@WestLangley
Copy link
Collaborator

I think the metal band is no longer transparent.

Right, you can't tell. That test case is not very useful for debugging. It is too complex. What are the material settings, for example?

@cx20
Copy link
Contributor

cx20 commented May 27, 2021

@emackey Do you have any advice for us?

@WestLangley
Copy link
Collaborator

@cx20 A test case with a GUI, similar to https://threejs.org/examples/webgl_materials_physical_transmission.html, is much more useful for determining if the shader is physically plausible.

IMHO, of course. :-)

@emackey
Copy link

emackey commented May 27, 2021

I agree this particular glTF model mixes test cases without fully indicating which is which.

In the sphere shown here, transmission is 1.0 for the entire sphere, roughness is 0.0, and the metallic channel is where the swirls are happening at full strength.

For what it's worth, @cx20's "after" image looks correct to me, because I can no longer see the checkerboard cloth through half of the swirls (those are the metallic ones). So I believe this matches the spec for transmission in glTF.

But given infinite time, yes there should be a clearer glTF test model, where a metallic transmissive sphere is compared to a metallic non-transmissive sphere, and a dozen other such tests.

@WestLangley
Copy link
Collaborator

But given infinite time, yes there should be a clearer glTF test model

We already have webgl_materials_physical_transmission.html which is being improved with a more-extensive GUI.

@emackey
Copy link

emackey commented May 27, 2021

We already have webgl_materials_physical_transmission.html which is being improved with a more-extensive GUI.

That's great!

I'm looking at this from the Khronos side, where we (3D Formats) want to test visual consistency of glTF among a number of different rendering engines, including ThreeJS.

@joshuaellis joshuaellis mentioned this pull request May 27, 2021
11 tasks
@takahirox
Copy link
Collaborator Author

takahirox commented May 27, 2021

@mrdoob @Mugen87

Would you mind if adding @whatisor next to us in the r129 change log for MeshPhysicalMaterial transmission improvement? It ended up I alone made the PR but I'm sure he worked in #21000, too, and helped us go forward. I want to respect his work.

@mrdoob
Copy link
Owner

mrdoob commented May 27, 2021

@takahirox definitely! 👍

@Mugen87
Copy link
Collaborator

Mugen87 commented May 28, 2021

@takahirox Updated the release notes!

@WestLangley
Copy link
Collaborator

With all due respect, I still do not think this implementation is physically plausible. A fully-transmissive, white material is not that dark or opaque. I stand by #21884 (comment).

https://threejs.org/examples/webgl_materials_physical_transmission.html

@mrdoob
Copy link
Owner

mrdoob commented Jun 10, 2021

With all due respect, I still do not think this implementation is physically plausible. A fully-transmissive, white material is not that dark or opaque. I stand by #21884 (comment).

@whatisor any ideas?

@takahirox
Copy link
Collaborator Author

takahirox commented Jun 10, 2021

Trying to improve the transmission... Is this looking better?

image

image

@whatisor
Copy link
Contributor

whatisor commented Jun 15, 2021

@takahirox I think your new tuning needs to consider Metallic factor.
Follow specification from gltf, metallic should not be affected by transmission (otherwise, it is partial plastic)

Therefore, for a material with metallicFactor=1.0, the value of transmissionFactor doesn't matter.

--https://github.com/KhronosGroup/glTF/blob/master/extensions/2.0/Khronos/KHR_materials_transmission/README.md--

@takahirox
Copy link
Collaborator Author

@whatisor

Thanks for the comment.

new tuning

It means #21975, doesn't it?

consider Metallic factor

Yes, it does. material.diffuseColor passed to getIBLVolumeRefraction() is diffuseColor.rgb * ( 1.0 - metalnessFactor );

The screenshot of the both metallic and transmission are 1. It is no transparent.

image

@whatisor
Copy link
Contributor

oh, it seems to be from metallicRoughnessTexture , not factor itself.

"metallicFactor": 0.0, "metallicRoughnessTexture": { "index": 4 }

I remember reference result with metallic test.
screenshot_large_target

@takahirox
Copy link
Collaborator Author

Ah, sorry the screenshots in #21884 (comment) are bit old.

This is the current screenshot.
image

@whatisor
Copy link
Contributor

Super cool! I love this

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.

8 participants