google / model-viewer

Easily display interactive 3D models on the web and in AR!
https://modelviewer.dev
Apache License 2.0
6.93k stars 818 forks source link

Rendering inconsistencies with fine detailed normals + roughness #587

Closed pushmatrix closed 5 years ago

pushmatrix commented 5 years ago

Description

I'm having an issue with the lighting on certain PBR models that have a lot of small details in the normal map. The way the details in the normal map are reflecting light seems to be off and the model comes across a bit too shiny and wet.

Below are images in various viewers. They use the same environment map from here: https://github.com/GoogleWebComponents/model-viewer/issues/521#issuecomment-488694339.

Model Viewer

Screen Shot 2019-06-10 at 3 34 52 PM Screen Shot 2019-06-10 at 3 34 58 PM

Sketchfab

Screen Shot 2019-06-10 at 3 29 51 PM Screen Shot 2019-06-10 at 3 29 55 PM

Substance

Screen Shot 2019-06-10 at 3 31 37 PM

AR Quick Look

(Doesn't use the same environment map)

arql

We created the material in substance painter, and Sketchfab & AR Quick Look render it as designed. Model-viewer makes it look like the normal map roughness is really low and reflective.

Live Demo

https://ninth-paste.glitch.me/

Download the model here

Browser Affected

OS

Versions

cdata commented 5 years ago

Thanks for reporting this @pushmatrix ! We'll take a look

pushmatrix commented 5 years ago

FYI: I updated the example given that shows the difference even more.

elalish commented 5 years ago

Okay, so looking at your metallic-roughness map, it looks like you're going for metal = 0 uniformly and roughness a bit noisy, between about 130-155? Inspecting threejs I'm seeing the metalnessMap and roughness map getting the same texture, and maybe some other issues. We might need to check out our gltf loader. I notice you have some indirection here, where scene.gltf encapsulates fabricball.glb; does this still repro if everything is baked into a single gltf?

pushmatrix commented 5 years ago

Yep that's what we're going for. Same thing happens with it baked into one. Here's the gltf

fabricball.gltf.zip

Not sure how the indirection happened. It was originally a .glb from substance then I exported the .gltf + textures from Sketchfab after uploading it.

pushmatrix commented 5 years ago

But yes you're right it does look like it has metalness applied to it.

pushmatrix commented 5 years ago

Playing around with setting the metalness map in Sketchfab to be the same as the roughness, and the model didn't look the same as model viewer:

Screen Shot 2019-06-10 at 5 42 08 PM

However setting the roughness multiplier to 0.5 and then it looked pretty similar to model-viewer:

Screen Shot 2019-06-10 at 5 38 28 PM
elalish commented 5 years ago

I take it back; metalness looks fine. I'm now investigating what I think is a difference between roughness and roughness squared.

pushmatrix commented 5 years ago

Filament does seem a bit less shiny:

Screen Shot 2019-06-12 at 11 43 49 AM
pushmatrix commented 5 years ago

@elalish Is the rendering issue here solved by #591 or did #591 just make it so PMREM is on by default? When this issue was first posted I had mistakenly not turned on PMREM, but then updated the issue to show the fabric ball being way too shiny even with PMREM on.

Does #591 make it less shiny?

elalish commented 5 years ago

No, but I’m also working on that. That’s a bigger change affecting the internals of three js materials and shaders.

On Mon, Jul 22, 2019 at 7:09 PM Daniel Beauchamp notifications@github.com wrote:

@elalish https://github.com/elalish Is the rendering issue here solved by #591 https://github.com/GoogleWebComponents/model-viewer/pull/591 or did #591 https://github.com/GoogleWebComponents/model-viewer/pull/591 just make it so PMREM is on by default? When this issue was first posted I had mistakenly not turned on PMREM, but then updated the issue to show the fabric ball being way too shiny even with PMREM on.

Does #591 https://github.com/GoogleWebComponents/model-viewer/pull/591 make it less shiny?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/GoogleWebComponents/model-viewer/issues/587?email_source=notifications&email_token=AAMS2LHGDL2BEPGQUFN67A3QAZR5RA5CNFSM4HR33KG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2RWHJA#issuecomment-514024356, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMS2LDIZ62T6WL7RQM2TSTQAZR5RANCNFSM4HR33KGQ .

pushmatrix commented 5 years ago

Is it ok if we reopen this issue then?

cdata commented 5 years ago

Sorry about letting this get caught in the issue-closing crossfire @pushmatrix