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

Examples: webgl_loader_gltf_transmission issues #22009

Closed
WestLangley opened this issue Jun 17, 2021 · 32 comments
Closed

Examples: webgl_loader_gltf_transmission issues #22009

WestLangley opened this issue Jun 17, 2021 · 32 comments
Labels

Comments

@WestLangley
Copy link
Collaborator

WestLangley commented Jun 17, 2021

Reposting #21000 (comment) so as to consolidate some remaining issues.

  1. webgl_loader_gltf_transmission has the transparent property of each sphere to set to false. Is that what the loader should be doing? What should transparent be set to when transmission is non-zero?

  2. After setting transparent to true, only front-faces show the hot-spot. (This may be a known limitation, but @mrdoob just added support for properly rendering back-faces of double-sided meshes.) There should be two hot-spots.

Screen Shot 2021-06-16 at 1 53 43 PM

  1. FIXED: This is caused by issue 7. below.

Screen Shot 2021-06-16 at 1 51 42 PM

  1. FIXED Also, in Chrome and Firefox (not Safari)

THREE.WebGLProgram: gl.getProgramInfoLog() WARNING: Output of vertex shader 'vUv' not read by fragment shader

Transmission always sets the USE_UV flag.

  1. FIXED: If performant, I think the dimensions of the "transmission render target" should match the dimensions of the "current render target". Doing so will prevent artifacts like the following:

Screen Shot 2021-06-21 at 9 04 34 PM

  1. FIXED In the following code snippet, transmissionFactor is applied to getIBLVolumeRefraction() twice. I expect that is a physical modeling error.

vec3 transmission = transmissionFactor * getIBLVolumeRefraction(
normal, v, roughnessFactor, material.diffuseColor, totalSpecular,
pos, modelMatrix, viewMatrix, projectionMatrix, ior, thicknessFactor,
attenuationColor, attenuationDistance );
totalDiffuse = mix( totalDiffuse, transmission, transmissionFactor );

  1. FIXED Also in the above snippet, totalSpecular has units of radiance, and therefore cannot be the correct argument because the function expects a unit-less quantity. A reasonable guess would be material.specularColor, instead.
  • Device: Desktop
  • OS: macOS 11.4
  • Browser: Chrome, Firefox, Safari
  • Three.js version: r130dev

/ping @takahirox @donmccurdy

@Mugen87 Mugen87 added the Addons label Jun 17, 2021
@mrdoob mrdoob added this to the r130 milestone Jun 17, 2021
@donmccurdy
Copy link
Collaborator

I think that displaying transmissive materials (as defined by KHR_materials_transmission) through other transmissive materials may be considered a feature request. Displaying opaque materials and the environment map through the transmission — as we do now — was priority (1), displaying alpha blend materials through transmission is probably second (2), and transmission through transmission third (3).

Similarly, I believe I'm seeing (1) in https://sandbox.babylonjs.com/ and https://github.khronos.org/glTF-Sample-Viewer-Release/, with reflections only from the nearest transmission material. Adding (2) and (3) requires techniques like depth peeling; see comment by @MiiBond in #16977 (comment). This would certainly be a welcome addition, but we'd want the flexibility to configure the number of layers, or disable that entirely for performance.

@mrdoob
Copy link
Owner

mrdoob commented Jun 23, 2021

  1. Also, in Chrome and Firefox (not Safari)

THREE.WebGLProgram: gl.getProgramInfoLog() WARNING: Output of vertex shader 'vUv' not read by fragment shader

Transmission always sets the USE_UV flag.

Fixed ac6d30a

@mrdoob
Copy link
Owner

mrdoob commented Jun 23, 2021

  1. If performant, I think the dimensions of the "transmission render target" should match the dimensions of the "current render target". Doing so will prevent artifacts like the following:

Worth noting that this would only work in WebGL2 as we do need mipmaps.

@mrdoob
Copy link
Owner

mrdoob commented Jun 23, 2021

Another problem with the current transmission code is that it "breaks" in transparent contexts.

Here I'm setting alpha: true and renderer.domElement.backgroundColor = 'red':

Screen Shot 2021-06-23 at 6 42 18 PM

/cc @elalish

@mrdoob mrdoob modified the milestones: r130, r131 Jun 30, 2021
@elalish
Copy link
Contributor

elalish commented Jul 2, 2021

I consider this pretty important. The DOM uses transparency pretty extensively which is why model-viewer uses alpha: true. This way DOM backgrounds and gradients and such fit nicely into the scene and the 3D doesn't have to look like a rectangle. Of course this is non-physical, so we'll need some kind of simple approximation. I'm hoping we can just get the shader to set the alpha to whatever the transmission % of the pixel was, though we'll have to be careful to keep it opaque if it was already drawn to by something opaque. It won't blur the DOM background, but I think that's fine, and more or less in line with not seeing transparent objects through each other.

@gonnavis
Copy link
Contributor

gonnavis commented Jul 20, 2021

Hello @takahirox, Thanks for the PR! I found that current example has gaps if look from a side view or back view:
image
But the front view is OK.
I tried with a simple box, seems all refracted pixels are pushed to +Z axis a little?

@mrdoob
Copy link
Owner

mrdoob commented Jul 27, 2021

I tried with a simple box, seems all refracted pixel are pushed to +Z axis a little?

@takahirox Any ideas?

@takahirox
Copy link
Collaborator

takahirox commented Jul 27, 2021

Unfortunately I currently have lack of time to tackle this problem.

I tried with a simple box, seems all refracted pixel are pushed to +Z axis a little?

This guess sounds right to me. The ray is calculated in

src/renderers/shaders/ShaderChunk/transmission_pars_fragment.glsl.js

I'm happy if anyone can help the investigation in the details.

