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

WebGPURenderer: MeshPhysicalNodeMaterial - Fix clearcoatRoughness #28507

Merged
merged 2 commits into from
May 28, 2024

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented May 27, 2024

Description

Value limits and geometry-based roughness adjustments were not applied to clearcoatRoughness.

More perceptive in the red sphere.

@sunag sunag added this to the r165 milestone May 27, 2024
@sunag sunag marked this pull request as ready for review May 27, 2024 21:09
@sunag sunag merged commit 4c6d0c4 into mrdoob:dev May 28, 2024
11 checks passed
@sunag sunag deleted the dev-fix-clearcoatRoughness branch May 28, 2024 01:18
@mrdoob
Copy link
Owner

mrdoob commented May 28, 2024

The material seem to be matching WebGLRenderer now 👌

WebGPU WebGL

But seems like the background is blurrier compared to master 🤔

WebGPU WebGPU (master )

@sunag
Copy link
Collaborator Author

sunag commented May 28, 2024

But seems like the background is blurrier compared to master 🤔

I think this is not related to this PR, I believe it is due to some PMREM update that the screenshot was not updated.

@mrdoob
Copy link
Owner

mrdoob commented May 28, 2024

Yes, it's definitely unrelated to this PR. I just wanted to make sure we're aware.

It's easier to see comparing these examples:

https://raw.githack.com/mrdoob/three.js/dev/examples/webgl_loader_gltf.html
https://raw.githack.com/mrdoob/three.js/dev/examples/webgpu_loader_gltf.html

@sunag
Copy link
Collaborator Author

sunag commented May 28, 2024

WebGLRenderer only uses PMREM if backgroundBlurriness is greater than zero, I added scene.backgroundBlurriness = 0.001; in webgl_loader_gltf and had the same result as in WebGPU. I will improve this in an update.

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.

2 participants