search

donmccurdy / three-gltf-viewer

Drag-and-drop preview for glTF 2.0 models in WebGL using three.js.
https://gltf-viewer.donmccurdy.com/
MIT License
2.09k stars 534 forks source link

Add checkbox for `.physicallyCorrectLights`? #315

Closed will-ca closed 1 year ago

will-ca commented 1 year ago

To better support the lights extension, and in this case so I can actually test https://github.com/KhronosGroup/glTF-Blender-IO/pull/1760

I was going to PR this, but today does not feel like a good day to install Node.JS.

donmccurdy commented 1 year ago

Hi! It's on by default in this viewer —

https://github.com/donmccurdy/three-gltf-viewer/blob/baa473a87250ebcf1064452a55bdc13a03385715/src/viewer.js#L100-L105

I've been hoping to discourage the non-physically-based mode, and eventually remove that from three.js, so I'd rather not let people disable that here if it's OK! There's no easy conversion of a glTF light to the non-physically-based mode.

will-ca commented 1 year ago

@donmccurdy Okay... I assumed when I was opening test models that it was off, because glTFs with light intensities scaled to reasonable values for lumens (I.E. a couple dozen to several hundred) looked really blown out, while light intensities scaled to non-physical values (E.G. 0.5 to 10.0) looked more like how they did in Blender. ...Which I just tried again and is still the case. I suppose that means that I have to play with the exposure?

....But then, the "exposure" slider doesn't actually do anything AFAICT on the website, and IIRC when I previously used it in my own application the exposure property only had an effect when I had tonemapping enabled or something like that?

I'm sorry, I've just gotten more and more confused as I've tried to dig deeper 😆. This is a pipeline I come back to every now and then, only to first discover I need something new implemented and then that I'm not as familiar with the system as a whole as I remembered.

Okay, can you advise on how I can use the viewer to test that a glTF file, exported with lumen-based units for light intensity as per KHR_lights_punctual, looks as it "should"?

donmccurdy commented 1 year ago

Sorry for that, the exposure slider is indeed broken at the moment. (https://github.com/donmccurdy/three-gltf-viewer/issues/269) 😔

glTFs with light intensities scaled to reasonable values for lumens (I.E. a couple dozen to several hundred) looked really blown out, while light intensities scaled to non-physical values (E.G. 0.5 to 10.0) looked more like how they did in Blender.

I've been testing Blender exports with your patch this evening, and am seeing exactly the same thing. Testing in several engines (three.js, babylon.js, khronos viewer) are all pretty close to one other, and all severely blown out compared to Blender.

So I think something is not quite going as expected, either in our conversion math or in Blender itself. Let's discuss in the other threads, may need more eyes on this!

donmccurdy commented 1 year ago

Fixed exposure slider and added a tone mapping option in f656ab26920ebc0109b9b50d218caea89bde0522