Update: The model looks fine if thickness is zero. So I suspect there is a problem in this function or the parameters passed to the function can be wrong.

@mrdoob
Copy link
Owner

mrdoob commented Jul 27, 2021

I've made a model that shows the issue: TransmissionThickness.gltf.zip

khronos babylon dev
Screenshot 2021-07-27 at 23 54 16 Screenshot 2021-07-27 at 23 54 43 Screenshot 2021-07-27 at 23 55 50

@takahirox
Copy link
Collaborator

Thanks for making the test model! The model looks correct in glTF sample viewer to which I referred for the transmission shader code.

image

So I guess I have wrongly editted the shader code calculating the ray or passed wrong parameters to the function.

@mrdoob mrdoob modified the milestones: r131, r132 Jul 28, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jul 28, 2021

@UX3D-eckerlein Any ideas of what we got wrong?

@mrdoob
Copy link
Owner

mrdoob commented Jul 29, 2021

@whatisor Is this something you dealt with in your implementation?

@takahirox
Copy link
Collaborator

takahirox commented Jul 30, 2021

Probably I figured out the root issue. I have passed wrong normal parameter to the function. The function requires world normal but I have passed world normal applied view matrix.

This is the screenshot of Three.js + test model with the fix.

image

I will make a PR maybe soon.

@takahirox
Copy link
Collaborator

#22224

@cx20
Copy link
Contributor

cx20 commented Jul 31, 2021

Thank you for fixing the shader. I tried again with the latest version of the library. Looks good.

Library IridescentDishWithOlives.gltf - closed IridescentDishWithOlives.gltf - open
glTF Sample Viewer image image
Three.js r131dev image image
Three.js r131.1 image image

@rohan-percept
Copy link

rohan-percept commented Dec 13, 2021

Hi @mrdoob @donmccurdy, Would it be possible to get an idea or pointer in the right direction with regards to getting the alpha blend materials to show through transmission / volume? I am pretty much new to three js and would appreciate some help.

@mrdoob mrdoob modified the milestones: r136, r137 Dec 24, 2021
@donmccurdy
Copy link
Collaborator

@rohan-percept sorry, I don't have pointers to get things started on (2) or (3).

@mrdoob mrdoob modified the milestones: r137, r138 Jan 26, 2022
@WestLangley
Copy link
Collaborator Author

Related: #23448

@gkjohnson
Copy link
Collaborator

I'm not sure if (3) "transmission through transmission" necessarily requires depth peeling. Another approach would be to copy render contents to a secondary transmission target sources while rendering the transmission material meshes back to front. This would be a bit more limiting (transmission from the same mesh couldn't be seen through the other) but it should work on the above glasses model if the side pieces are separate meshes and I'd think be more performant than depth peeling.

The down side is that it would require another render target and render target copies. Consider the following case with three transmissive meshes sorted back to front A, B, C:

  1. Render full scene including transparent meshes to render target RT2.
  2. For every mesh A, B, C
  • Copy the contents from RT2 to RT1.
  • Render transmissive mesh A using RT1 as transmission texture to RT2.
  1. Copy the contents of RT2 to the canvas.

This is is a rough algorithm, at least. Not sure how how intensive a full target copy would be but maybe it's a starting point. Do we know what Babylon is doing?

Depth peeling is definitely a nice option but definitely expensive and I think a big first step would be getting the ability to use custom and change depth textures on render targets would be a big first step for that (#19554)

@mrdoob
Copy link
Owner

mrdoob commented Feb 10, 2022

I think we could achieve something decent by doing:

  1. Render all opaque meshes into the framebuffer.
  2. Render all opaque meshes into RT1
  3. Copy RT1 into RT2.
  4. Use RT2 as transmission input and render the backside of all doublesided transmissive meshes into RT1.
  5. Use RT1 as transmission input and render transmissive meshes into the framebuffer.
  6. Render transparent meshes into the framebuffer.

Hmm, now that we're moving into WebGL2... Sooner or later we'll have to investigate rendering everything into a multisampled RT in linear and copying the RT into the framebuffer while applying gamma and tonemapping.

Although I think @Mugen87 tried this recently and it was slower than the current setup.

@gkjohnson
Copy link
Collaborator

  1. Use RT2 as transmission input and render the backside of all doublesided transmissive meshes into RT1.
  2. Use RT1 as transmission input and render transmissive meshes into the framebuffer.

I don't know if this is going to have all that great of an effect and probably wouldn't work well on the sun glasses example above. If backfaces are rendered first and used for transmission when rendering front faces then every frontface transmission fragment is effectively going to have the transmission tint doubly applied (once for backside, once for frontside). And with refraction you'll probably get artifacts at the edges of the mesh when the refracted sample doesn't line up with the back face mesh edges.

And if only backfaces are used in the transmission then continuity of things like the top highlight caused by the front ridge of glasses side piece will be lost:

image

Might be worth trying out I just expect there to be more issues than you might expect.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 11, 2022

Although I think @Mugen87 tried this recently and it was slower than the current setup.

Correct. Applying gamma and tone mapping in an additional render pass was noticeably slower than doing it inline. In certain examples, performance dropped from 60 to 40 FPS when using a FP32 render target, see #23019 (comment).

@mrdoob mrdoob modified the milestones: r138, r139 Feb 23, 2022
@mrdoob mrdoob modified the milestones: r139, r140 Mar 24, 2022
@mrdoob mrdoob modified the milestones: r140, r141 Apr 30, 2022
@mrdoob mrdoob modified the milestones: r141, r142 May 26, 2022
@mrdoob mrdoob modified the milestones: r142, r143 Jun 29, 2022
@WestLangley
Copy link
Collaborator Author

Most of the issues raised in the original post have been addressed, so I think it is OK to close this now.

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

No branches or pull requests